Don't throw parsing errors for top-level statements

sqlparser/CHANGELOG.md

@@ -6,6 +6,7 @@
 - Breaking: Removed `visitQueryable`. Use `defaultQueryable` instead.
 - Support parsing and analyzing `CREATE VIEW` statements (see `SchemaFromCreateTable.readView`).
   Thanks to [@mqus](https://github.com/mqus) for their contribution!
+- `SqlEngine.parse` will no longer throw when there's a parsing error (use `ParseResult.errors` instead).
 
 ## 0.9.0
 

sqlparser/lib/src/analysis/error.dart

@@ -1,23 +1,31 @@
 part of 'analysis.dart';
 
 class AnalysisError {
-  final AstNode relevantNode;
+  final SyntacticEntity source;
+
   final String message;
   final AnalysisErrorType type;
 
-  AnalysisError({@required this.type, this.message, this.relevantNode});
+  AnalysisError._internal(this.type, this.message, this.source);
+
+  AnalysisError({@required this.type, this.message, AstNode relevantNode})
+      : source = relevantNode;
+
+  @Deprecated('Use source instead')
+  AstNode get relevantNode => source as AstNode;
+
+  factory AnalysisError.fromParser(ParsingError error) {
+    return AnalysisError._internal(
+      AnalysisErrorType.synctactic,
+      error.message,
+      error.token,
+    );
+  }
 
   /// 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 be null.
+  /// nodes don't have a span, in that case this error is going to have a null
-  FileSpan get span {
+  /// span as well.
-    final first = relevantNode?.first?.span;
+  FileSpan get span => source.span;
-    final last = relevantNode?.last?.span;
-
-    if (first != null && last != null) {
-      return first.expand(last);
-    }
-    return null;
-  }
 
   @override
   String toString() {
@@ -54,8 +62,6 @@ enum AnalysisErrorType {
   referencedUnknownTable,
   referencedUnknownColumn,
   ambiguousReference,
-
-  /// Note that most syntax errors are reported as [ParsingError]
   synctactic,
 
   unknownFunction,

sqlparser/lib/src/ast/ast.dart

@@ -37,6 +37,7 @@ part 'statements/create_trigger.dart';
 part 'statements/create_view.dart';
 part 'statements/delete.dart';
 part 'statements/insert.dart';
+part 'statements/invalid.dart';
 part 'statements/select.dart';
 part 'statements/statement.dart';
 part 'statements/update.dart';

sqlparser/lib/src/ast/statements/invalid.dart

@@ -0,0 +1,18 @@
+part of '../ast.dart';
+
+/// Used as a top-level parsing
+class InvalidStatement extends Statement {
+  @override
+  R accept<A, R>(AstVisitor<A, R> visitor, A arg) {
+    return visitor.visitInvalidStatement(this, arg);
+  }
+
+  @override
+  Iterable<AstNode> get childNodes => const Iterable.empty();
+
+  @override
+  bool contentEquals(InvalidStatement other) => true;
+
+  @override
+  void transformChildren<A>(Transformer<A> transformer, A arg) {}
+}

sqlparser/lib/src/ast/visitor.dart

@@ -14,6 +14,7 @@ abstract class AstVisitor<A, R> {
   R visitCreateTriggerStatement(CreateTriggerStatement e, A arg);
   R visitCreateIndexStatement(CreateIndexStatement e, A arg);
   R visitCreateViewStatement(CreateViewStatement e, A arg);
+  R visitInvalidStatement(InvalidStatement e, A arg);
 
   R visitWithClause(WithClause e, A arg);
   R visitUpsertClause(UpsertClause e, A arg);
@@ -96,6 +97,11 @@ abstract class AstVisitor<A, R> {
 class RecursiveVisitor<A, R> implements AstVisitor<A, R> {
   // Statements
 
+  @override
+  R visitInvalidStatement(InvalidStatement e, A arg) {
+    return visitStatement(e, arg);
+  }
+
   @override
   R visitSelectStatement(SelectStatement e, A arg) {
     return visitBaseSelectStatement(e, arg);

sqlparser/lib/src/engine/sql_engine.dart

@@ -110,7 +110,7 @@ class SqlEngine {
     final tokensForParser = tokens.where((t) => !t.invisibleToParser).toList();
     final parser = Parser(tokensForParser, useMoor: options.useMoorExtensions);
 
-    final stmt = parser.statement();
+    final stmt = parser.safeStatement();
     return ParseResult._(stmt, tokens, parser.errors, sql, null);
   }
 
@@ -143,7 +143,14 @@ class SqlEngine {
   /// this statement only.
   AnalysisContext analyze(String sql, {AnalyzeStatementOptions stmtOptions}) {
     final result = parse(sql);
-    return analyzeParsed(result, stmtOptions: stmtOptions);
+    final analyzed = analyzeParsed(result, stmtOptions: stmtOptions);
+
+    // Add parsing errors that occured to the beginning since they are the most
+    // prominent problems.
+    analyzed.errors
+        .insertAll(0, result.errors.map((e) => AnalysisError.fromParser(e)));
+
+    return analyzed;
   }
 
   /// Analyzes a parsed [result] statement. The [AnalysisContext] returned

sqlparser/lib/src/reader/parser/parser.dart

@@ -225,6 +225,12 @@ class Parser extends ParserBase
   // todo remove this and don't be that lazy in moorFile()
   var _lastStmtHadParsingError = false;
 
+  /// Parses a statement without throwing when there's a parsing error.
+  Statement /*?*/ safeStatement() {
+    return _parseAsStatement(statement, requireSemicolon: false) ??
+        InvalidStatement();
+  }
+
   Statement statement() {
     final first = _peek;
     Statement stmt = _crud();
@@ -369,14 +375,15 @@ class Parser extends ParserBase
 
   /// Invokes [parser], sets the appropriate source span and attaches a
   /// semicolon if one exists.
-  T _parseAsStatement<T extends Statement>(T Function() parser) {
+  T _parseAsStatement<T extends Statement>(T Function() parser,
+      {bool requireSemicolon = true}) {
     _lastStmtHadParsingError = false;
     final first = _peek;
     T result;
     try {
       result = parser();
 
-      if (result != null) {
+      if (result != null && requireSemicolon) {
         result.semicolon = _consume(TokenType.semicolon,
             'Expected a semicolon after the statement ended');
         result.setSpan(first, _previous);

sqlparser/test/engine/sql_engine_test.dart

@@ -0,0 +1,30 @@
+import 'package:sqlparser/sqlparser.dart';
+import 'package:test/test.dart';
+
+void main() {
+  test('does not throw when parsing invalid statements', () {
+    final engine = SqlEngine();
+    ParseResult result;
+
+    try {
+      result = engine.parse('UDPATE foo SET bar = foo;');
+    } on ParsingError {
+      fail('Calling engine.parse threw an error');
+    }
+
+    expect(result.errors, isNotEmpty);
+  });
+
+  test('does not throw when analyzing invalid statements', () {
+    final engine = SqlEngine();
+    AnalysisContext result;
+
+    try {
+      result = engine.analyze('UDPATE foo SET bar = foo;');
+    } on ParsingError {
+      fail('Calling engine.parse threw an error');
+    }
+
+    expect(result.errors, isNotEmpty);
+  });
+}

sqlparser/test/parser/create_table_test.dart

@@ -188,16 +188,16 @@ void main() {
   test("can't have empty arguments in CREATE VIRTUAL TABLE", () {
     final engine = SqlEngine();
     expect(
-      () => engine.parse('CREATE VIRTUAL TABLE foo USING bar(a,)'),
+      engine.parse('CREATE VIRTUAL TABLE foo USING bar(a,)').errors,
-      throwsA(
+      contains(
         const TypeMatcher<ParsingError>()
             .having((e) => e.token.lexeme, 'fails at closing bracket', ')'),
       ),
     );
 
     expect(
-      () => engine.parse('CREATE VIRTUAL TABLE foo USING bar(a,,b)'),
+      engine.parse('CREATE VIRTUAL TABLE foo USING bar(a,,b)').errors,
-      throwsA(
+      contains(
         const TypeMatcher<ParsingError>()
             .having((e) => e.token.lexeme, 'fails at next comma', ','),
       ),