diff --git a/packages/language-service/ivy/template_target.ts b/packages/language-service/ivy/template_target.ts
index 6c94a2b544..09650a2df3 100644
--- a/packages/language-service/ivy/template_target.ts
+++ b/packages/language-service/ivy/template_target.ts
@@ -156,11 +156,6 @@ export function getTargetAtPosition(template: t.Node[], position: number): Templ
}
}
- let parent: t.Node|e.AST|null = null;
- if (path.length >= 2) {
- parent = path[path.length - 2];
- }
-
// Given the candidate node, determine the full targeted context.
let nodeInContext: TargetContext;
if (candidate instanceof e.AST) {
@@ -191,7 +186,16 @@ export function getTargetAtPosition(template: t.Node[], position: number): Templ
(candidate instanceof t.BoundAttribute || candidate instanceof t.BoundEvent ||
candidate instanceof t.TextAttribute) &&
candidate.keySpan !== undefined) {
- if (isWithin(position, candidate.keySpan)) {
+ const previousCandidate = path[path.length - 2];
+ if (candidate instanceof t.BoundEvent && previousCandidate instanceof t.BoundAttribute &&
+ candidate.name === previousCandidate.name + 'Change') {
+ const boundAttribute: t.BoundAttribute = previousCandidate;
+ const boundEvent: t.BoundEvent = candidate;
+ nodeInContext = {
+ kind: TargetNodeKind.TwoWayBindingContext,
+ nodes: [boundAttribute, boundEvent],
+ };
+ } else if (isWithin(position, candidate.keySpan)) {
nodeInContext = {
kind: TargetNodeKind.AttributeInKeyContext,
node: candidate,
@@ -209,6 +213,13 @@ export function getTargetAtPosition(template: t.Node[], position: number): Templ
};
}
+ let parent: t.Node|e.AST|null = null;
+ if (nodeInContext.kind === TargetNodeKind.TwoWayBindingContext && path.length >= 3) {
+ parent = path[path.length - 3];
+ } else if (path.length >= 2) {
+ parent = path[path.length - 2];
+ }
+
return {position, context: nodeInContext, template: context, parent};
}
@@ -247,20 +258,25 @@ class TemplateTargetVisitor implements t.Visitor {
private constructor(private readonly position: number) {}
visit(node: t.Node) {
- const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
- if (last && isTemplateNodeWithKeyAndValue(last) && isWithin(this.position, last.keySpan)) {
- // We've already identified that we are within a `keySpan` of a node.
- // We should stop processing nodes at this point to prevent matching
- // any other nodes. This can happen when the end span of a different node
- // touches the start of the keySpan for the candidate node. Because
- // our `isWithin` logic is inclusive on both ends, we can match both nodes.
- return;
- }
const {start, end} = getSpanIncludingEndTag(node);
if (!isWithin(this.position, {start, end})) {
return;
}
+ const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
+ const withinKeySpanOfLastNode =
+ last && isTemplateNodeWithKeyAndValue(last) && isWithin(this.position, last.keySpan);
+ const withinKeySpanOfCurrentNode =
+ isTemplateNodeWithKeyAndValue(node) && isWithin(this.position, node.keySpan);
+ if (withinKeySpanOfLastNode && !withinKeySpanOfCurrentNode) {
+ // We've already identified that we are within a `keySpan` of a node.
+ // Unless we are _also_ in the `keySpan` of the current node (happens with two way bindings),
+ // we should stop processing nodes at this point to prevent matching any other nodes. This can
+ // happen when the end span of a different node touches the start of the keySpan for the
+ // candidate node. Because our `isWithin` logic is inclusive on both ends, we can match both
+ // nodes.
+ return;
+ }
if (isTemplateNodeWithKeyAndValue(node) && !isWithinKeyValue(this.position, node)) {
// If cursor is within source span but not within key span or value span,
// do not return the node.
@@ -272,31 +288,33 @@ class TemplateTargetVisitor implements t.Visitor {
}
visitElement(element: t.Element) {
+ this.visitElementOrTemplate(element);
+ }
+
+
+ visitTemplate(template: t.Template) {
+ this.visitElementOrTemplate(template);
+ }
+
+ visitElementOrTemplate(element: t.Template|t.Element) {
this.visitAll(element.attributes);
this.visitAll(element.inputs);
this.visitAll(element.outputs);
+ if (element instanceof t.Template) {
+ this.visitAll(element.templateAttrs);
+ }
this.visitAll(element.references);
- const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
+ if (element instanceof t.Template) {
+ this.visitAll(element.variables);
+ }
+
// If we get here and have not found a candidate node on the element itself, proceed with
// looking for a more specific node on the element children.
- if (last === element) {
- this.visitAll(element.children);
+ if (this.path[this.path.length - 1] !== element) {
+ return;
}
- }
- visitTemplate(template: t.Template) {
- this.visitAll(template.attributes);
- this.visitAll(template.inputs);
- this.visitAll(template.outputs);
- this.visitAll(template.templateAttrs);
- this.visitAll(template.references);
- this.visitAll(template.variables);
- const last: t.Node|e.AST|undefined = this.path[this.path.length - 1];
- // If we get here and have not found a candidate node on the template itself, proceed with
- // looking for a more specific node on the template children.
- if (last === template) {
- this.visitAll(template.children);
- }
+ this.visitAll(element.children);
}
visitContent(content: t.Content) {
@@ -321,18 +339,6 @@ class TemplateTargetVisitor implements t.Visitor {
}
visitBoundEvent(event: t.BoundEvent) {
- const isTwoWayBinding =
- this.path.some(n => n instanceof t.BoundAttribute && event.name === n.name + 'Change');
- if (isTwoWayBinding) {
- // For two-way binding aka banana-in-a-box, there are two matches:
- // BoundAttribute and BoundEvent. Both have the same spans. We choose to
- // return BoundAttribute because it matches the identifier name verbatim.
- // TODO: For operations like go to definition, ideally we want to return
- // both.
- this.path.pop(); // remove bound event from the AST path
- return;
- }
-
// An event binding with no value (e.g. `(event|)`) parses to a `BoundEvent` with a
// `LiteralPrimitive` handler with value `'ERROR'`, as opposed to a property binding with no
// value which has an `EmptyExpr` as its value. This is a synthetic node created by the binding
diff --git a/packages/language-service/ivy/test/legacy/template_target_spec.ts b/packages/language-service/ivy/test/legacy/template_target_spec.ts
index 52e58a7903..56b62ade24 100644
--- a/packages/language-service/ivy/test/legacy/template_target_spec.ts
+++ b/packages/language-service/ivy/test/legacy/template_target_spec.ts
@@ -10,7 +10,7 @@ import {ParseError, parseTemplate} from '@angular/compiler';
import * as e from '@angular/compiler/src/expression_parser/ast'; // e for expression AST
import * as t from '@angular/compiler/src/render3/r3_ast'; // t for template AST
-import {getTargetAtPosition, SingleNodeTarget, TargetNodeKind} from '../../template_target';
+import {getTargetAtPosition, SingleNodeTarget, TargetNodeKind, TwoWayBindingContext} from '../../template_target';
import {isExpressionNode, isTemplateNode} from '../../utils';
interface ParseResult {
@@ -180,11 +180,11 @@ describe('getTargetAtPosition for template AST', () => {
it('should locate template bound attribute key in two-way binding', () => {
const {errors, nodes, position} = parse(``);
expect(errors).toBe(null);
- const {context} = getTargetAtPosition(nodes, position)!;
- const {node} = context as SingleNodeTarget;
- expect(isTemplateNode(node!)).toBe(true);
- expect(node).toBeInstanceOf(t.BoundAttribute);
- expect((node as t.BoundAttribute).name).toBe('foo');
+ const {context, parent} = getTargetAtPosition(nodes, position)!;
+ expect(parent).toBeInstanceOf(t.Template);
+ const {nodes: [boundAttribute, boundEvent]} = context as TwoWayBindingContext;
+ expect(boundAttribute.name).toBe('foo');
+ expect(boundEvent.name).toBe('fooChange');
});
it('should locate template bound attribute value in two-way binding', () => {
@@ -193,8 +193,12 @@ describe('getTargetAtPosition for template AST', () => {
const {context} = getTargetAtPosition(nodes, position)!;
const {node} = context as SingleNodeTarget;
expect(isExpressionNode(node!)).toBe(true);
- expect(node).toBeInstanceOf(e.PropertyRead);
- expect((node as e.PropertyRead).name).toBe('bar');
+ // It doesn't actually matter if the template target returns the read or the write.
+ // When the template target returns a property read, we only use the LHS downstream because the
+ // RHS would have its own node in the AST that would have been returned instead. The LHS of the
+ // `e.PropertyWrite` is the same as the `e.PropertyRead`.
+ expect((node instanceof e.PropertyRead) || (node instanceof e.PropertyWrite)).toBeTrue();
+ expect((node as e.PropertyRead | e.PropertyWrite).name).toBe('bar');
});
it('should locate template bound event key', () => {
@@ -342,21 +346,26 @@ describe('getTargetAtPosition for template AST', () => {
it('should locate bound attribute key in two-way binding', () => {
const {errors, nodes, position} = parse(``);
expect(errors).toBe(null);
- const {context} = getTargetAtPosition(nodes, position)!;
- const {node} = context as SingleNodeTarget;
- expect(isTemplateNode(node!)).toBe(true);
- expect(node).toBeInstanceOf(t.BoundAttribute);
- expect((node as t.BoundAttribute).name).toBe('foo');
+ const {context, parent} = getTargetAtPosition(nodes, position)!;
+ expect(parent).toBeInstanceOf(t.Element);
+ const {nodes: [boundAttribute, boundEvent]} = context as TwoWayBindingContext;
+ expect(boundAttribute.name).toBe('foo');
+ expect(boundEvent.name).toBe('fooChange');
});
- it('should locate bound attribute value in two-way binding', () => {
+ it('should locate node when in value span of two-way binding', () => {
const {errors, nodes, position} = parse(``);
expect(errors).toBe(null);
- const {context} = getTargetAtPosition(nodes, position)!;
+ const {context, parent} = getTargetAtPosition(nodes, position)!;
+ // It doesn't actually matter if the template target returns the read or the write.
+ // When the template target returns a property read, we only use the LHS downstream because the
+ // RHS would have its own node in the AST that would have been returned instead. The LHS of the
+ // `e.PropertyWrite` is the same as the `e.PropertyRead`.
+ expect((parent instanceof t.BoundAttribute) || (parent instanceof t.BoundEvent)).toBe(true);
const {node} = context as SingleNodeTarget;
expect(isExpressionNode(node!)).toBe(true);
- expect(node).toBeInstanceOf(e.PropertyRead);
- expect((node as e.PropertyRead).name).toBe('bar');
+ expect((node instanceof e.PropertyRead) || (node instanceof e.PropertyWrite)).toBeTrue();
+ expect((node as e.PropertyRead | e.PropertyWrite).name).toBe('bar');
});
it('should locate switch value in ICUs', () => {
diff --git a/packages/language-service/ivy/utils.ts b/packages/language-service/ivy/utils.ts
index 7c67840c3c..57c80704f2 100644
--- a/packages/language-service/ivy/utils.ts
+++ b/packages/language-service/ivy/utils.ts
@@ -54,6 +54,16 @@ export function isTemplateNodeWithKeyAndValue(node: t.Node|e.AST): node is NodeW
return isTemplateNode(node) && node.hasOwnProperty('keySpan');
}
+export function isWithinKey(position: number, node: NodeWithKeyAndValue): boolean {
+ let {keySpan, valueSpan} = node;
+ if (valueSpan === undefined && node instanceof TmplAstBoundEvent) {
+ valueSpan = node.handlerSpan;
+ }
+ const isWithinKeyValue =
+ isWithin(position, keySpan) || !!(valueSpan && isWithin(position, valueSpan));
+ return isWithinKeyValue;
+}
+
export function isWithinKeyValue(position: number, node: NodeWithKeyAndValue): boolean {
let {keySpan, valueSpan} = node;
if (valueSpan === undefined && node instanceof TmplAstBoundEvent) {