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

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Prev Previous commit
Next Next commit
add linter
dickermoshe committed Dec 12, 2024
commit 3a97943c5a05367f05cea152617005c781483e28
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -37,3 +37,6 @@ docs/docs/*.wasm
docs/docs/*.css
docs/docs/examples/**
docs/web/robots.txt

# Linting
custom_lint.log
7 changes: 7 additions & 0 deletions docs/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
include: package:lints/recommended.yaml

analyzer:
plugins:
- custom_lint
language:
strict-casts: true
exclude:
- "**/*.g.dart"

custom_lint:
debug: true
# Optional, will cause custom_lint to log its internal debug information
verbose: true
3 changes: 2 additions & 1 deletion docs/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@ dependencies:
sqlite3: ^2.0.0

# Used in examples
rxdart: ^0.27.3
rxdart: ^0.28.0
yaml: ^3.1.1
drift_dev: any
test: ^1.18.0
@@ -46,6 +46,7 @@ dev_dependencies:
hosted: https://pub-simonbinder-eu.fsn1.your-objectstorage.com
version: ^0.0.15
sass_builder: ^2.2.1
custom_lint: ^0.6.7

dependency_overrides:
drift_flutter:
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() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of this, is there a way to tell custom_lint to use a different import for a package providing linters (my preferred library would be lib/integrations/custom_lint.dart).

But if that's not possible so be it, I can file an issue on custom_lint and we can leave this as-is until that gets resolved.

return DriftLinter();
}
26 changes: 18 additions & 8 deletions drift_dev/lib/src/analysis/driver/error.dart
Original file line number Diff line number Diff line change
@@ -4,39 +4,49 @@ import 'package:source_gen/source_gen.dart';
import 'package:source_span/source_span.dart';
import 'package:sqlparser/sqlparser.dart' as sql;

enum DriftAnalysisErrorLevel { warning, error }

class DriftAnalysisError {
final SourceSpan? span;
final String message;
final DriftAnalysisErrorLevel level;

DriftAnalysisError(this.span, this.message);
DriftAnalysisError(this.span, this.message,
{this.level = DriftAnalysisErrorLevel.error});

factory DriftAnalysisError.forDartElement(
dart.Element element, String message) {
dart.Element element, String message,
{DriftAnalysisErrorLevel level = DriftAnalysisErrorLevel.error}) {
return DriftAnalysisError(
spanForElement(element),
message,
level: level,
);
}

factory DriftAnalysisError.inDartAst(
dart.Element element, dart.SyntacticEntity entity, String message) {
return DriftAnalysisError(dartAstSpan(element, entity), message);
dart.Element element, dart.SyntacticEntity entity, String message,
{DriftAnalysisErrorLevel level = DriftAnalysisErrorLevel.error}) {
return DriftAnalysisError(dartAstSpan(element, entity), message,
level: level);
}

factory DriftAnalysisError.inDriftFile(
sql.SyntacticEntity sql, String message) {
return DriftAnalysisError(sql.span, message);
sql.SyntacticEntity sql, String message,
{DriftAnalysisErrorLevel level = DriftAnalysisErrorLevel.error}) {
return DriftAnalysisError(sql.span, message, level: level);
}

factory DriftAnalysisError.fromSqlError(sql.AnalysisError error) {
factory DriftAnalysisError.fromSqlError(sql.AnalysisError error,
{DriftAnalysisErrorLevel level = DriftAnalysisErrorLevel.error}) {
var message = error.message ?? '';
if (error.type == sql.AnalysisErrorType.notSupportedInDesiredVersion) {
message =
'$message\nNote: You can change the assumed sqlite version with build '
'options. See https://drift.simonbinder.eu/options/#assumed-sql-environment for details!';
}

return DriftAnalysisError(error.span, message);
return DriftAnalysisError(error.span, message, level: level);
}

@override
5 changes: 4 additions & 1 deletion drift_dev/lib/src/analysis/resolver/dart/column.dart
Original file line number Diff line number Diff line change
@@ -509,6 +509,7 @@ class ColumnParser {
customConstraints: foundCustomConstraint,
referenceName: _readReferenceName(element),
),
element: element,
referencesColumnInSameTable: referencesColumnInSameTable,
);
}
@@ -661,6 +662,7 @@ class ColumnParser {

class PendingColumnInformation {
final DriftColumn column;
final Element element;

/// If the returned column references another column in the same table, its
/// [ForeignKeyReference] is still unresolved when the local column resolver
@@ -670,5 +672,6 @@ class PendingColumnInformation {
/// this column in that case.
final String? referencesColumnInSameTable;

PendingColumnInformation(this.column, {this.referencesColumnInSameTable});
PendingColumnInformation(this.column,
{this.referencesColumnInSameTable, required this.element});
}
8 changes: 8 additions & 0 deletions drift_dev/lib/src/analysis/resolver/dart/table.dart
Original file line number Diff line number Diff line change
@@ -42,6 +42,14 @@ class DartTableResolver extends LocalElementResolver<DiscoveredDartTable> {
} else {
for (final constraint in column.column.constraints) {
if (constraint is ForeignKeyReference) {
if (column.column.sqlType.builtin !=
constraint.otherColumn.sqlType.builtin ||
column.column.typeConverter?.dartType !=
constraint.otherColumn.typeConverter?.dartType) {
reportError(DriftAnalysisError.forDartElement(column.element,
"This column references a column whose type doesn't match this one. The generated managers will ignore this relation",
level: DriftAnalysisErrorLevel.warning));
}
references.add(constraint.otherColumn.owner);
}
}
18 changes: 18 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,18 @@
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:drift_dev/src/lints/drift_backend_error_lint.dart';
import 'package:drift_dev/src/lints/non_null_insert_with_ignore_lint.dart';
import 'package:drift_dev/src/lints/offset_without_limit_lint.dart';
import 'package:drift_dev/src/lints/unawaited_futures_in_transaction_lint.dart';
import 'package:meta/meta.dart';

@internal
class DriftLinter extends PluginBase {
@override
List<LintRule> getLintRules(CustomLintConfigs configs) => [
unawaitedFuturesInMigration,
unawaitedFuturesInTransaction,
OffsetWithoutLimit(),
DriftBuildErrors(),
NonNullInsertWithIgnore()
];
}
113 changes: 113 additions & 0 deletions drift_dev/lib/src/lints/drift_backend_error_lint.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
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/driver/error.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: _errorCode);

static const _errorCode = LintCode(
name: 'drift_build_errors',
problemMessage: '{0}',
errorSeverity: ErrorSeverity.ERROR,
);
LintCode get _warningCode => LintCode(
name: _errorCode.name,
problemMessage: _errorCode.problemMessage,
errorSeverity: ErrorSeverity.WARNING);

@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?) {
// ignore: deprecated_member_use
reporter.reportErrorForSpan(
error.level == DriftAnalysisErrorLevel.warning
? _warningCode
: _errorCode,
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);
}
60 changes: 60 additions & 0 deletions drift_dev/lib/src/lints/non_null_insert_with_ignore_lint.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import 'package:analyzer/dart/ast/ast.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 managerTypeChecker =
TypeChecker.fromName('BaseTableManager', packageName: 'drift');
final insertStatementChecker =
TypeChecker.fromName('InsertStatement', packageName: 'drift');
final insertOrIgnoreChecker =
TypeChecker.fromName('InsertMode', packageName: 'drift');

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

static const _code = LintCode(
name: 'non_null_insert_with_ignore',
problemMessage:
'`insertReturning` and `createReturning` will throw an exception if a row isn\'t actually inserted. Use `createReturningOrNull` or `insertReturningOrNull` if you want to ignore conflicts.',
errorSeverity: ErrorSeverity.WARNING,
);

@override
void run(CustomLintResolver resolver, ErrorReporter reporter,
CustomLintContext context) async {
context.registry.addMethodInvocation(
(node) {
if (node.argumentList.arguments.isEmpty) return;
switch (node.function) {
case SimpleIdentifier func:
if (func.name == "insertReturning" ||
func.name == "createReturning") {
switch (func.parent) {
case MethodInvocation func:
final targetType = func.realTarget?.staticType;
if (targetType != null) {
if (managerTypeChecker.isSuperTypeOf(targetType) ||
insertStatementChecker.isExactlyType(targetType)) {
final namedArgs = func.argumentList.arguments
.whereType<NamedExpression>();
for (final arg in namedArgs) {
if (arg.name.label.name == "mode") {
switch (arg.expression) {
case PrefixedIdentifier mode:
if (mode.identifier.name == "insertOrIgnore") {
print("Found insertOrIgnore");
reporter.atNode(node, _code);
}
}
}
}
}
}
}
}
}
},
);
}
}
51 changes: 51 additions & 0 deletions drift_dev/lib/src/lints/offset_without_limit_lint.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
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';

final managerTypeChecker =
TypeChecker.fromName('BaseTableManager', packageName: 'drift');

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

static const _code = LintCode(
name: 'offset_without_limit',
problemMessage: 'Using offset without a limit doesnt have any effect.',
errorSeverity: ErrorSeverity.ERROR,
);

@override
void run(CustomLintResolver resolver, ErrorReporter reporter,
CustomLintContext context) async {
context.registry.addMethodInvocation(
(node) {
if (node.argumentList.arguments.isEmpty) return;
final func = _typeCheck<SimpleIdentifier>(node.function);

if (func?.name == "get" || func?.name == "watch") {
final target = _typeCheck<PrefixedIdentifier>(node.target);
final managerGetter =
_typeCheck<PropertyAccessorElement>(target?.staticElement);
if (managerGetter != null) {
if (managerTypeChecker.isSuperTypeOf(managerGetter.returnType)) {
final namedArgs =
node.argumentList.arguments.whereType<NamedExpression>();
if (namedArgs
.every((element) => element.name.label.name != "limit") &&
namedArgs
.any((element) => element.name.label.name == "offset")) {
reporter.atNode(node, _code);
}
}
}
}
},
);
}
}

T? _typeCheck<T>(i) {
return i is T ? i : null;
}
Loading