diff --git a/sqlparser/lib/src/analysis/error.dart b/sqlparser/lib/src/analysis/error.dart index 59fd0114..b149a098 100644 --- a/sqlparser/lib/src/analysis/error.dart +++ b/sqlparser/lib/src/analysis/error.dart @@ -26,7 +26,7 @@ class AnalysisError { /// The relevant portion of the source code that caused this error. Some AST /// nodes don't have a span, in that case this error is going to have a null /// span as well. - FileSpan? get span => source!.span; + FileSpan? get span => source?.span; @override String toString() { @@ -75,5 +75,9 @@ enum AnalysisErrorType { notSupportedInDesiredVersion, illegalUseOfReturning, raiseMisuse, + nullableColumnInStrictPrimaryKey, + missingPrimaryKey, + noTypeNameInStrictTable, + invalidTypeNameInStrictTable, other, } diff --git a/sqlparser/lib/src/analysis/schema/from_create_table.dart b/sqlparser/lib/src/analysis/schema/from_create_table.dart index ee613ae9..45a6fd04 100644 --- a/sqlparser/lib/src/analysis/schema/from_create_table.dart +++ b/sqlparser/lib/src/analysis/schema/from_create_table.dart @@ -126,6 +126,20 @@ class SchemaFromCreateTable { return const ResolvedType(type: BasicType.real); } + bool isValidTypeNameForStrictTable(String typeName) { + // See https://www.sqlite.org/draft/stricttables.html + const allowed = {'INT', 'INTEGER', 'REAL', 'TEXT', 'BLOB', 'ANY'}; + const alsoAllowedInMoor = {'ENUM', 'BOOL', 'DATE'}; + + if (allowed.contains(typeName.toUpperCase()) || + (moorExtensions && + alsoAllowedInMoor.contains(typeName.toUpperCase()))) { + return true; + } + + return false; + } + /// Looks up the correct column affinity for a declared type name with the /// rules described here: /// https://www.sqlite.org/datatype3.html#determination_of_column_affinity diff --git a/sqlparser/lib/src/analysis/steps/linting_visitor.dart b/sqlparser/lib/src/analysis/steps/linting_visitor.dart index 0185bba7..54b7545f 100644 --- a/sqlparser/lib/src/analysis/steps/linting_visitor.dart +++ b/sqlparser/lib/src/analysis/steps/linting_visitor.dart @@ -26,7 +26,24 @@ class LintingVisitor extends RecursiveVisitor { @override void visitCreateTableStatement(CreateTableStatement e, void arg) { + final schemaReader = + SchemaFromCreateTable(moorExtensions: options.useMoorExtensions); var hasPrimaryKeyDeclaration = false; + var isStrict = false; + + if (e.isStrict) { + if (options.version < SqliteVersion.v3_37) { + context.reportError(AnalysisError( + type: AnalysisErrorType.notSupportedInDesiredVersion, + message: 'STRICT tables are only supported from sqlite3 version 37', + relevantNode: e.strict ?? e, + )); + } else { + // only report warnings related to STRICT tables if strict tables are + // supported. + isStrict = true; + } + } // Ensure that a table declaration only has one PRIMARY KEY constraint void handlePrimaryKeyNode(AstNode node) { @@ -41,9 +58,40 @@ class LintingVisitor extends RecursiveVisitor { } for (final column in e.columns) { + if (isStrict) { + final typeName = column.typeName; + + if (typeName == null) { + // Columns in strict tables must have a type name, even if it's + // `ANY`. + context.reportError(AnalysisError( + type: AnalysisErrorType.noTypeNameInStrictTable, + message: 'In `STRICT` tables, columns must have a type name!', + relevantNode: column.nameToken ?? column, + )); + } else if (!schemaReader.isValidTypeNameForStrictTable(typeName)) { + context.reportError(AnalysisError( + type: AnalysisErrorType.invalidTypeNameInStrictTable, + message: 'Invalid type name for a `STRICT` table.', + relevantNode: column.typeNames?.toSingleEntity ?? column, + )); + } + } + for (final constraint in column.constraints) { if (constraint is PrimaryKeyColumn) { handlePrimaryKeyNode(constraint); + + // A primary key in a STRICT table must be annoted with "NOT NULL" + if (isStrict && !column.isNonNullable) { + context.reportError(AnalysisError( + type: AnalysisErrorType.nullableColumnInStrictPrimaryKey, + message: + 'The column is used as a `PRIMARY KEY` in a `STRICT` table, ' + 'which means that is must be marked as `NOT NULL`', + relevantNode: constraint, + )); + } } } } @@ -51,9 +99,40 @@ class LintingVisitor extends RecursiveVisitor { for (final constraint in e.tableConstraints) { if (constraint is KeyClause && constraint.isPrimaryKey) { handlePrimaryKeyNode(constraint); + + if (isStrict) { + for (final columnName in constraint.columns) { + final expr = columnName.expression; + if (expr is! Reference) continue; + + final column = e.columns.firstWhereOrNull((c) => + c.columnName.toLowerCase() == expr.columnName.toLowerCase()); + if (column != null && !column.isNonNullable) { + context.reportError( + AnalysisError( + type: AnalysisErrorType.nullableColumnInStrictPrimaryKey, + message: + 'This column must be marked as `NOT NULL` to be used in ' + 'a `PRIMARY KEY` clause of a `STRICT` table.', + relevantNode: columnName, + ), + ); + } + } + } } } + if (e.withoutRowId && !hasPrimaryKeyDeclaration) { + context.reportError( + AnalysisError( + type: AnalysisErrorType.missingPrimaryKey, + message: 'Missing PRIMARY KEY declaration for a table without rowid.', + relevantNode: e.tableNameToken ?? e, + ), + ); + } + visitChildren(e, arg); } diff --git a/sqlparser/lib/src/ast/schema/column_definition.dart b/sqlparser/lib/src/ast/schema/column_definition.dart index 4b54d72b..9ce47a68 100644 --- a/sqlparser/lib/src/ast/schema/column_definition.dart +++ b/sqlparser/lib/src/ast/schema/column_definition.dart @@ -28,6 +28,8 @@ class ColumnDefinition extends AstNode { @override Iterable get childNodes => constraints; + bool get isNonNullable => findConstraint() != null; + /// Finds a constraint of type [T], or null, if none is set. T? findConstraint() { final typedConstraints = constraints.whereType().iterator; diff --git a/sqlparser/lib/src/ast/statements/create_table.dart b/sqlparser/lib/src/ast/statements/create_table.dart index af90e3c4..d97c2a72 100644 --- a/sqlparser/lib/src/ast/statements/create_table.dart +++ b/sqlparser/lib/src/ast/statements/create_table.dart @@ -40,6 +40,7 @@ class CreateTableStatement extends TableInducingStatement { Token? openingBracket; Token? closingBracket; + Token? strict; CreateTableStatement({ bool ifNotExists = false, diff --git a/sqlparser/lib/src/reader/parser.dart b/sqlparser/lib/src/reader/parser.dart index 5b21c73b..4cd958d1 100644 --- a/sqlparser/lib/src/reader/parser.dart +++ b/sqlparser/lib/src/reader/parser.dart @@ -1886,12 +1886,14 @@ class Parser { var withoutRowId = false; var isStrict = false; + Token? strict; // Parses a `WITHOUT ROWID` or a `STRICT` keyword. Returns if either such // option has been parsed. bool tableOptions() { if (_matchOne(TokenType.strict)) { isStrict = true; + strict = _previous; return true; } else if (_matchOne(TokenType.without)) { _consume(TokenType.rowid, @@ -1936,7 +1938,8 @@ class Parser { ..setSpan(first, _previous) ..openingBracket = leftParen ..tableNameToken = tableIdentifier - ..closingBracket = rightParen; + ..closingBracket = rightParen + ..strict = strict; } /// Parses a `CREATE VIRTUAL TABLE` statement, after the `CREATE VIRTUAL TABLE diff --git a/sqlparser/lib/src/reader/syntactic_entity.dart b/sqlparser/lib/src/reader/syntactic_entity.dart index 27be6207..55d89c50 100644 --- a/sqlparser/lib/src/reader/syntactic_entity.dart +++ b/sqlparser/lib/src/reader/syntactic_entity.dart @@ -53,4 +53,28 @@ extension UnionEntityExtension on Iterable { FileSpan? get spanOrNull { return isEmpty ? null : span; } + + /// Returns a single [SyntacticEntity] representing this range of entities. + SyntacticEntity get toSingleEntity => _UnionSyntacticEntity(toList()); +} + +class _UnionSyntacticEntity extends SyntacticEntity { + final List _members; + + _UnionSyntacticEntity(this._members); + + @override + int get firstPosition => _members.first.firstPosition; + + @override + bool get hasSpan => _members.any((entity) => entity.hasSpan); + + @override + int get lastPosition => _members.last.lastPosition; + + @override + FileSpan? get span => _members.spanOrNull; + + @override + bool get synthetic => _members.every((entity) => entity.synthetic); } diff --git a/sqlparser/test/analysis/errors/create_table_errors_test.dart b/sqlparser/test/analysis/errors/create_table_errors_test.dart new file mode 100644 index 00000000..11cc8187 --- /dev/null +++ b/sqlparser/test/analysis/errors/create_table_errors_test.dart @@ -0,0 +1,112 @@ +import 'package:sqlparser/sqlparser.dart'; +import 'package:test/test.dart'; + +import 'utils.dart'; + +void main() { + final oldEngine = SqlEngine(EngineOptions(version: SqliteVersion.v3_35)); + final engine = SqlEngine(EngineOptions(version: SqliteVersion.v3_37)); + + group('using STRICT', () { + test('with an old sqlite3 version', () { + const stmt = 'CREATE TABLE a (c INTEGER) STRICT'; + + oldEngine.analyze(stmt).expectError('STRICT', + type: AnalysisErrorType.notSupportedInDesiredVersion); + engine.analyze(stmt).expectNoError(); + }); + + test( + 'with a nullable primary key column', + () { + const stmt = 'CREATE TABLE a (c INTEGER PRIMARY KEY) STRICT;'; + + engine.analyze(stmt).expectError('PRIMARY KEY', + type: AnalysisErrorType.nullableColumnInStrictPrimaryKey); + }, + ); + + test( + 'with a nullable primary key table constraint', + () { + final errors = engine.analyze( + ''' + CREATE TABLE a ( + c INTEGER, + c2 INTEGER NOT NULL, + c3 INTEGER, + PRIMARY KEY (c, c2, c3) + ) STRICT;''', + ).errors; + + expect( + errors, + containsAll([ + analysisErrorWith( + lexeme: 'c', + type: AnalysisErrorType.nullableColumnInStrictPrimaryKey), + analysisErrorWith( + lexeme: 'c3', + type: AnalysisErrorType.nullableColumnInStrictPrimaryKey), + ]), + ); + }, + ); + + test( + 'without a column type', + () { + engine + .analyze('CREATE TABLE a (c) STRICT;') + .expectError('c', type: AnalysisErrorType.noTypeNameInStrictTable); + }, + ); + + test( + 'with an invalid column type', + () { + engine.analyze('CREATE TABLE a (c INTEGER(12)) STRICT;').expectError( + 'INTEGER(12)', + type: AnalysisErrorType.invalidTypeNameInStrictTable, + ); + }, + ); + }); + + test('using WITHOUT ROWID and then not declaring a primary key', () { + engine + .analyze('CREATE TABLE a (c INTEGER) WITHOUT ROWID') + .expectError('a', type: AnalysisErrorType.missingPrimaryKey); + + engine.analyze('CREATE TABLE a (c INTEGER);').expectNoError(); + engine + .analyze('CREATE TABLE a (c INTEGER PRIMARY KEY) WITHOUT ROWID;') + .expectNoError(); + + final errors = + engine.analyze('CREATE TABLE a (c INTEGER, PRIMARY KEY (c));'); + expect( + errors, + isNot(contains( + analysisErrorWith(type: AnalysisErrorType.missingPrimaryKey)))); + }); + + test('multiple primary key constraints', () { + engine + .analyze( + 'CREATE TABLE a (c INTEGER PRIMARY KEY, c2 INTEGER PRIMARY KEY)') + .expectError('PRIMARY KEY', + type: AnalysisErrorType.duplicatePrimaryKeyDeclaration); + + final errors = engine + .analyze('CREATE TABLE a (c INTEGER PRIMARY KEY, c2, PRIMARY KEY (c2))') + .errors; + + expect( + errors, + contains(analysisErrorWith( + lexeme: 'PRIMARY KEY (c2)', + type: AnalysisErrorType.duplicatePrimaryKeyDeclaration)), + ); + }); +} diff --git a/sqlparser/test/analysis/errors/row_value_misuse_test.dart b/sqlparser/test/analysis/errors/row_value_misuse_test.dart index 86a519eb..ff8febe0 100644 --- a/sqlparser/test/analysis/errors/row_value_misuse_test.dart +++ b/sqlparser/test/analysis/errors/row_value_misuse_test.dart @@ -1,6 +1,8 @@ import 'package:sqlparser/sqlparser.dart'; import 'package:test/test.dart'; +import 'utils.dart'; + void main() { late SqlEngine engine; setUp(() { @@ -8,33 +10,39 @@ void main() { }); test('when using row value in select', () { - engine.analyze('SELECT (1, 2, 3)').expectError('(1, 2, 3)'); + engine + .analyze('SELECT (1, 2, 3)') + .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse); }); test('as left hand operator of in', () { - engine.analyze('SELECT (1, 2, 3) IN (4, 5, 6)').expectError('(1, 2, 3)'); + engine + .analyze('SELECT (1, 2, 3) IN (4, 5, 6)') + .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse); }); test('in BETWEEN expression', () { - engine.analyze('SELECT 1 BETWEEN (1, 2, 3) AND 3').expectError('(1, 2, 3)'); + engine + .analyze('SELECT 1 BETWEEN (1, 2, 3) AND 3') + .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse); }); test('in CASE - value', () { engine .analyze('SELECT CASE 1 WHEN 1 THEN (1, 2, 3) ELSE 1 END') - .expectError('(1, 2, 3)'); + .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse); }); test('in CASE - when', () { engine .analyze('SELECT CASE 1 WHEN (1, 2, 3) THEN 1 ELSE 1 END') - .expectError('(1, 2, 3)'); + .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse); }); test('in CASE - base', () { engine .analyze('SELECT CASE (1, 2, 3) WHEN 1 THEN 1 ELSE 1 END') - .expectError('(1, 2, 3)'); + .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse); }); group('does not generate error for valid usage', () { @@ -61,20 +69,3 @@ void main() { }); }); } - -extension on AnalysisContext { - void expectError(String lexeme) { - expect( - errors, - [ - isA() - .having((e) => e.type, 'type', AnalysisErrorType.rowValueMisuse) - .having((e) => e.span!.text, 'span.text', lexeme), - ], - ); - } - - void expectNoError() { - expect(errors, isEmpty); - } -} diff --git a/sqlparser/test/analysis/errors/utils.dart b/sqlparser/test/analysis/errors/utils.dart new file mode 100644 index 00000000..f2bc4263 --- /dev/null +++ b/sqlparser/test/analysis/errors/utils.dart @@ -0,0 +1,28 @@ +import 'package:sqlparser/sqlparser.dart'; +import 'package:test/test.dart'; + +extension ExpectErrors on AnalysisContext { + void expectError(String lexeme, {AnalysisErrorType? type}) { + expect( + errors, + [analysisErrorWith(lexeme: lexeme, type: type)], + ); + } + + void expectNoError() { + expect(errors, isEmpty); + } +} + +Matcher analysisErrorWith({String? lexeme, AnalysisErrorType? type}) { + var matcher = isA(); + + if (lexeme != null) { + matcher = matcher.having((e) => e.span?.text, 'span.text', lexeme); + } + if (type != null) { + matcher = matcher.having((e) => e.type, 'type', type); + } + + return matcher; +}