Change behavior to not override analyzer classes where not necessary.
Work required on the analyzer side to remove `AngularErrorVerifier` and
`AngularResolver`, but this now decouples most of the rest of the behavior
so that removing the former should be simple. Harder will be removing
pipes so that `AngularResolver` can be removed, but that's in the works
too.
diff --git a/angular_analyzer_plugin/lib/src/resolver.dart b/angular_analyzer_plugin/lib/src/resolver.dart
index aa0bac1..3d05268 100644
--- a/angular_analyzer_plugin/lib/src/resolver.dart
+++ b/angular_analyzer_plugin/lib/src/resolver.dart
@@ -33,80 +33,29 @@
return parent is ElementInfo && parent.tagMatchedAsCustomTag;
}
-/// Override the standard [ErrorVerifier] class to report unacceptable nodes,
-/// while suppressing secondary errors that would have been raised by
-/// [ErrorVerifier] if we let it see the bogus definitions.
-class AngularErrorVerifier extends _IntermediateErrorVerifier
- with ReportUnacceptableNodesMixin {
- final bool acceptAssignment;
+/// Overrides standard [ErrorVerifier] to prevent issues with analyzing dangling
+/// angular nodes. Not intended as a long-term solution.
+class AngularErrorVerifier extends ErrorVerifier {
+ AngularErrorVerifier(
+ ErrorReporter errorReporter,
+ LibraryElement currentLibrary,
+ TypeProvider typeProvider,
+ InheritanceManager2 inheritanceManager,
+ {@required bool enableSuperMixins})
+ : super(errorReporter, currentLibrary, typeProvider, inheritanceManager,
+ enableSuperMixins);
@override
- ErrorReporter errorReporter;
- @override
- TypeProvider typeProvider;
-
- AngularErrorVerifier(ErrorReporter errorReporter, LibraryElement library,
- TypeProvider typeProvider, InheritanceManager2 inheritanceManager2,
- {@required this.acceptAssignment})
- : errorReporter = errorReporter,
- typeProvider = typeProvider,
- super(errorReporter, library, typeProvider, inheritanceManager2);
-
- @override
- void visitAssignmentExpression(AssignmentExpression exp) {
- final variableElement = ErrorVerifier.getVariableElement(exp.leftHandSide);
- if ((variableElement == null ||
- variableElement is PropertyInducingElement) &&
- acceptAssignment) {
- super.visitAssignmentExpression(exp);
- } else {
- exp.visitChildren(this);
- }
+ void visitFunctionExpression(FunctionExpression func) {
+ // Stop resolving or analyzer will crash.
+ // TODO(mfairhurst): fix the analyzer crash and remove this.
}
-
- @override
- void visitFunctionExpression(FunctionExpression exp) {
- // error reported in [AngularResolverVisitor] but [ErrorVerifier] crashes
- // because it isn't resolved
- }
-
- @override
- void visitInstanceCreationExpression(InstanceCreationExpression exp) =>
- _reportUnacceptableNode(exp, "Usage of new");
-
- @override
- void visitListLiteral(ListLiteral list) {
- if (list.typeArguments != null) {
- _reportUnacceptableNode(list, "Typed list literals");
- } else {
- super.visitListLiteral(list);
- }
- }
-
- @override
- void visitMapLiteral(MapLiteral map) {
- if (map.typeArguments != null) {
- _reportUnacceptableNode(map, "Typed map literals");
- } else {
- super.visitMapLiteral(map);
- }
- }
-
- @override
- void visitRethrowExpression(RethrowExpression exp) =>
- _reportUnacceptableNode(exp, "Rethrow");
-
- @override
- void visitThisExpression(ThisExpression exp) =>
- _reportUnacceptableNode(exp, "This references");
}
-/// Override the standard [ResolverVisitor] class to report unacceptable nodes,
-/// while suppressing secondary errors that would have been raised by
-/// [ResolverVisitor] if we let it see the bogus definitions.
-class AngularResolverVisitor extends _IntermediateResolverVisitor
- with ReportUnacceptableNodesMixin {
- final bool acceptAssignment;
+/// Overrides standard [ResolverVisitor] to prevent issues with analyzing
+/// dangling angular nodes, while also allowing custom resolution of pipes. Not
+/// intended as a long-term solution.
+class AngularResolverVisitor extends _IntermediateResolverVisitor {
final List<Pipe> pipes;
AngularResolverVisitor(
@@ -115,14 +64,14 @@
Source source,
TypeProvider typeProvider,
AnalysisErrorListener errorListener,
- {@required this.acceptAssignment,
- @required this.pipes})
+ {@required this.pipes})
: super(
inheritanceManager2, library, source, typeProvider, errorListener);
@override
void visitAsExpression(AsExpression exp) {
// This means we generated this in a pipe, and its OK.
+ // TODO(mfairhurst): figure out an alternative approach to this.
if (exp.asOperator.offset == 0) {
super.visitAsExpression(exp);
final pipeName = exp.getProperty<SimpleIdentifier>('_ng_pipeName');
@@ -146,75 +95,14 @@
[exp.expression.staticType, matchingPipe.requiredArgumentType]);
}
}
- } else {
- _reportUnacceptableNode(exp, "As expression");
}
}
@override
- void visitAssignmentExpression(AssignmentExpression exp) {
- if (exp.operator.type != TokenType.EQ) {
- _reportUnacceptableNode(exp, 'Compound assignment');
- }
- // Only block reassignment of locals, not poperties. Resolve elements to
- // check that.
- exp.leftHandSide.accept(elementResolver);
- final variableElement = ErrorVerifier.getVariableElement(exp.leftHandSide);
- if ((variableElement == null ||
- variableElement is PropertyInducingElement) &&
- acceptAssignment) {
- super.visitAssignmentExpression(exp);
- } else {
- _reportUnacceptableNode(exp, "Assignment of locals");
- }
+ void visitFunctionExpression(FunctionExpression func) {
+ // Stop resolving or analyzer will crash.
+ // TODO(mfairhurst): fix the analyzer crash and remove this.
}
-
- @override
- void visitAwaitExpression(AwaitExpression exp) =>
- _reportUnacceptableNode(exp, "Await");
-
- @override
- void visitCascadeExpression(CascadeExpression exp) {
- _reportUnacceptableNode(exp, "Cascades", false);
- // Only resolve the target, not the cascade sections.
- exp.target.accept(this);
- }
-
- @override
- void visitFunctionExpression(FunctionExpression exp) =>
- _reportUnacceptableNode(exp, "Anonymous functions", false);
-
- @override
- void visitIsExpression(IsExpression exp) =>
- _reportUnacceptableNode(exp, "Is expression");
-
- @override
- void visitNamedExpression(NamedExpression exp) =>
- _reportUnacceptableNode(exp, "Named arguments");
-
- @override
- void visitPostfixExpression(PostfixExpression exp) {
- _reportUnacceptableNode(exp, exp.operator.lexeme);
- }
-
- @override
- void visitPrefixExpression(PrefixExpression exp) {
- if (exp.operator.type != TokenType.MINUS) {
- _reportUnacceptableNode(exp, exp.operator.lexeme);
- }
- }
-
- @override
- void visitSuperExpression(SuperExpression exp) =>
- _reportUnacceptableNode(exp, "Super references");
-
- @override
- void visitSymbolLiteral(SymbolLiteral exp) =>
- _reportUnacceptableNode(exp, "Symbol literal");
-
- @override
- void visitThrowExpression(ThrowExpression exp) =>
- _reportUnacceptableNode(exp, "Throw");
}
/// Probably the most important visitor to understand in how we process angular
@@ -311,6 +199,119 @@
}
}
+/// Find nodes which are not supported in angular, such as compound assignment
+/// and function expressions etc.
+class AngularSubsetVisitor extends RecursiveAstVisitor<Object> {
+ final bool acceptAssignment;
+
+ final ErrorReporter errorReporter;
+
+ AngularSubsetVisitor(
+ {@required this.errorReporter, @required this.acceptAssignment});
+
+ @override
+ void visitAsExpression(AsExpression exp) {
+ if (exp.asOperator.offset == 0) {
+ // This means we generated this in a pipe, and its OK.
+ } else {
+ _reportDisallowedExpression(exp, "As expression", visitChildren: false);
+ }
+
+ super.visitAsExpression(exp);
+ }
+
+ @override
+ void visitAssignmentExpression(AssignmentExpression exp) {
+ if (exp.operator.type != TokenType.EQ) {
+ _reportDisallowedExpression(exp, 'Compound assignment',
+ visitChildren: false);
+ }
+ // Only block reassignment of locals, not poperties. Resolve elements to
+ // check that.
+ final variableElement = ErrorVerifier.getVariableElement(exp.leftHandSide);
+ final isLocal =
+ variableElement != null && variableElement is! PropertyInducingElement;
+ if (!acceptAssignment || isLocal) {
+ _reportDisallowedExpression(exp, 'Assignment of locals',
+ visitChildren: false);
+ }
+
+ super.visitAssignmentExpression(exp);
+ }
+
+ @override
+ void visitAwaitExpression(AwaitExpression exp) =>
+ _reportDisallowedExpression(exp, "Await");
+
+ @override
+ void visitCascadeExpression(CascadeExpression exp) =>
+ _reportDisallowedExpression(exp, "Cascades");
+
+ @override
+ void visitFunctionExpression(FunctionExpression exp) =>
+ _reportDisallowedExpression(exp, "Anonymous functions");
+
+ @override
+ void visitInstanceCreationExpression(InstanceCreationExpression exp) =>
+ _reportDisallowedExpression(exp, "Usage of new");
+
+ @override
+ void visitIsExpression(IsExpression exp) =>
+ _reportDisallowedExpression(exp, "Is expression");
+
+ @override
+ void visitListLiteral(ListLiteral list) {
+ if (list.typeArguments != null) {
+ _reportDisallowedExpression(list, "Typed list literals");
+ } else {
+ super.visitListLiteral(list);
+ }
+ }
+
+ @override
+ void visitMapLiteral(MapLiteral map) {
+ if (map.typeArguments != null) {
+ _reportDisallowedExpression(map, "Typed map literals");
+ } else {
+ super.visitMapLiteral(map);
+ }
+ }
+
+ @override
+ void visitNamedExpression(NamedExpression exp) =>
+ _reportDisallowedExpression(exp, "Named arguments");
+
+ @override
+ void visitPostfixExpression(PostfixExpression exp) {
+ _reportDisallowedExpression(exp, exp.operator.lexeme);
+ }
+
+ @override
+ void visitPrefixExpression(PrefixExpression exp) {
+ if (exp.operator.type != TokenType.MINUS) {
+ _reportDisallowedExpression(exp, exp.operator.lexeme);
+ }
+ }
+
+ @override
+ void visitSymbolLiteral(SymbolLiteral exp) =>
+ _reportDisallowedExpression(exp, "Symbol literal");
+
+ @override
+ void visitThrowExpression(ThrowExpression exp) =>
+ _reportDisallowedExpression(exp, "Throw");
+
+ void _reportDisallowedExpression(Expression node, String description,
+ {bool visitChildren = true}) {
+ errorReporter.reportErrorForNode(
+ AngularWarningCode.DISALLOWED_EXPRESSION, node, [description]);
+
+ if (visitChildren) {
+ node.visitChildren(this);
+ }
+ }
+}
+
class ComponentContentResolver extends AngularAstVisitor {
final Source templateSource;
final Template template;
@@ -1187,23 +1188,6 @@
type.lookUpInheritedGetter(name)?.returnType;
}
-abstract class ReportUnacceptableNodesMixin
- implements RecursiveAstVisitor<Object> {
- ErrorReporter get errorReporter;
- TypeProvider get typeProvider;
- void _reportUnacceptableNode(Expression node, String description,
- [bool visitChildren = true]) {
- errorReporter.reportErrorForNode(
- AngularWarningCode.DISALLOWED_EXPRESSION, node, [description]);
-
- // "resolve" the node, a null type causes later errors.
- node.propagatedType = node.staticType = typeProvider.dynamicType;
- if (visitChildren) {
- node.visitChildren(this);
- }
- }
-}
-
/// Once all the scopes for all the expressions & statements are prepared, we're
/// ready to resolve all the expressions inside and typecheck everything.
///
@@ -1565,7 +1549,7 @@
new InheritanceManager2(typeSystem as StrongTypeSystemImpl);
final resolver = new AngularResolverVisitor(inheritanceManager2, library,
templateSource, typeProvider, errorListener,
- acceptAssignment: acceptAssignment, pipes: pipes);
+ pipes: pipes);
// fill the name scope
final classScope = new ClassScope(resolver.nameScope, classElement);
final localScope = new EnclosedScope(classScope);
@@ -1579,9 +1563,13 @@
// verify
final verifier = new AngularErrorVerifier(
errorReporter, library, typeProvider, inheritanceManager2,
- acceptAssignment: acceptAssignment)
+ enableSuperMixins: true)
..enclosingClass = classElement;
astNode.accept(verifier);
+ // Check for concepts illegal to templates (for instance function literals).
+ final angularSubsetChecker = new AngularSubsetVisitor(
+ errorReporter: errorReporter, acceptAssignment: acceptAssignment);
+ astNode.accept(angularSubsetChecker);
}
/// Resolve the Dart expression with the given [code] at [offset].
@@ -1929,20 +1917,6 @@
}
/// Workaround for "This mixin application is invalid because all of the
-/// constructors in the base class 'ErrorVerifier' have optional parameters."
-/// in the definition of [AngularErrorVerifier].
-///
-/// See https://github.com/dart-lang/sdk/issues/15101 for details
-class _IntermediateErrorVerifier extends ErrorVerifier {
- _IntermediateErrorVerifier(
- ErrorReporter errorReporter,
- LibraryElement library,
- TypeProvider typeProvider,
- InheritanceManager2 inheritanceManager2,
- ) : super(errorReporter, library, typeProvider, inheritanceManager2, false);
-}
-
-/// Workaround for "This mixin application is invalid because all of the
/// constructors in the base class 'ResolverVisitor' have optional parameters."
/// in the definition of [AngularResolverVisitor].
///
diff --git a/angular_analyzer_plugin/test/abstract_angular.dart b/angular_analyzer_plugin/test/abstract_angular.dart
index 0901d0e..b9573ce 100644
--- a/angular_analyzer_plugin/test/abstract_angular.dart
+++ b/angular_analyzer_plugin/test/abstract_angular.dart
@@ -116,16 +116,22 @@
AbstractAngularTest() : includeQueryList = true;
AbstractAngularTest.future() : includeQueryList = false;
- /// Assert that the [errCode] is reported for [code], highlighting the [snippet].
+ /// Assert that the [errCode] is reported for [code], highlighting the
+ /// [snippet]. Optionally, expect [additionalErrorCodes] to appear at any
+ /// location.
void assertErrorInCodeAtPosition(
- ErrorCode errCode, String code, String snippet) {
+ ErrorCode errCode, String code, String snippet,
+ {List<ErrorCode> additionalErrorCodes}) {
final snippetIndex = code.indexOf(snippet);
expect(snippetIndex, greaterThan(-1),
reason: 'Error in test: snippet $snippet not part of code $code');
- errorListener.assertErrorsWithCodes(<ErrorCode>[errCode]);
- final error = errorListener.errors.single;
+ final expectedErrorCodes = (additionalErrorCodes ?? <ErrorCode>[])
+ ..add(errCode);
+ errorListener.assertErrorsWithCodes(expectedErrorCodes);
+ final error =
+ errorListener.errors.singleWhere((e) => e.errorCode == errCode);
expect(error.offset, snippetIndex);
- expect(errorListener.errors.single.length, snippet.length);
+ expect(error.length, snippet.length);
}
/// For [expectedErrors], it is a List of Tuple4 (1 per error):
diff --git a/angular_analyzer_plugin/test/resolver_test.dart b/angular_analyzer_plugin/test/resolver_test.dart
index fef6200..8122080 100644
--- a/angular_analyzer_plugin/test/resolver_test.dart
+++ b/angular_analyzer_plugin/test/resolver_test.dart
@@ -1893,7 +1893,10 @@
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "#symbol");
+ AngularWarningCode.DISALLOWED_EXPRESSION, code, "#symbol",
+ additionalErrorCodes: [
+ StaticTypeWarningCode.INVOCATION_OF_NON_FUNCTION_EXPRESSION
+ ]);
}
Future
@@ -2147,16 +2150,16 @@
_addDartSource(r'''
@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
class TestPanel {
- String str;
+ bool val;
}
''');
final code = r"""
-<h1 [hidden]="str..x"></h1>
+<h1 [hidden]="val..toString"></h1>
""";
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "str..x");
+ AngularWarningCode.DISALLOWED_EXPRESSION, code, "val..toString");
}
// ignore: non_constant_identifier_names
@@ -2270,12 +2273,12 @@
}
''');
final code = r"""
-<h1 [hidden]="new String().isEmpty"></h1>
+<h1 [hidden]="new TestPanel() != null"></h1>
""";
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "new String()");
+ AngularWarningCode.DISALLOWED_EXPRESSION, code, "new TestPanel()");
}
Future test_expressionNotAllowed_plusEq() async {
@@ -2370,12 +2373,12 @@
}
''');
final code = r"""
-<h1 #h1 [hidden]="h1 = 4"></h1>
+ <h1 #h1 [hidden]="(h1 = null) == null"></h1>
""";
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "h1 = 4");
+ AngularWarningCode.DISALLOWED_EXPRESSION, code, "h1 = null");
}
// ignore: non_constant_identifier_names
@@ -2392,7 +2395,7 @@
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "rethrow");
+ CompileTimeErrorCode.RETHROW_OUTSIDE_CATCH, code, "rethrow");
}
// ignore: non_constant_identifier_names
@@ -2404,7 +2407,7 @@
}
''');
final code = r"""
-<h1 [hidden]="str = 'hey'"></h1>
+<h1 [hidden]="(str = 'hey') == null"></h1>
""";
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
@@ -2421,12 +2424,12 @@
}
''');
final code = r"""
-<h1 #h1 (click)="h1 = 4"></h1>
+<h1 #h1 (click)="h1 = null"></h1>
""";
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "h1 = 4");
+ AngularWarningCode.DISALLOWED_EXPRESSION, code, "h1 = null");
}
// ignore: non_constant_identifier_names
@@ -2443,7 +2446,7 @@
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "super");
+ CompileTimeErrorCode.SUPER_IN_INVALID_CONTEXT, code, "super");
}
// ignore: non_constant_identifier_names
@@ -2455,7 +2458,7 @@
}
''');
final code = r"""
-<h1 [hidden]="#symbol"></h1>
+<h1 [hidden]="#symbol == null"></h1>
""";
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
@@ -2477,7 +2480,7 @@
_addHtmlSource(code);
await _resolveSingleTemplate(dartSource);
assertErrorInCodeAtPosition(
- AngularWarningCode.DISALLOWED_EXPRESSION, code, "this");
+ CompileTimeErrorCode.INVALID_REFERENCE_TO_THIS, code, "this");
}
// ignore: non_constant_identifier_names
