Feedback from Paul: Don't use tuple, ambiguous pipes, etc (#630)
- Don't use tuple
- Ambiguous pipe error message
- replace `parseIdentifierList().first` with the intended,
`parseSimpleIdentifier()`
- unnecessary null check
diff --git a/angular_analyzer_plugin/lib/errors.dart b/angular_analyzer_plugin/lib/errors.dart
index c373786..50e964e 100644
--- a/angular_analyzer_plugin/lib/errors.dart
+++ b/angular_analyzer_plugin/lib/errors.dart
@@ -65,6 +65,7 @@
AngularWarningCode.PIPE_TRANSFORM_NO_NAMED_ARGS,
AngularWarningCode.PIPE_TRANSFORM_REQ_ONE_ARG,
AngularWarningCode.PIPE_NOT_FOUND,
+ AngularWarningCode.AMBIGUOUS_PIPE,
AngularWarningCode.UNSAFE_BINDING,
AngularWarningCode.EVENT_REDUCTION_NOT_ALLOWED,
AngularWarningCode.FUNCTIONAL_DIRECTIVES_CANT_BE_EXPORTED,
@@ -498,6 +499,13 @@
"Pipe by name of {0} not found. Did you reference it in your @Component"
" configuration?");
+ /// An error indicating that pipe syntax was used in an angular template, but
+ /// the name of the pipe doesn't match one defined in the component
+ static const AMBIGUOUS_PIPE = const AngularWarningCode(
+ 'AMBIGUOUS_PIPE',
+ "Multiple pipes by name of {0} found. Check the `pipes` field of your "
+ "@Component annotation for duplicates and/or conflicts.");
+
/// An error indicating that a security exception will be thrown by this input
/// binding
static const UNSAFE_BINDING = const AngularWarningCode(
diff --git a/angular_analyzer_plugin/lib/src/ng_expr_parser.dart b/angular_analyzer_plugin/lib/src/ng_expr_parser.dart
index 2202537..1c010e1 100644
--- a/angular_analyzer_plugin/lib/src/ng_expr_parser.dart
+++ b/angular_analyzer_plugin/lib/src/ng_expr_parser.dart
@@ -13,36 +13,42 @@
Token get _currentToken => super.currentToken;
- /// Parse a bitwise or expression to be treated as a pipe.
- /// Return the resolved left-hand expression as a dynamic type.
+ /// Override the bitwise or operator to parse pipes instead
+ @override
+ Expression parseBitwiseOrExpression() => parsePipeExpression();
+
+ /// Parse pipe expression. Return the result as a cast expression `as dynamic`
+ /// with _ng_pipeXXX properties to be resolved specially later.
///
/// bitwiseOrExpression ::=
- /// bitwiseXorExpression ('|' pipeExpression)*
- @override
- Expression parseBitwiseOrExpression() {
+ /// bitwiseXorExpression ('|' pipeName [: arg]*)*
+ Expression parsePipeExpression() {
Expression expression;
Token beforePipeToken;
expression = parseBitwiseXorExpression();
while (_currentToken.type == TokenType.BAR) {
beforePipeToken ??= _currentToken.previous;
getAndAdvance();
- final pipeData = parsePipeExpression();
- if (beforePipeToken != null) {
- final asToken = new KeywordToken(Keyword.AS, 0);
- final dynamicIdToken = new SyntheticStringToken(TokenType.IDENTIFIER,
- "dynamic", _currentToken.offset - "dynamic".length);
+ final pipeEntities = parsePipeExpressionEntities();
+ final asToken = new KeywordToken(Keyword.AS, 0);
+ final dynamicIdToken = new SyntheticStringToken(TokenType.IDENTIFIER,
+ "dynamic", _currentToken.offset - "dynamic".length);
- beforePipeToken.setNext(asToken);
- asToken.setNext(dynamicIdToken);
- dynamicIdToken.setNext(_currentToken);
+ beforePipeToken.setNext(asToken);
+ asToken.setNext(dynamicIdToken);
+ dynamicIdToken.setNext(_currentToken);
- final dynamicIdentifier = astFactory.simpleIdentifier(dynamicIdToken);
+ final dynamicIdentifier = astFactory.simpleIdentifier(dynamicIdToken);
- expression = astFactory.asExpression(
- expression, asToken, astFactory.typeName(dynamicIdentifier, null))
- ..setProperty('_ng_pipeName', pipeData.item1)
- ..setProperty('_ng_pipeArgs', pipeData.item2);
- }
+ // TODO(mfairhurst) Now that we are resolving pipes, probably should store
+ // the result in a different expression type -- a function call, most
+ // likely. This will be required so that the pipeArgs become part of the
+ // tree, but it may create secondary fallout inside the analyzer
+ // resolution code if done wrong.
+ expression = astFactory.asExpression(
+ expression, asToken, astFactory.typeName(dynamicIdentifier, null))
+ ..setProperty('_ng_pipeName', pipeEntities.name)
+ ..setProperty('_ng_pipeArgs', pipeEntities.arguments);
}
return expression;
}
@@ -51,15 +57,21 @@
/// Return the resolved left-hand expression as a dynamic type.
///
/// pipeExpression ::= identifier[':' expression]*
- Tuple2<SimpleIdentifier, List<Expression>> parsePipeExpression() {
- final identifiers = parseIdentifierList();
+ _PipeEntities parsePipeExpressionEntities() {
+ final identifier = parseSimpleIdentifier();
final expressions = <Expression>[];
while (_currentToken.type == TokenType.COLON) {
getAndAdvance();
expressions.add(parseExpression2());
}
- return Tuple2<SimpleIdentifier, List<Expression>>(
- identifiers.first, expressions);
+ return _PipeEntities(identifier, expressions);
}
}
+
+class _PipeEntities {
+ final SimpleIdentifier name;
+ final List<Expression> arguments;
+
+ _PipeEntities(this.name, this.arguments);
+}
diff --git a/angular_analyzer_plugin/lib/src/resolver.dart b/angular_analyzer_plugin/lib/src/resolver.dart
index 7b058b4..1aa4a94 100644
--- a/angular_analyzer_plugin/lib/src/resolver.dart
+++ b/angular_analyzer_plugin/lib/src/resolver.dart
@@ -121,9 +121,12 @@
final pipeName = exp.getProperty<SimpleIdentifier>('_ng_pipeName');
final matchingPipes =
pipes.where((pipe) => pipe.pipeName == pipeName.name);
- if (matchingPipes.length != 1) {
+ if (matchingPipes.isEmpty) {
errorReporter.reportErrorForNode(
AngularWarningCode.PIPE_NOT_FOUND, pipeName, [pipeName]);
+ } else if (matchingPipes.length > 1) {
+ errorReporter.reportErrorForNode(
+ AngularWarningCode.AMBIGUOUS_PIPE, pipeName, [pipeName]);
} else {
final matchingPipe = matchingPipes.single;
exp.staticType = matchingPipe.transformReturnType;
diff --git a/angular_analyzer_plugin/test/resolver_test.dart b/angular_analyzer_plugin/test/resolver_test.dart
index 0a9d030..e4cf391 100644
--- a/angular_analyzer_plugin/test/resolver_test.dart
+++ b/angular_analyzer_plugin/test/resolver_test.dart
@@ -1663,6 +1663,33 @@
}
// ignore: non_constant_identifier_names
+ Future test_expression_pipe_in_moustache_ambiguous() async {
+ _addDartSource(r'''
+@Component(selector: 'test-panel', templateUrl: 'test_panel.html',
+ pipes: [AmbiguousPipe1, AmbiguousPipe2])
+class TestPanel {
+}
+
+@Pipe('ambiguous')
+class AmbiguousPipe1 extends PipeTransform {
+ String transform(String x => "";
+}
+
+@Pipe('ambiguous')
+class AmbiguousPipe2 extends PipeTransform {
+ String transform(String x => "";
+}
+''');
+ final code = r"""
+{{"" | ambiguous}}
+""";
+ _addHtmlSource(code);
+ await _resolveSingleTemplate(dartSource);
+ assertErrorInCodeAtPosition(
+ AngularWarningCode.AMBIGUOUS_PIPE, code, 'ambiguous');
+ }
+
+ // ignore: non_constant_identifier_names
@failingTest
Future test_expression_pipe_in_moustache_extraArg() async {
_addDartSource(r'''
