Validate `[attr.foo.if]`: check type bool, and `[attr.foo]` exists. (#624)
As a bonus, make `[attr.foo.if]` click-thru navigable to `[attr.foo]`.
Did some cleanup of the way that these binding type names are cut off.
The diff is ugly but it's mostly just a simple rename. Previous naming
was not clear, and the change required to lop off the 'if' from the name
was not easy to clearly add in.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76e277f..f4c8ffd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
- Typecheck the results and input of pipe expressions and the existence of a
matching pipe. Optional arguments are not yet typechecked.
+- Add typechecking support for `[attr.foo.if]`, and ensure that a corresponding
+ `[attr.foo]` binding exists.
## 0.0.17+3
diff --git a/README.md b/README.md
index 8b35d1f..2b3ed1c 100644
--- a/README.md
+++ b/README.md
@@ -106,6 +106,7 @@
`<input [value]="firstName">` | :white_check_mark: soundness of expression, type of expression, existence of `value` on element or directive | :white_check_mark: | :white_check_mark: | :x:
`<input bind-value="firstName">` | :white_check_mark: | :skull: | :white_check_mark: | :x:
`<div [attr.role]="myAriaRole">` | :last_quarter_moon: soundness of expression, but no other validation | :last_quarter_moon: complete inside binding but binding not suggested | :last_quarter_moon: no html specific navigation | :x:
+`<div [attr.role.if]="myAriaRole">` | :white_check_mark: | :last_quarter_moon: complete inside binding but binding not suggested | :white_check_mark: | :x:
`<div [class.extra-sparkle]="isDelightful">` | :white_check_mark: validity of clasname, soundness of expression, type of expression must be bool | :last_quarter_moon: complete inside binding but binding not suggested | :last_quarter_moon: no css specific navigation | :x:
`<div [style.width.px]="mySize">` | :waning_gibbous_moon: soundness of expression, css properties are generally checked but not against a dictionary, same for units, expression must type to `int` if units are present | :last_quarter_moon: complete inside binding but binding not suggested | :last_quarter_moon: no css specific navigation | :x:
`<button (click)="readRainbow($event)">` | :white_check_mark: soundness of expression, type of `$event`, existence of output on component/element and DOM events which propagate can be tracked anywhere | :white_check_mark: | :last_quarter_moon: no navigation for `$event` | :x:
diff --git a/angular_analyzer_plugin/lib/ast.dart b/angular_analyzer_plugin/lib/ast.dart
index e983e91..ec682ef 100644
--- a/angular_analyzer_plugin/lib/ast.dart
+++ b/angular_analyzer_plugin/lib/ast.dart
@@ -272,7 +272,7 @@
String toString() => '(${super.toString()}, [$bound, $expression])';
}
-enum ExpressionBoundType { input, twoWay, attr, clazz, style }
+enum ExpressionBoundType { input, twoWay, attr, attrIf, clazz, style }
/// An AngularAstNode which has directives, such as [ElementInfo] and
/// [TemplateAttribute]. Contains an array of [DirectiveBinding]s because those
diff --git a/angular_analyzer_plugin/lib/errors.dart b/angular_analyzer_plugin/lib/errors.dart
index 5493cf4..c373786 100644
--- a/angular_analyzer_plugin/lib/errors.dart
+++ b/angular_analyzer_plugin/lib/errors.dart
@@ -26,6 +26,8 @@
AngularWarningCode.NONEXIST_TWO_WAY_OUTPUT_BOUND,
AngularWarningCode.TWO_WAY_BINDING_OUTPUT_TYPE_ERROR,
AngularWarningCode.INPUT_BINDING_TYPE_ERROR,
+ AngularWarningCode.ATTR_IF_BINDING_TYPE_ERROR,
+ AngularWarningCode.UNMATCHED_ATTR_IF_BINDING,
AngularWarningCode.TRAILING_EXPRESSION,
AngularWarningCode.OUTPUT_MUST_BE_STREAM,
AngularWarningCode.TWO_WAY_BINDING_NOT_ASSIGNABLE,
@@ -229,6 +231,19 @@
'Attribute value expression (of type {0}) is not assignable to component'
' input (of type {1})');
+ /// An error code indicating that an [attr.foo.if] binding was bound to an
+ /// expression that was not of type bool
+ static const ATTR_IF_BINDING_TYPE_ERROR = const AngularWarningCode(
+ 'ATTR_IF_BINDING_TYPE_ERROR',
+ 'Attribute value expression (of type {0}) must be of type bool');
+
+ /// An error code indicating that an [attr.foo.if] binding was used, but no
+ /// [attr.foo] binding simultaneously occured.
+ static const UNMATCHED_ATTR_IF_BINDING = const AngularWarningCode(
+ 'UNMATCHED_ATTR_IF_BINDING',
+ 'attr-if binding for attr {0} does not have a corresponding'
+ ' [attr.{0}] binding for it to affect');
+
/// An error code indicating that an expression did not correctly
/// end with an EOF token.
static const TRAILING_EXPRESSION = const AngularWarningCode(
diff --git a/angular_analyzer_plugin/lib/src/converter.dart b/angular_analyzer_plugin/lib/src/converter.dart
index bfcd7f4..695685a 100644
--- a/angular_analyzer_plugin/lib/src/converter.dart
+++ b/angular_analyzer_plugin/lib/src/converter.dart
@@ -732,23 +732,40 @@
var propNameOffset = nameToken.offset;
if (ast is ParsedPropertyAst) {
- final name = ast.name;
+ // For some inputs, like `[class.foo]`, the [ast.name] here is actually
+ // not a name, but a prefix. If so, use the [ast.postfix] as the [name] of
+ // the [ExpressionBoundAttribute] we're creating here, by changing
+ // [propName].
+ final nameOrPrefix = ast.name;
+
if (ast.postfix != null) {
- var replacePropName = false;
- if (name == 'class') {
+ var usePostfixForName = false;
+ var preserveUnitInName = false;
+
+ if (nameOrPrefix == 'class') {
bound = ExpressionBoundType.clazz;
- replacePropName = true;
- } else if (name == 'attr') {
- bound = ExpressionBoundType.attr;
- replacePropName = true;
- } else if (name == 'style') {
+ usePostfixForName = true;
+ preserveUnitInName = true;
+ } else if (nameOrPrefix == 'attr') {
+ if (ast.unit == 'if') {
+ bound = ExpressionBoundType.attrIf;
+ } else {
+ bound = ExpressionBoundType.attr;
+ }
+ usePostfixForName = true;
+ } else if (nameOrPrefix == 'style') {
bound = ExpressionBoundType.style;
- replacePropName = true;
+ usePostfixForName = true;
+ preserveUnitInName = ast.unit != null;
}
- if (replacePropName) {
- final _unitName = ast.unit == null ? '' : '.${ast.unit}';
+
+ if (usePostfixForName) {
+ final _unitName =
+ preserveUnitInName && ast.unit != null ? '.${ast.unit}' : '';
propName = '${ast.postfix}$_unitName';
- propNameOffset = nameToken.offset + name.length + '.'.length;
+ propNameOffset = nameToken.offset + ast.name.length + '.'.length;
+ } else {
+ assert(!preserveUnitInName);
}
}
} else {
diff --git a/angular_analyzer_plugin/lib/src/resolver.dart b/angular_analyzer_plugin/lib/src/resolver.dart
index 294f667..cf964a0 100644
--- a/angular_analyzer_plugin/lib/src/resolver.dart
+++ b/angular_analyzer_plugin/lib/src/resolver.dart
@@ -1300,6 +1300,8 @@
_resolveStyleAttribute(attribute);
} else if (attribute.bound == ExpressionBoundType.attr) {
_resolveAttributeBoundAttribute(attribute);
+ } else if (attribute.bound == ExpressionBoundType.attrIf) {
+ _resolveAttributeBoundAttributeIf(attribute);
}
}
@@ -1448,6 +1450,55 @@
// failing tests for this)
}
+ /// Resolve attributes of type [attribute.some-attribute]="someExpr"
+ void _resolveAttributeBoundAttributeIf(ExpressionBoundAttribute attribute) {
+ if (attribute.parent is! ElementInfo) {
+ assert(false, 'Got an attr-if bound attribute on non element! Aborting!');
+ return;
+ }
+
+ final parent = attribute.parent as ElementInfo;
+
+ // For the [attr.foo.if] attribute, find the matching [attr.foo] attribute.
+ final matchingAttr = parent.attributes
+ .where((attr) =>
+ attr is ExpressionBoundAttribute &&
+ attr.bound == ExpressionBoundType.attr)
+ .firstWhere((attrAttr) => attrAttr.name == attribute.name,
+ orElse: () => null);
+
+ // Error: no matching attribute to make conditional via this attr-if.
+ if (matchingAttr == null) {
+ errorListener.onError(new AnalysisError(
+ templateSource,
+ attribute.nameOffset,
+ attribute.name.length,
+ AngularWarningCode.UNMATCHED_ATTR_IF_BINDING,
+ [attribute.name]));
+ return;
+ }
+
+ // Add navigation from [attribute] (`[attr.foo.if]`) to [matchingAttr]
+ // (`[attr.foo]`).
+ final range = new SourceRange(attribute.nameOffset, attribute.name.length);
+ template.addRange(
+ range,
+ new AngularElementImpl('attr.${attribute.name}',
+ matchingAttr.nameOffset, matchingAttr.name.length, templateSource));
+
+ // Ensure the if condition was a boolean.
+ if (attribute.expression != null &&
+ !typeSystem.isAssignableTo(
+ attribute.expression.staticType, typeProvider.boolType)) {
+ errorListener.onError(new AnalysisError(
+ templateSource,
+ attribute.valueOffset,
+ attribute.value.length,
+ AngularWarningCode.ATTR_IF_BINDING_TYPE_ERROR,
+ [attribute.name]));
+ }
+ }
+
/// Resolve attributes of type [class.some-class]="someBoolExpr", ensuring
/// the class is a valid css identifier and that the expression is of boolean
/// type
diff --git a/angular_analyzer_plugin/test/navigation_test.dart b/angular_analyzer_plugin/test/navigation_test.dart
index 939f42b..e481388 100644
--- a/angular_analyzer_plugin/test/navigation_test.dart
+++ b/angular_analyzer_plugin/test/navigation_test.dart
@@ -212,6 +212,33 @@
}
// ignore: non_constant_identifier_names
+ Future test_navigate_attrIf() async {
+ code = r'''
+import 'package:angular/src/core/metadata.dart';
+
+@Component(selector: 'foo', template: r"""
+<div
+ [attr.foo]="123"
+ [attr.foo.if]="true">
+</div>
+""")
+class TextPanel {
+}
+''';
+ final source = newSource('/test.dart', code);
+ // compute navigation regions
+ final result = await resolveDart(source);
+ new AngularNavigation(angularDriver.contentOverlay).computeNavigation(
+ new AngularNavigationRequest(null, null, null, result), collector,
+ templatesOnly: false);
+ {
+ _findRegionString('foo', '.if');
+ expect(targetLocation.file, '/test.dart');
+ expect(targetLocation.offset, code.indexOf('foo]'));
+ }
+ }
+
+ // ignore: non_constant_identifier_names
Future test_navigateOnfocusin() async {
code = r'''
import 'package:angular/src/core/metadata.dart';
diff --git a/angular_analyzer_plugin/test/resolver_test.dart b/angular_analyzer_plugin/test/resolver_test.dart
index 6f19d92..2900752 100644
--- a/angular_analyzer_plugin/test/resolver_test.dart
+++ b/angular_analyzer_plugin/test/resolver_test.dart
@@ -351,6 +351,92 @@
}
// ignore: non_constant_identifier_names
+ Future test_expression_attrBindingIf_attrNotBound() async {
+ _addDartSource(r'''
+@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
+class TestPanel {
+ bool cond;
+}
+''');
+ final code = r"""
+<span [attr.foo.if]='cond'></span>
+""";
+ _addHtmlSource(code);
+ await _resolveSingleTemplate(dartSource);
+ assertErrorInCodeAtPosition(
+ AngularWarningCode.UNMATCHED_ATTR_IF_BINDING, code, 'foo');
+ }
+
+ // ignore: non_constant_identifier_names
+ Future test_expression_attrBindingIf_empty() async {
+ _addDartSource(r'''
+@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
+class TestPanel {
+ String text1;
+}
+''');
+ final code = r"""
+<span [attr.foo]='text1' [attr.foo.if]></span>
+""";
+ _addHtmlSource(code);
+ await _resolveSingleTemplate(dartSource);
+ assertErrorInCodeAtPosition(
+ AngularWarningCode.EMPTY_BINDING, code, '[attr.foo.if]');
+ }
+
+ // ignore: non_constant_identifier_names
+ Future test_expression_attrBindingIf_emptyWithQuotes() async {
+ _addDartSource(r'''
+@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
+class TestPanel {
+ String text1;
+}
+''');
+ final code = r"""
+<span [attr.foo]='text1' [attr.foo.if]=''></span>
+""";
+ _addHtmlSource(code);
+ await _resolveSingleTemplate(dartSource);
+ assertErrorInCodeAtPosition(
+ AngularWarningCode.EMPTY_BINDING, code, '[attr.foo.if]');
+ }
+
+ // ignore: non_constant_identifier_names
+ Future test_expression_attrBindingIf_typeError() async {
+ _addDartSource(r'''
+@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
+class TestPanel {
+ String text1;
+ String text2;
+}
+''');
+ final code = r"""
+<span [attr.foo]='text1' [attr.foo.if]='text2'></span>
+""";
+ _addHtmlSource(code);
+ await _resolveSingleTemplate(dartSource);
+ assertErrorInCodeAtPosition(
+ AngularWarningCode.ATTR_IF_BINDING_TYPE_ERROR, code, 'text2');
+ }
+
+ // ignore: non_constant_identifier_names
+ Future test_expression_attrBindingIf_valid() async {
+ _addDartSource(r'''
+@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
+class TestPanel {
+ String text;
+ bool cond;
+}
+''');
+ final code = r"""
+<span [attr.foo]='text' [attr.foo.if]='cond'></span>
+""";
+ _addHtmlSource(code);
+ await _resolveSingleTemplate(dartSource);
+ errorListener.assertNoErrors();
+ }
+
+ // ignore: non_constant_identifier_names
Future test_expression_await_not_allowed() async {
_addDartSource(r'''
@Component(selector: 'test-panel', templateUrl: 'test_panel.html')
