Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linter for drift #3281

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,7 @@ docs/.cache
docs/docs/*.js
docs/docs/*.wasm
docs/docs/examples/**
docs/web/robots.txt
docs/web/robots.txt

# Linting
custom_lint.log
7 changes: 7 additions & 0 deletions drift_dev/lib/drift_dev.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:drift_dev/src/lints/custom_lint_plugin.dart';

/// This function is automaticly recognized by custom_lint to include this drift_dev package as a linter
PluginBase createPlugin() {
return DriftLinter();
}
11 changes: 11 additions & 0 deletions drift_dev/lib/src/lints/custom_lint_plugin.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:drift_dev/src/lints/drift_backend_errors.dart';
import 'package:drift_dev/src/lints/unawaited_futures_in_transaction.dart';
import 'package:meta/meta.dart';

@internal
class DriftLinter extends PluginBase {
@override
List<LintRule> getLintRules(CustomLintConfigs configs) =>
[UnawaitedFuturesInTransaction(), DriftBuildErrors()];
}
102 changes: 102 additions & 0 deletions drift_dev/lib/src/lints/drift_backend_errors.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import 'dart:io';

import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/error/error.dart' hide LintCode;
import 'package:analyzer/error/listener.dart';
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:drift_dev/src/analysis/backend.dart';
import 'package:drift_dev/src/analysis/options.dart';
import 'package:logging/logging.dart';

import '../analysis/driver/driver.dart';

final columnBuilderChecker =
TypeChecker.fromName('DriftDatabase', packageName: 'drift');

class DriftBuildErrors extends DartLintRule {
DriftBuildErrors() : super(code: _code);

static const _code = LintCode(
name: 'drift_build_errors',
problemMessage: '{0}',
errorSeverity: ErrorSeverity.ERROR,
);

@override
void run(CustomLintResolver resolver, ErrorReporter reporter,
CustomLintContext context) async {
final unit = await resolver.getResolvedUnitResult();
final backend = CustomLintBackend(unit.session);
final driver = DriftAnalysisDriver(backend, const DriftOptions.defaults());

final file = await driver.fullyAnalyze(unit.uri);
for (final error in file.allErrors) {
if (error.span case final span?) {
reporter.reportErrorForSpan(_code, span, [error.message.trim()]);
}
}
}
}

class CustomLintBackend extends DriftBackend {
@override
final Logger log = Logger('drift_dev.CustomLintBackend');
final AnalysisSession session;

CustomLintBackend(this.session);

@override
bool get canReadDart => true;

@override
Future<AstNode?> loadElementDeclaration(Element element) async {
final library = element.library;
if (library == null) return null;

final info = await library.session.getResolvedLibraryByElement(library);
if (info is ResolvedLibraryResult) {
return info.getElementDeclaration(element)?.node;
} else {
return null;
}
}

@override
Future<String> readAsString(Uri uri) async {
final file = session.getFile(uri.path);

if (file is FileResult) {
return file.content;
}

throw FileSystemException('Not a file result: $file');
}

@override
Future<LibraryElement> readDart(Uri uri) async {
final result = await session.getLibraryByUri(uri.toString());
if (result is LibraryElementResult) {
return result.element;
}

throw NotALibraryException(uri);
}

@override
Future<Expression> resolveExpression(
Uri context, String dartExpression, Iterable<String> imports) {
throw CannotReadExpressionException('Not supported at the moment');
}

@override
Future<Element?> resolveTopLevelElement(
Uri context, String reference, Iterable<Uri> imports) {
throw UnimplementedError();
}

@override
Uri resolveUri(Uri base, String uriString) => base.resolve(uriString);
}
156 changes: 156 additions & 0 deletions drift_dev/lib/src/lints/unawaited_futures_in_transaction.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/error/error.dart' hide LintCode;
import 'package:analyzer/error/listener.dart';
import 'package:custom_lint_builder/custom_lint_builder.dart';

final tableChecker = TypeChecker.fromName('Table', packageName: 'drift');
final databaseConnectionUserChecker =
TypeChecker.fromName('DatabaseConnectionUser', packageName: 'drift');
final columnBuilderChecker =
TypeChecker.fromName('ColumnBuilder', packageName: 'drift');

class UnawaitedFuturesInTransaction extends DartLintRule {
UnawaitedFuturesInTransaction() : super(code: _code);

static const _code = LintCode(
name: 'unawaited_futures_in_transaction',
problemMessage:
'All futures in a transaction should be awaited to ensure that all operations are completed before the transaction is closed.',
errorSeverity: ErrorSeverity.ERROR,
);
@override
void run(CustomLintResolver resolver, ErrorReporter reporter,
CustomLintContext context) {
context.registry.addExpressionStatement((node) {
node.accept(_Visitor(this, reporter, _code));
});
context.registry.addCascadeExpression((node) {
node.accept(_Visitor(this, reporter, _code));
});
context.registry.addInterpolationExpression((node) {
node.accept(_Visitor(this, reporter, _code));
});
}
}

// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
dickermoshe marked this conversation as resolved.
Show resolved Hide resolved
class _Visitor extends SimpleAstVisitor<void> {
final LintRule rule;
final ErrorReporter reporter;
final LintCode code;

_Visitor(this.rule, this.reporter, this.code);

@override
void visitCascadeExpression(CascadeExpression node) {
var sections = node.cascadeSections;
for (var i = 0; i < sections.length; i++) {
_visit(sections[i]);
}
}

@override
void visitExpressionStatement(ExpressionStatement node) {
var expr = node.expression;
if (expr is AssignmentExpression) return;

var type = expr.staticType;
if (type == null) {
return;
}
if (type.implementsInterface('Future', 'dart.async')) {
// Ignore a couple of special known cases.
if (_isFutureDelayedInstanceCreationWithComputation(expr) ||
_isMapPutIfAbsentInvocation(expr)) {
return;
}

if (_isEnclosedInAsyncFunctionBody(node) && _inTransactionBlock(node)) {
// Future expression statement that isn't awaited in an async function:
// while this is legal, it's a very frequent sign of an error.
reporter.atNode(node, code);
}
}
}

@override
void visitInterpolationExpression(InterpolationExpression node) {
_visit(node.expression);
}

bool _isEnclosedInAsyncFunctionBody(AstNode node) {
var enclosingFunctionBody = node.thisOrAncestorOfType<FunctionBody>();
return enclosingFunctionBody?.isAsynchronous ?? false;
}

bool _inTransactionBlock(AstNode node) {
return node.thisOrAncestorMatching(
(method) {
if (method is! MethodInvocation) return false;
final methodElement = method.methodName.staticElement;
if (methodElement is! MethodElement ||
methodElement.name != 'transaction') return false;
final enclosingElement = methodElement.enclosingElement;
if (enclosingElement is! ClassElement ||
!databaseConnectionUserChecker.isExactly(enclosingElement)) {
return false;
}
return true;
},
) !=
null;
}

/// Detects `Future.delayed(duration, [computation])` creations with a
/// computation.
bool _isFutureDelayedInstanceCreationWithComputation(Expression expr) =>
expr is InstanceCreationExpression &&
(expr.staticType?.isDartAsyncFuture ?? false) &&
expr.constructorName.name?.name == 'delayed' &&
expr.argumentList.arguments.length == 2;

bool _isMapClass(Element? e) =>
e is ClassElement && e.name == 'Map' && e.library.name == 'dart.core';

/// Detects Map.putIfAbsent invocations.
bool _isMapPutIfAbsentInvocation(Expression expr) =>
expr is MethodInvocation &&
expr.methodName.name == 'putIfAbsent' &&
_isMapClass(expr.methodName.staticElement?.enclosingElement);

void _visit(Expression expr) {
if ((expr.staticType?.isDartAsyncFuture ?? false) &&
_isEnclosedInAsyncFunctionBody(expr) &&
expr is! AssignmentExpression &&
_inTransactionBlock(expr)) {
reporter.atNode(expr, code);
}
}
}

extension DartTypeExtension on DartType? {
bool implementsInterface(String interface, String library) {
var self = this;
if (self is! InterfaceType) {
return false;
}
bool predicate(InterfaceType i) => i.isSameAs(interface, library);
var element = self.element;
return predicate(self) ||
!element.isSynthetic && element.allSupertypes.any(predicate);
}

/// Returns whether `this` is the same element as [interface], declared in
/// [library].
bool isSameAs(String? interface, String? library) {
var self = this;
return self is InterfaceType &&
self.element.name == interface &&
self.element.library.name == library;
}
}
3 changes: 3 additions & 0 deletions drift_dev/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ dependencies:
source_gen: ">=0.9.4 <2.0.0"
string_scanner: ^1.1.1

# Linter
custom_lint_builder: ^0.6.7

dev_dependencies:
lints: ^4.0.0
checked_yaml: ^2.0.1
Expand Down
20 changes: 20 additions & 0 deletions drift_dev/test/linter_test/linter_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import 'dart:io';

import 'package:test/test.dart';
import 'package:path/path.dart' as p;

void main() {
// Test the linter_test.dart file
test('linter', () async {
final workingDir = p.join(p.current, 'test/linter_test/pkg');
expect(
await Process.run('dart', ['pub', 'get'], workingDirectory: workingDir)
.then((v) => v.exitCode),
0);
expect(
simolus3 marked this conversation as resolved.
Show resolved Hide resolved
await Process.run('custom_lint', ['--fatal-infos', '--fatal-warnings'],
workingDirectory: workingDir)
.then((v) => v.exitCode),
0);
});
}
11 changes: 11 additions & 0 deletions drift_dev/test/linter_test/pkg/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
include: package:lints/recommended.yaml

analyzer:
plugins:
- custom_lint
language:
strict-casts: true

custom_lint:
debug: true
verbose: true
32 changes: 32 additions & 0 deletions drift_dev/test/linter_test/pkg/lib/db.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import 'package:drift/drift.dart';

part 'db.g.dart';

class Users extends Table {
late final id = integer().autoIncrement()();
late final name = text()();
// expect_lint: drift_build_errors
late final age = integer();
}

class BrokenTable extends Table {
// expect_lint: drift_build_errors
IntColumn get unknownRef => integer().customConstraint('CHECK foo > 10')();
}

@DriftDatabase(tables: [Users])
class TestDatabase extends _$TestDatabase {
TestDatabase(super.e);

@override
int get schemaVersion => 1;

a() async {
transaction(
() async {
// expect_lint: unawaited_futures_in_transaction
into(users).insert(UsersCompanion.insert(name: 'name'));
},
);
}
}
Loading
Loading