From 8eb5d388279c2556a0c381ea4b64954fb0380e84 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Tue, 21 Jan 2025 16:33:26 +0100 Subject: [PATCH] Use common class for AST variables --- .../src/analysis/resolver/file_analysis.dart | 8 ++-- .../resolver/queries/query_analyzer.dart | 2 +- drift_dev/lib/src/analysis/results/query.dart | 4 +- .../lib/src/writer/queries/sql_writer.dart | 2 +- drift_dev/pubspec.yaml | 2 +- sqlparser/CHANGELOG.md | 4 ++ sqlparser/lib/src/analysis/options.dart | 4 +- .../lib/src/analysis/steps/prepare_ast.dart | 6 +-- .../lib/src/ast/expressions/variables.dart | 11 +++-- sqlparser/lib/src/ast/visitor.dart | 4 +- sqlparser/lib/src/reader/parser.dart | 11 ++++- .../lib/src/reader/tokenizer/scanner.dart | 2 +- sqlparser/lib/src/reader/tokenizer/token.dart | 44 +++++++++++++------ sqlparser/lib/src/utils/ast_equality.dart | 6 +-- sqlparser/lib/utils/node_to_text.dart | 5 +-- sqlparser/pubspec.yaml | 2 +- .../test/analysis/types2/resolver_test.dart | 8 ++-- sqlparser/test/parser/drift_file_test.dart | 14 +++--- sqlparser/test/parser/expression_test.dart | 29 +++++++++++- .../test/parser/regression_891_test.dart | 6 +-- sqlparser/test/scanner/single_token_test.dart | 10 +++++ 21 files changed, 126 insertions(+), 58 deletions(-) diff --git a/drift_dev/lib/src/analysis/resolver/file_analysis.dart b/drift_dev/lib/src/analysis/resolver/file_analysis.dart index cd7de3d5b..d6bf60a55 100644 --- a/drift_dev/lib/src/analysis/resolver/file_analysis.dart +++ b/drift_dev/lib/src/analysis/resolver/file_analysis.dart @@ -222,8 +222,8 @@ class FileAnalyzer { final variable = parameter.variable; if (parameter.isRequired) { - if (variable is ColonNamedVariable) { - requiredName.add(variable.name); + if (variable is NamedVariable) { + requiredName.add(variable.fullName); } else if (variable is NumberedVariable) { requiredIndex.add(variable.resolvedIndex!); } @@ -234,8 +234,8 @@ class FileAnalyzer { .resolveColumnType(parameter.typeName) .withNullable(parameter.orNull); - if (variable is ColonNamedVariable) { - namedHints[variable.name] = type; + if (variable is NamedVariable) { + namedHints[variable.fullName] = type; } else if (variable is NumberedVariable) { indexedHints[variable.resolvedIndex!] = type; } diff --git a/drift_dev/lib/src/analysis/resolver/queries/query_analyzer.dart b/drift_dev/lib/src/analysis/resolver/queries/query_analyzer.dart index e7fee6e8a..952771e6f 100644 --- a/drift_dev/lib/src/analysis/resolver/queries/query_analyzer.dart +++ b/drift_dev/lib/src/analysis/resolver/queries/query_analyzer.dart @@ -606,7 +606,7 @@ class QueryAnalyzer { } currentIndex = used.resolvedIndex!; - final name = (used is ColonNamedVariable) ? used.name : null; + final name = (used is NamedVariable) ? used.fullName : null; final explicitIndex = (used is NumberedVariable) ? used.explicitIndex : null; final forCapture = used.meta(); diff --git a/drift_dev/lib/src/analysis/results/query.dart b/drift_dev/lib/src/analysis/results/query.dart index d2bc7ebd9..c68997722 100644 --- a/drift_dev/lib/src/analysis/results/query.dart +++ b/drift_dev/lib/src/analysis/results/query.dart @@ -373,12 +373,12 @@ class CapturedVariable { /// /// This variable is not mounted to the same syntax tree as [reference], it /// will be mounted into the tree returned by [addHelperNodes]. - final ColonNamedVariable introducedVariable; + final NamedVariable introducedVariable; String get helperColumn => '\$n_$queryGlobalId'; CapturedVariable(this.reference, this.queryGlobalId) - : introducedVariable = ColonNamedVariable.synthetic(':r$queryGlobalId') { + : introducedVariable = NamedVariable.synthetic(':', 'r$queryGlobalId') { introducedVariable.setMeta(this); } } diff --git a/drift_dev/lib/src/writer/queries/sql_writer.dart b/drift_dev/lib/src/writer/queries/sql_writer.dart index a9892a1cf..c1dbc310c 100644 --- a/drift_dev/lib/src/writer/queries/sql_writer.dart +++ b/drift_dev/lib/src/writer/queries/sql_writer.dart @@ -173,7 +173,7 @@ class SqlWriter extends NodeSqlBuilder { } @override - void visitNamedVariable(ColonNamedVariable e, void arg) { + void visitNamedVariable(NamedVariable e, void arg) { final found = _findVariable(e); if (found != null) { _writeAnalyzedVariable(found); diff --git a/drift_dev/pubspec.yaml b/drift_dev/pubspec.yaml index faf5380c6..8ddecaf52 100644 --- a/drift_dev/pubspec.yaml +++ b/drift_dev/pubspec.yaml @@ -36,7 +36,7 @@ dependencies: # Drift-specific analysis and apis drift: ">=2.23.0 <2.24.0" sqlite3: ^2.4.6 - sqlparser: ^0.40.0 + sqlparser: ^0.41.0 # Dart analysis analyzer: ^6.4.1 diff --git a/sqlparser/CHANGELOG.md b/sqlparser/CHANGELOG.md index fc9248ed2..6b01fd72f 100644 --- a/sqlparser/CHANGELOG.md +++ b/sqlparser/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.41.0 + +- Replace `ColonNamedVariable` with `NamedVariable` for all named variables. + ## 0.40.0 - Add support for the `dbstat` module. diff --git a/sqlparser/lib/src/analysis/options.dart b/sqlparser/lib/src/analysis/options.dart index 221323545..9fa8d9776 100644 --- a/sqlparser/lib/src/analysis/options.dart +++ b/sqlparser/lib/src/analysis/options.dart @@ -30,8 +30,8 @@ class AnalyzeStatementOptions { final index = variable.resolvedIndex; if (index != null && indexedVariableTypes.containsKey(index)) { return indexedVariableTypes[index]; - } else if (variable is ColonNamedVariable) { - return namedVariableTypes[variable.name]; + } else if (variable is NamedVariable) { + return namedVariableTypes[variable.fullName]; } return null; } diff --git a/sqlparser/lib/src/analysis/steps/prepare_ast.dart b/sqlparser/lib/src/analysis/steps/prepare_ast.dart index 1c984721c..85f991c23 100644 --- a/sqlparser/lib/src/analysis/steps/prepare_ast.dart +++ b/sqlparser/lib/src/analysis/steps/prepare_ast.dart @@ -151,7 +151,7 @@ class AstPreparingVisitor extends RecursiveVisitor { } @override - void visitNamedVariable(ColonNamedVariable e, void arg) { + void visitNamedVariable(NamedVariable e, void arg) { _foundVariables.add(e); visitChildren(e, arg); } @@ -177,11 +177,11 @@ class AstPreparingVisitor extends RecursiveVisitor { variable.resolvedIndex = largestAssigned + 1; largestAssigned++; } - } else if (variable is ColonNamedVariable) { + } else if (variable is NamedVariable) { // named variables behave just like numbered vars without an explicit // index, but of course two variables with the same name must have the // same index. - final index = resolvedNames.putIfAbsent(variable.name, () { + final index = resolvedNames.putIfAbsent(variable.fullName, () { return ++largestAssigned; }); variable.resolvedIndex = index; diff --git a/sqlparser/lib/src/ast/expressions/variables.dart b/sqlparser/lib/src/ast/expressions/variables.dart index 876fff812..2cc91ec2d 100644 --- a/sqlparser/lib/src/ast/expressions/variables.dart +++ b/sqlparser/lib/src/ast/expressions/variables.dart @@ -27,15 +27,20 @@ class NumberedVariable extends Expression implements Variable { Iterable get childNodes => const []; } -class ColonNamedVariable extends Expression implements Variable { +class NamedVariable extends Expression implements Variable { final String name; + /// The [name] plus a variable prefix (e.g. `:user`) + final String fullName; + @override int? resolvedIndex; - ColonNamedVariable.synthetic(this.name); + NamedVariable.synthetic(String prefix, this.name) : fullName = '$prefix$name'; - ColonNamedVariable(ColonVariableToken token) : name = token.name; + NamedVariable(NamedVariableToken token) + : fullName = token.fullName, + name = token.name; @override R accept(AstVisitor visitor, A arg) { diff --git a/sqlparser/lib/src/ast/visitor.dart b/sqlparser/lib/src/ast/visitor.dart index ae7922bc3..ca2bc7a17 100644 --- a/sqlparser/lib/src/ast/visitor.dart +++ b/sqlparser/lib/src/ast/visitor.dart @@ -90,7 +90,7 @@ abstract class AstVisitor { R visitIndexedColumn(IndexedColumn e, A arg); R visitNumberedVariable(NumberedVariable e, A arg); - R visitNamedVariable(ColonNamedVariable e, A arg); + R visitNamedVariable(NamedVariable e, A arg); R visitBlock(Block block, A arg); R visitSemicolonSeparatedStatements(SemicolonSeparatedStatements e, A arg); @@ -564,7 +564,7 @@ class RecursiveVisitor implements AstVisitor { } @override - R? visitNamedVariable(ColonNamedVariable e, A arg) { + R? visitNamedVariable(NamedVariable e, A arg) { return visitVariable(e, arg); } diff --git a/sqlparser/lib/src/reader/parser.dart b/sqlparser/lib/src/reader/parser.dart index 24d972cf4..bce8f03b4 100644 --- a/sqlparser/lib/src/reader/parser.dart +++ b/sqlparser/lib/src/reader/parser.dart @@ -974,9 +974,18 @@ class Parser { ..token = token ..setSpan(token, token); } else if (_matchOne(TokenType.colonVariable)) { - return ColonNamedVariable(_previous as ColonVariableToken) + return NamedVariable(_previous as ColonVariableToken) ..setSpan(_previous, _previous); + } else if (!enableDriftExtensions) { + // These have special meanings in drift files, but are general variables + // otherwise. + if (_match( + const [TokenType.atSignVariable, TokenType.dollarSignVariable])) { + return NamedVariable(_previous as NamedVariableToken) + ..setSpan(_previous, _previous); + } } + return null; } diff --git a/sqlparser/lib/src/reader/tokenizer/scanner.dart b/sqlparser/lib/src/reader/tokenizer/scanner.dart index f20bce9b4..69e683a58 100644 --- a/sqlparser/lib/src/reader/tokenizer/scanner.dart +++ b/sqlparser/lib/src/reader/tokenizer/scanner.dart @@ -171,7 +171,7 @@ class Scanner { if (name == null) { _addToken(TokenType.colon); } else { - tokens.add(ColonVariableToken(_currentSpan, ':$name')); + tokens.add(ColonVariableToken(_currentSpan, name)); } break; case $dollar: diff --git a/sqlparser/lib/src/reader/tokenizer/token.dart b/sqlparser/lib/src/reader/tokenizer/token.dart index ce9f63985..cdf4e19a7 100644 --- a/sqlparser/lib/src/reader/tokenizer/token.dart +++ b/sqlparser/lib/src/reader/tokenizer/token.dart @@ -517,11 +517,24 @@ class IdentifierToken extends Token { : super(TokenType.identifier, span); } -abstract class VariableToken extends Token { +sealed class VariableToken extends Token { VariableToken(super.type, super.span); } -class QuestionMarkVariableToken extends Token { +sealed class NamedVariableToken extends VariableToken { + /// The name of this variable, not including the [prefix]. + final String name; + + /// The prefix introducing this variable, specific to the token type. E.g. + /// ":" for [ColonVariableToken]. + String get prefix; + + String get fullName => prefix + name; + + NamedVariableToken(this.name, super.type, super.span); +} + +class QuestionMarkVariableToken extends VariableToken { /// The explicit index, if this variable was of the form `?123`. Otherwise /// null. final int? explicitIndex; @@ -530,25 +543,28 @@ class QuestionMarkVariableToken extends Token { : super(TokenType.questionMarkVariable, span); } -class ColonVariableToken extends Token { - final String name; +class ColonVariableToken extends NamedVariableToken { + @override + String get prefix => ':'; - ColonVariableToken(FileSpan span, this.name) - : super(TokenType.colonVariable, span); + ColonVariableToken(FileSpan span, String name) + : super(name, TokenType.colonVariable, span); } -class DollarSignVariableToken extends Token { - final String name; +class DollarSignVariableToken extends NamedVariableToken { + @override + String get prefix => r'$'; - DollarSignVariableToken(FileSpan span, this.name) - : super(TokenType.dollarSignVariable, span); + DollarSignVariableToken(FileSpan span, String name) + : super(name, TokenType.dollarSignVariable, span); } -class AtSignVariableToken extends Token { - final String name; +class AtSignVariableToken extends NamedVariableToken { + @override + String get prefix => '@'; - AtSignVariableToken(FileSpan span, this.name) - : super(TokenType.atSignVariable, span); + AtSignVariableToken(FileSpan span, String name) + : super(name, TokenType.atSignVariable, span); } /// Inline Dart appearing in a create table statement. Only parsed when the diff --git a/sqlparser/lib/src/utils/ast_equality.dart b/sqlparser/lib/src/utils/ast_equality.dart index 38463909e..8a89c065b 100644 --- a/sqlparser/lib/src/utils/ast_equality.dart +++ b/sqlparser/lib/src/utils/ast_equality.dart @@ -506,9 +506,9 @@ class EqualityEnforcingVisitor implements AstVisitor { } @override - void visitNamedVariable(ColonNamedVariable e, void arg) { - final current = _currentAs(e); - _assert(current.name == e.name, e); + void visitNamedVariable(NamedVariable e, void arg) { + final current = _currentAs(e); + _assert(current.fullName == e.fullName, e); _checkChildren(e); } diff --git a/sqlparser/lib/utils/node_to_text.dart b/sqlparser/lib/utils/node_to_text.dart index cef500197..3177e6d2c 100644 --- a/sqlparser/lib/utils/node_to_text.dart +++ b/sqlparser/lib/utils/node_to_text.dart @@ -906,9 +906,8 @@ class NodeSqlBuilder extends AstVisitor { } @override - void visitNamedVariable(ColonNamedVariable e, void arg) { - // Note: The name already starts with the colon - symbol(e.name, spaceBefore: true, spaceAfter: true); + void visitNamedVariable(NamedVariable e, void arg) { + symbol(e.fullName, spaceBefore: true, spaceAfter: true); } @override diff --git a/sqlparser/pubspec.yaml b/sqlparser/pubspec.yaml index fb3386a62..b3b64b07a 100644 --- a/sqlparser/pubspec.yaml +++ b/sqlparser/pubspec.yaml @@ -1,6 +1,6 @@ name: sqlparser description: Parses sqlite statements and performs static analysis on them -version: 0.40.0 +version: 0.41.0 homepage: https://github.com/simolus3/drift/tree/develop/sqlparser repository: https://github.com/simolus3/drift #homepage: https://drift.simonbinder.eu/ diff --git a/sqlparser/test/analysis/types2/resolver_test.dart b/sqlparser/test/analysis/types2/resolver_test.dart index 3b2b27432..3f7524b5e 100644 --- a/sqlparser/test/analysis/types2/resolver_test.dart +++ b/sqlparser/test/analysis/types2/resolver_test.dart @@ -367,10 +367,10 @@ WITH RECURSIVE ).session; Variable? start, end; - for (final variable in session.context.root.allDescendants - .whereType()) { - if (variable.name == ':start') start = variable; - if (variable.name == ':end') end = variable; + for (final variable + in session.context.root.allDescendants.whereType()) { + if (variable.name == 'start') start = variable; + if (variable.name == 'end') end = variable; } assert(start != null && end != null); diff --git a/sqlparser/test/parser/drift_file_test.dart b/sqlparser/test/parser/drift_file_test.dart index 397eceb89..d9f56be4c 100644 --- a/sqlparser/test/parser/drift_file_test.dart +++ b/sqlparser/test/parser/drift_file_test.dart @@ -81,8 +81,8 @@ void main() { SelectStatement( columns: [ ExpressionResultColumn( - expression: ColonNamedVariable( - ColonVariableToken(fakeSpan(':foo'), ':foo'), + expression: NamedVariable( + ColonVariableToken(fakeSpan(':foo'), 'foo'), ), ), ], @@ -90,8 +90,8 @@ void main() { ), parameters: [ VariableTypeHint( - ColonNamedVariable( - ColonVariableToken(fakeSpan(':foo'), ':foo'), + NamedVariable( + ColonVariableToken(fakeSpan(':foo'), 'foo'), ), 'TEXT', orNull: true, @@ -205,7 +205,7 @@ SELECT DISTINCT A.* FROM works A, works B ON A.id = }); test('parses REQUIRED without type hint', () { - final variable = ColonVariableToken(fakeSpan(':category'), ':category'); + final variable = ColonVariableToken(fakeSpan(':category'), 'category'); testDriftFile( 'test(REQUIRED :category): SELECT :category;', DriftFile([ @@ -213,12 +213,12 @@ SELECT DISTINCT A.* FROM works A, works B ON A.id = SimpleName('test'), SelectStatement(columns: [ ExpressionResultColumn( - expression: ColonNamedVariable(variable), + expression: NamedVariable(variable), ), ]), parameters: [ VariableTypeHint( - ColonNamedVariable(variable), + NamedVariable(variable), null, isRequired: true, ), diff --git a/sqlparser/test/parser/expression_test.dart b/sqlparser/test/parser/expression_test.dart index 66e80aafb..928f14265 100644 --- a/sqlparser/test/parser/expression_test.dart +++ b/sqlparser/test/parser/expression_test.dart @@ -1,4 +1,4 @@ -import 'package:sqlparser/src/ast/ast.dart'; +import 'package:sqlparser/sqlparser.dart'; import 'package:sqlparser/src/reader/parser.dart'; import 'package:sqlparser/src/reader/tokenizer/scanner.dart'; import 'package:sqlparser/src/utils/ast_equality.dart'; @@ -48,7 +48,7 @@ final Map _testCases = { NumberedVariable(2), ), token(TokenType.doubleEqual), - ColonNamedVariable(ColonVariableToken(fakeSpan(':test'), ':test')), + NamedVariable(ColonVariableToken(fakeSpan(':test'), 'test')), ), 'CASE x WHEN a THEN b WHEN c THEN d ELSE e END': CaseExpression( base: Reference(columnName: 'x'), @@ -238,4 +238,29 @@ void main() { }); }); }); + + group('named variables', () { + test('support all kinds when not in drift mode', () { + for (final prefix in [':', '@', r'$']) { + final result = SqlEngine().parse('SELECT ${prefix}name;'); + final value = ((result.rootNode as SelectStatement).columns.single + as ExpressionResultColumn) + .expression as NamedVariable; + + expect(value.name, 'name'); + expect(value.fullName, '${prefix}name'); + } + }); + + test('does not parse at and dollar signs as variables in drift mode', () { + final engine = SqlEngine(EngineOptions(driftOptions: DriftSqlOptions())); + expect(engine.parse('SELECT @name').rootNode, isA()); + + final result = engine.parse(r'SELECT $name;'); + final value = ((result.rootNode as SelectStatement).columns.single + as ExpressionResultColumn) + .expression; + expect(value, isA()); + }); + }); } diff --git a/sqlparser/test/parser/regression_891_test.dart b/sqlparser/test/parser/regression_891_test.dart index 810c21974..7aa21d998 100644 --- a/sqlparser/test/parser/regression_891_test.dart +++ b/sqlparser/test/parser/regression_891_test.dart @@ -4,7 +4,7 @@ import 'package:test/test.dart'; import 'utils.dart'; ColonVariableToken _colon(String name) { - return ColonVariableToken(fakeSpan(name), name); + return ColonVariableToken(fakeSpan(':$name'), name); } void main() { @@ -14,7 +14,7 @@ void main() { when: BinaryExpression( NumericLiteral(1), token(TokenType.equal), - ColonNamedVariable(_colon(':isReviewFolderSelected')), + NamedVariable(_colon('isReviewFolderSelected')), ), then: IsExpression( true, @@ -33,7 +33,7 @@ void main() { final folderExpr = BinaryExpression( Reference(entityName: 'n', columnName: 'folderId'), token(TokenType.equal), - ColonNamedVariable(_colon(':selectedFolderId')), + NamedVariable(_colon('selectedFolderId')), ); test('repro 1', () { diff --git a/sqlparser/test/scanner/single_token_test.dart b/sqlparser/test/scanner/single_token_test.dart index 9e3941ab6..6b9133f50 100644 --- a/sqlparser/test/scanner/single_token_test.dart +++ b/sqlparser/test/scanner/single_token_test.dart @@ -195,4 +195,14 @@ void main() { ); }); }); + + test('named variables', () { + for (final prefix in [':', '@', r'$']) { + final scanner = Scanner('${prefix}name')..scanTokens(); + final token = scanner.tokens.first as NamedVariableToken; + + expect(token.name, 'name'); + expect(token.fullName, '${prefix}name'); + } + }); }