mirror of https://github.com/AMT-Cheif/drift.git
Warn about invalid custom foreign key constraint
This commit is contained in:
parent
e0d5520ee1
commit
849f245f53
|
@ -1,6 +1,8 @@
|
|||
import 'package:analyzer/dart/ast/ast.dart';
|
||||
import 'package:analyzer/dart/ast/syntactic_entity.dart';
|
||||
import 'package:analyzer/dart/element/element.dart';
|
||||
import 'package:collection/collection.dart';
|
||||
import 'package:sqlparser/sqlparser.dart' as sql;
|
||||
|
||||
import '../../driver/error.dart';
|
||||
import '../../results/results.dart';
|
||||
|
@ -58,7 +60,7 @@ class DartTableResolver extends LocalElementResolver<DiscoveredDartTable> {
|
|||
for (final uniqueKey in uniqueKeys ?? const <Set<DriftColumn>>[])
|
||||
UniqueColumns(uniqueKey),
|
||||
],
|
||||
overrideTableConstraints: await _readCustomConstraints(element),
|
||||
overrideTableConstraints: await _readCustomConstraints(columns, element),
|
||||
withoutRowId: await _overrideWithoutRowId(element) ?? false,
|
||||
);
|
||||
|
||||
|
@ -270,7 +272,8 @@ class DartTableResolver extends LocalElementResolver<DiscoveredDartTable> {
|
|||
return ColumnParser(this).parse(declaration, element);
|
||||
}
|
||||
|
||||
Future<List<String>> _readCustomConstraints(ClassElement element) async {
|
||||
Future<List<String>> _readCustomConstraints(
|
||||
List<DriftColumn> localColumns, ClassElement element) async {
|
||||
final customConstraints =
|
||||
element.lookUpGetter('customConstraints', element.library);
|
||||
|
||||
|
@ -289,6 +292,7 @@ class DartTableResolver extends LocalElementResolver<DiscoveredDartTable> {
|
|||
}
|
||||
final expression = body.expression;
|
||||
final foundConstraints = <String>[];
|
||||
final foundConstraintSources = <SyntacticEntity>[];
|
||||
|
||||
if (expression is ListLiteral) {
|
||||
for (final entry in expression.elements) {
|
||||
|
@ -296,6 +300,7 @@ class DartTableResolver extends LocalElementResolver<DiscoveredDartTable> {
|
|||
final value = entry.stringValue;
|
||||
if (value != null) {
|
||||
foundConstraints.add(value);
|
||||
foundConstraintSources.add(entry);
|
||||
}
|
||||
} else {
|
||||
reportError(DriftAnalysisError.inDartAst(
|
||||
|
@ -307,6 +312,52 @@ class DartTableResolver extends LocalElementResolver<DiscoveredDartTable> {
|
|||
customConstraints, 'This must return a list literal!'));
|
||||
}
|
||||
|
||||
// Try to parse these constraints and emit warnings
|
||||
final engine = resolver.driver.newSqlEngine();
|
||||
for (var i = 0; i < foundConstraintSources.length; i++) {
|
||||
final parsed = engine.parseTableConstraint(foundConstraints[i]).rootNode;
|
||||
|
||||
if (parsed is sql.InvalidStatement) {
|
||||
reportError(DriftAnalysisError.inDartAst(
|
||||
customConstraints,
|
||||
foundConstraintSources[i],
|
||||
'Could not parse this table constraint'));
|
||||
} else if (parsed is sql.ForeignKeyTableConstraint) {
|
||||
final source = foundConstraintSources[i];
|
||||
|
||||
// Check that the columns exist locally
|
||||
final missingLocals = parsed.columns.where(
|
||||
(e) => localColumns.every((l) => !l.hasEqualSqlName(e.columnName)));
|
||||
if (missingLocals.isNotEmpty) {
|
||||
reportError(DriftAnalysisError.inDartAst(
|
||||
element,
|
||||
source,
|
||||
'Columns ${missingLocals.join(', ')} don\'t exist locally.',
|
||||
));
|
||||
}
|
||||
|
||||
// Also see if we can resolve the referenced table.
|
||||
final clause = parsed.clause;
|
||||
final table = await resolveSqlReferenceOrReportError<DriftTable>(
|
||||
clause.foreignTable.tableName,
|
||||
(msg) => DriftAnalysisError.inDartAst(element, source, msg));
|
||||
|
||||
if (table != null) {
|
||||
final missingColumns = clause.columnNames
|
||||
.map((e) => e.columnName)
|
||||
.where((e) => !table.columnBySqlName.containsKey(e));
|
||||
|
||||
if (missingColumns.isNotEmpty) {
|
||||
reportError(DriftAnalysisError.inDartAst(
|
||||
element,
|
||||
source,
|
||||
'Columns ${missingColumns.join(', ')} not found in table `${table.schemaName}`.',
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return foundConstraints;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -324,8 +324,44 @@ class WithConstraints extends Table {
|
|||
isDriftError('This must return a list literal!')
|
||||
.withSpan('customConstraints')
|
||||
]);
|
||||
expect(withConstraints.errorsDuringAnalysis,
|
||||
[isDriftError('This must be a string literal.').withSpan('1')]);
|
||||
expect(withConstraints.errorsDuringAnalysis, [
|
||||
isDriftError('This must be a string literal.').withSpan('1'),
|
||||
isDriftError('Could not parse this table constraint').withSpan("'two'"),
|
||||
isDriftError('Could not parse this table constraint').withSpan("'three'"),
|
||||
]);
|
||||
});
|
||||
|
||||
test('warns about foreign key references from customConstraints', () async {
|
||||
final backend = TestBackend.inTest({
|
||||
'a|lib/a.dart': '''
|
||||
import 'package:drift/drift.dart';
|
||||
|
||||
class References extends Table {
|
||||
IntColumn get id => integer().autoIncrement()();
|
||||
}
|
||||
|
||||
class WithConstraints extends Table {
|
||||
IntColumn get id => integer().autoIncrement()();
|
||||
IntColumn get foo => integer()();
|
||||
|
||||
@override
|
||||
List<String> get customConstraints => [
|
||||
'FOREIGN KEY (bar) REFERENCES foo (bar)',
|
||||
'FOREIGN KEY (foo) REFERENCES "references" (bar)',
|
||||
'FOREIGN KEY (foo) REFERENCES "references" (id)',
|
||||
];
|
||||
}
|
||||
'''
|
||||
});
|
||||
|
||||
final state = await backend.analyze('package:a/a.dart');
|
||||
final withConstraints = state.analysis[state.id('with_constraints')]!;
|
||||
|
||||
expect(withConstraints.errorsDuringAnalysis, [
|
||||
isDriftError("Columns bar don't exist locally."),
|
||||
isDriftError("`foo` could not be found in any import."),
|
||||
isDriftError("Columns bar not found in table `references`."),
|
||||
]);
|
||||
});
|
||||
|
||||
test('supports autoIncrement on int64 columns', () async {
|
||||
|
|
|
@ -164,6 +164,30 @@ class SqlEngine {
|
|||
);
|
||||
}
|
||||
|
||||
/// Parses [sql] as a single table constraint.
|
||||
///
|
||||
/// The [ParseResult.rootNode] will either be a [TableConstraint] or an
|
||||
/// [InvalidStatement] in case of parsing errors.
|
||||
ParseResult parseTableConstraint(String sql) {
|
||||
final tokens = tokenize(sql);
|
||||
final parser = _createParser(tokens, driftExtensions: false);
|
||||
|
||||
AstNode? constraint;
|
||||
try {
|
||||
constraint = parser.tableConstraintOrNull(requireConstraint: true);
|
||||
} on ParsingError {
|
||||
// Ignore, will be added to parser.errors anyway
|
||||
}
|
||||
|
||||
return ParseResult._(
|
||||
constraint ?? InvalidStatement(),
|
||||
tokens,
|
||||
parser.errors,
|
||||
sql,
|
||||
null,
|
||||
);
|
||||
}
|
||||
|
||||
/// Parses a `.drift` file, which can consist of multiple statements and
|
||||
/// additional components like import statements.
|
||||
ParseResult parseDriftFile(String content) {
|
||||
|
|
|
@ -2066,7 +2066,7 @@ class Parser {
|
|||
|
||||
do {
|
||||
try {
|
||||
final tableConstraint = _tableConstraintOrNull();
|
||||
final tableConstraint = tableConstraintOrNull();
|
||||
|
||||
if (tableConstraint != null) {
|
||||
encounteredTableConstraint = true;
|
||||
|
@ -2614,7 +2614,7 @@ class Parser {
|
|||
_error('Expected a constraint (primary key, nullability, etc.)');
|
||||
}
|
||||
|
||||
TableConstraint? _tableConstraintOrNull() {
|
||||
TableConstraint? tableConstraintOrNull({bool requireConstraint = false}) {
|
||||
final first = _peek;
|
||||
final nameToken = _constraintNameOrNull();
|
||||
final name = nameToken?.identifier;
|
||||
|
@ -2657,7 +2657,7 @@ class Parser {
|
|||
return result;
|
||||
}
|
||||
|
||||
if (name != null) {
|
||||
if (name != null || requireConstraint) {
|
||||
// if a constraint was started with CONSTRAINT <name> but then we didn't
|
||||
// find a constraint, that's an syntax error
|
||||
_error('Expected a table constraint (e.g. a primary key)');
|
||||
|
|
|
@ -74,4 +74,35 @@ void main() {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
group('parseTableConstraint', () {
|
||||
test('parses constraint', () {
|
||||
final result = SqlEngine().parseTableConstraint(
|
||||
'CONSTRAINT foo FOREIGN KEY (a, b) REFERENCES c (d, e);');
|
||||
expect(result.errors, isEmpty);
|
||||
|
||||
enforceEqual(
|
||||
result.rootNode,
|
||||
ForeignKeyTableConstraint(
|
||||
'foo',
|
||||
columns: [
|
||||
Reference(columnName: 'a'),
|
||||
Reference(columnName: 'b'),
|
||||
],
|
||||
clause: ForeignKeyClause(
|
||||
foreignTable: TableReference('c'),
|
||||
columnNames: [
|
||||
Reference(columnName: 'd'),
|
||||
Reference(columnName: 'e'),
|
||||
],
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('report errors', () {
|
||||
final result = SqlEngine().parseTableConstraint('parse error');
|
||||
expect(result.errors, isNotEmpty);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue