From 4b0add64de915cc8d9310f65f3755eb75c5e6b96 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Mon, 26 Aug 2019 22:26:38 +0200 Subject: [PATCH] Provide better error messages at unknown tables --- sqlparser/lib/src/analysis/analysis.dart | 1 + sqlparser/lib/src/analysis/error.dart | 37 +++++++++++++++++-- .../lib/src/analysis/schema/references.dart | 14 +++++-- .../src/analysis/steps/column_resolver.dart | 7 +++- .../lib/src/ast/expressions/reference.dart | 9 +++++ sqlparser/lib/src/reader/parser/crud.dart | 6 ++- 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/sqlparser/lib/src/analysis/analysis.dart b/sqlparser/lib/src/analysis/analysis.dart index d467477f..b52b5ea2 100644 --- a/sqlparser/lib/src/analysis/analysis.dart +++ b/sqlparser/lib/src/analysis/analysis.dart @@ -1,6 +1,7 @@ import 'dart:math'; import 'package:meta/meta.dart'; +import 'package:source_span/source_span.dart'; import 'package:sqlparser/sqlparser.dart'; import 'package:sqlparser/src/reader/tokenizer/token.dart'; diff --git a/sqlparser/lib/src/analysis/error.dart b/sqlparser/lib/src/analysis/error.dart index 6fad03d4..5dd805b1 100644 --- a/sqlparser/lib/src/analysis/error.dart +++ b/sqlparser/lib/src/analysis/error.dart @@ -7,20 +7,49 @@ class AnalysisError { AnalysisError({@required this.type, this.message, this.relevantNode}); - @override - String toString() { + /// 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. + SourceSpan get span { final first = relevantNode?.first?.span; final last = relevantNode?.last?.span; if (first != null && last != null) { - final span = first.expand(last); - return span.message(message ?? type.toString(), color: true); + return first.expand(last); + } + return null; + } + + @override + String toString() { + final msgSpan = span; + if (msgSpan != null) { + return msgSpan.message(message ?? type.toString(), color: true); } else { return 'Error: $type: $message at $relevantNode'; } } } +class UnresolvedReferenceError extends AnalysisError { + /// The attempted reference that couldn't be resolved + final String reference; + + /// A list of alternative references that would be available for [reference]. + final Iterable available; + + UnresolvedReferenceError( + {@required AnalysisErrorType type, + this.reference, + this.available, + AstNode relevantNode}) + : super(type: type, relevantNode: relevantNode); + + @override + String get message { + return 'Could not find $reference. Available are: ${available.join(', ')}'; + } +} + enum AnalysisErrorType { referencedUnknownTable, referencedUnknownColumn, diff --git a/sqlparser/lib/src/analysis/schema/references.dart b/sqlparser/lib/src/analysis/schema/references.dart index 5f8318d8..f6fbc6f4 100644 --- a/sqlparser/lib/src/analysis/schema/references.dart +++ b/sqlparser/lib/src/analysis/schema/references.dart @@ -13,7 +13,7 @@ mixin Referencable {} /// many things, basically only tables. /// /// For instance: "SELECT *, 1 AS d, (SELECT id FROM demo WHERE id = out.id) FROM demo AS out;" -/// is a valid sql query when the demo table as an id column. However, +/// is a valid sql query when the demo table has an id column. However, /// "SELECT *, 1 AS d, (SELECT id FROM demo WHERE id = d) FROM demo AS out;" is /// not, the "d" referencable is not visible for the child select statement. mixin VisibleToChildren on Referencable {} @@ -79,12 +79,20 @@ class ReferenceScope { /// Returns everything that is in scope and a subtype of [T]. List allOf() { var scope = this; + var isInCurrentScope = true; final collected = []; while (scope != null) { - collected.addAll( - scope._references.values.expand((list) => list).whereType()); + var foundValues = + scope._references.values.expand((list) => list).whereType(); + + if (!isInCurrentScope) { + foundValues = foundValues.whereType().cast(); + } + + collected.addAll(foundValues); scope = scope.parent; + isInCurrentScope = false; } return collected; } diff --git a/sqlparser/lib/src/analysis/steps/column_resolver.dart b/sqlparser/lib/src/analysis/steps/column_resolver.dart index 06029e2f..3d38d9c5 100644 --- a/sqlparser/lib/src/analysis/steps/column_resolver.dart +++ b/sqlparser/lib/src/analysis/steps/column_resolver.dart @@ -115,10 +115,13 @@ class ColumnResolver extends RecursiveVisitor { Table _resolveTableReference(TableReference r) { final scope = r.scope; final resolvedTable = scope.resolve(r.tableName, orElse: () { - context.reportError(AnalysisError( + final available = scope.allOf
().map((t) => t.name); + + context.reportError(UnresolvedReferenceError( type: AnalysisErrorType.referencedUnknownTable, relevantNode: r, - message: 'The table ${r.tableName} could not be found', + reference: r.tableName, + available: available, )); }); return r.resolved = resolvedTable; diff --git a/sqlparser/lib/src/ast/expressions/reference.dart b/sqlparser/lib/src/ast/expressions/reference.dart index 4617016f..bba0a6d4 100644 --- a/sqlparser/lib/src/ast/expressions/reference.dart +++ b/sqlparser/lib/src/ast/expressions/reference.dart @@ -23,4 +23,13 @@ class Reference extends Expression with ReferenceOwner { bool contentEquals(Reference other) { return other.tableName == tableName && other.columnName == columnName; } + + @override + String toString() { + if (tableName != null) { + return '$tableName.$columnName'; + } else { + return columnName; + } + } } diff --git a/sqlparser/lib/src/reader/parser/crud.dart b/sqlparser/lib/src/reader/parser/crud.dart index 951ce921..9545eb84 100644 --- a/sqlparser/lib/src/reader/parser/crud.dart +++ b/sqlparser/lib/src/reader/parser/crud.dart @@ -130,9 +130,11 @@ mixin CrudParser on ParserBase { if (_matchOne(TokenType.identifier)) { // ignore the schema name, it's not supported. Besides that, we're on the // first branch in the diagram here - final tableName = (_previous as IdentifierToken).identifier; + final firstToken = _previous as IdentifierToken; + final tableName = firstToken.identifier; final alias = _as(); - return TableReference(tableName, alias?.identifier); + return TableReference(tableName, alias?.identifier) + ..setSpan(firstToken, _previous); } return null; }