Skip to content

Commit

Permalink
add migration future lint
Browse files Browse the repository at this point in the history
  • Loading branch information
dickermoshe committed Oct 14, 2024
1 parent f2da76c commit 53ee4e6
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 34 deletions.
3 changes: 2 additions & 1 deletion drift_dev/lib/src/lints/custom_lint_plugin.dart
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:drift_dev/src/lints/offset_without_limit.dart';
import 'package:drift_dev/src/lints/drift_backend_errors.dart';
import 'package:drift_dev/src/lints/unawaited_futures_in_transaction.dart';
import 'package:drift_dev/src/lints/unawaited_futures.dart';
import 'package:meta/meta.dart';

@internal
class DriftLinter extends PluginBase {
@override
List<LintRule> getLintRules(CustomLintConfigs configs) => [
UnawaitedFuturesInTransaction(),
UnawaitedFuturesInMigration(),
DriftBuildErrors(),
OffsetWithoutLimit()
];
Expand Down
9 changes: 0 additions & 9 deletions drift_dev/lib/src/lints/offset_without_limit.dart
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
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 managerTypeChecker =
TypeChecker.fromName('BaseTableManager', packageName: 'drift');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ final databaseConnectionUserChecker =
TypeChecker.fromName('DatabaseConnectionUser', packageName: 'drift');
final columnBuilderChecker =
TypeChecker.fromName('ColumnBuilder', packageName: 'drift');
final migrationStrategyChecker =
TypeChecker.fromName('MigrationStrategy', packageName: 'drift');

class UnawaitedFuturesInTransaction extends DartLintRule {
UnawaitedFuturesInTransaction() : super(code: _code);
Expand All @@ -24,14 +26,70 @@ class UnawaitedFuturesInTransaction extends DartLintRule {
@override
void run(CustomLintResolver resolver, ErrorReporter reporter,
CustomLintContext context) {
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;
}

context.registry.addExpressionStatement((node) {
node.accept(
_Visitor(this, reporter, _code, additionalCheck: inTransactionBlock));
});
context.registry.addCascadeExpression((node) {
node.accept(
_Visitor(this, reporter, _code, additionalCheck: inTransactionBlock));
});
context.registry.addInterpolationExpression((node) {
node.accept(
_Visitor(this, reporter, _code, additionalCheck: inTransactionBlock));
});
}
}

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

static const _code = LintCode(
name: 'unawaited_futures_in_migration',
problemMessage:
'All futures in a migrations should be awaited to ensure that all operations are completed before the other opperations are performed.',
errorSeverity: ErrorSeverity.ERROR,
);
@override
void run(CustomLintResolver resolver, ErrorReporter reporter,
CustomLintContext context) {
bool inMigrationBlock(AstNode node) {
return node.thisOrAncestorMatching((node) =>
(node is InstanceCreationExpression &&
node.staticType != null &&
migrationStrategyChecker.isExactlyType(node.staticType!))) !=
null;
}

context.registry.addExpressionStatement((node) {
node.accept(_Visitor(this, reporter, _code));
node.accept(
_Visitor(this, reporter, _code, additionalCheck: inMigrationBlock));
});
context.registry.addCascadeExpression((node) {
node.accept(_Visitor(this, reporter, _code));
node.accept(
_Visitor(this, reporter, _code, additionalCheck: inMigrationBlock));
});
context.registry.addInterpolationExpression((node) {
node.accept(_Visitor(this, reporter, _code));
node.accept(
_Visitor(this, reporter, _code, additionalCheck: inMigrationBlock));
});
}
}
Expand All @@ -40,12 +98,16 @@ class UnawaitedFuturesInTransaction extends DartLintRule {
// 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.
// Source: https://github.com/dart-lang/sdk/blob/main/pkg/linter/lib/src/rules/unawaited_futures.dart
/// A visitor which will report any future that is not awaited.
/// If an [additionalCheck] is provided, it will be used to check if the future should be reported
class _Visitor extends SimpleAstVisitor<void> {
final LintRule rule;
final ErrorReporter reporter;
final LintCode code;
final bool Function(AstNode node) additionalCheck;

_Visitor(this.rule, this.reporter, this.code);
_Visitor(this.rule, this.reporter, this.code,
{required this.additionalCheck});

@override
void visitCascadeExpression(CascadeExpression node) {
Expand All @@ -71,9 +133,10 @@ class _Visitor extends SimpleAstVisitor<void> {
return;
}

if (_isEnclosedInAsyncFunctionBody(node) && _inTransactionBlock(node)) {
if (_isEnclosedInAsyncFunctionBody(node) && additionalCheck(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);
}
}
Expand All @@ -89,24 +152,6 @@ class _Visitor extends SimpleAstVisitor<void> {
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) =>
Expand All @@ -128,7 +173,7 @@ class _Visitor extends SimpleAstVisitor<void> {
if ((expr.staticType?.isDartAsyncFuture ?? false) &&
_isEnclosedInAsyncFunctionBody(expr) &&
expr is! AssignmentExpression &&
_inTransactionBlock(expr)) {
additionalCheck(expr)) {
reporter.atNode(expr, code);
}
}
Expand Down
7 changes: 7 additions & 0 deletions drift_dev/test/linter_test/pkg/lib/db.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ class TestDatabase extends _$TestDatabase {
@override
int get schemaVersion => 1;

@override
MigrationStrategy get migration => MigrationStrategy(
onUpgrade: (m, from, to) async {
m.addColumn(users, users.name);
},
);

a() async {
// expect_lint: offset_without_limit
managers.users.get(offset: 1);
Expand Down

0 comments on commit 53ee4e6

Please sign in to comment.