refactor(compiler-cli): add keySpan to reference nodes (#39616)

Similar to #39613, #39609, and #38898, we should store the `keySpan` for
Reference nodes so that we can accurately map from a template node to a
span in the original file. This is most notably an issue at the moment
for directive references `#ref="exportAs"`. The current behavior for the
language service when requesting information for the reference
is that it will return a text span that results in
highlighting the entire source when it should only highlight "ref" (test
added for this case as well).

PR Close #39616
This commit is contained in:
Andrew Scott 2020-11-09 13:35:48 -08:00 committed by atscott
parent c33326c538
commit 8a1c98c5e8
5 changed files with 30 additions and 19 deletions

View File

@ -139,7 +139,7 @@ export class Variable implements Node {
export class Reference implements Node {
constructor(
public name: string, public value: string, public sourceSpan: ParseSourceSpan,
public valueSpan?: ParseSourceSpan) {}
readonly keySpan: ParseSourceSpan, public valueSpan?: ParseSourceSpan) {}
visit<Result>(visitor: Visitor<Result>): Result {
return visitor.visitReference(this);
}

View File

@ -357,7 +357,8 @@ class HtmlAstToIvyAst implements html.Visitor {
} else if (bindParts[KW_REF_IDX]) {
const identifier = bindParts[IDENT_KW_IDX];
this.parseReference(identifier, value, srcSpan, attribute.valueSpan, references);
const keySpan = createKeySpan(srcSpan, bindParts[KW_REF_IDX], identifier);
this.parseReference(identifier, value, srcSpan, keySpan, attribute.valueSpan, references);
} else if (bindParts[KW_ON_IDX]) {
const events: ParsedEvent[] = [];
const identifier = bindParts[IDENT_KW_IDX];
@ -451,7 +452,7 @@ class HtmlAstToIvyAst implements html.Visitor {
}
private parseReference(
identifier: string, value: string, sourceSpan: ParseSourceSpan,
identifier: string, value: string, sourceSpan: ParseSourceSpan, keySpan: ParseSourceSpan,
valueSpan: ParseSourceSpan|undefined, references: t.Reference[]) {
if (identifier.indexOf('-') > -1) {
this.reportError(`"-" is not allowed in reference names`, sourceSpan);
@ -459,7 +460,7 @@ class HtmlAstToIvyAst implements html.Visitor {
this.reportError(`Reference does not have a name`, sourceSpan);
}
references.push(new t.Reference(identifier, value, sourceSpan, valueSpan));
references.push(new t.Reference(identifier, value, sourceSpan, keySpan, valueSpan));
}
private parseAssignmentEvent(

View File

@ -59,8 +59,10 @@ class R3AstSourceSpans implements t.Visitor<void> {
}
visitReference(reference: t.Reference) {
this.result.push(
['Reference', humanizeSpan(reference.sourceSpan), humanizeSpan(reference.valueSpan)]);
this.result.push([
'Reference', humanizeSpan(reference.sourceSpan), humanizeSpan(reference.keySpan),
humanizeSpan(reference.valueSpan)
]);
}
visitTextAttribute(attribute: t.TextAttribute) {
@ -211,7 +213,7 @@ describe('R3 AST source spans', () => {
it('is correct for reference via #...', () => {
expectFromHtml('<ng-template #a></ng-template>').toEqual([
['Template', '<ng-template #a></ng-template>', '<ng-template #a>', '</ng-template>'],
['Reference', '#a', '<empty>'],
['Reference', '#a', 'a', '<empty>'],
]);
});
@ -220,14 +222,14 @@ describe('R3 AST source spans', () => {
[
'Template', '<ng-template #a="b"></ng-template>', '<ng-template #a="b">', '</ng-template>'
],
['Reference', '#a="b"', 'b'],
['Reference', '#a="b"', 'a', 'b'],
]);
});
it('is correct for reference via ref-...', () => {
expectFromHtml('<ng-template ref-a></ng-template>').toEqual([
['Template', '<ng-template ref-a></ng-template>', '<ng-template ref-a>', '</ng-template>'],
['Reference', 'ref-a', '<empty>'],
['Reference', 'ref-a', 'a', '<empty>'],
]);
});
@ -237,7 +239,7 @@ describe('R3 AST source spans', () => {
'Template', '<ng-template data-ref-a></ng-template>', '<ng-template data-ref-a>',
'</ng-template>'
],
['Reference', 'data-ref-a', '<empty>'],
['Reference', 'data-ref-a', 'a', '<empty>'],
]);
});
@ -405,28 +407,28 @@ describe('R3 AST source spans', () => {
it('is correct for references via #...', () => {
expectFromHtml('<div #a></div>').toEqual([
['Element', '<div #a></div>', '<div #a>', '</div>'],
['Reference', '#a', '<empty>'],
['Reference', '#a', 'a', '<empty>'],
]);
});
it('is correct for references with name', () => {
expectFromHtml('<div #a="b"></div>').toEqual([
['Element', '<div #a="b"></div>', '<div #a="b">', '</div>'],
['Reference', '#a="b"', 'b'],
['Reference', '#a="b"', 'a', 'b'],
]);
});
it('is correct for references via ref-', () => {
expectFromHtml('<div ref-a></div>').toEqual([
['Element', '<div ref-a></div>', '<div ref-a>', '</div>'],
['Reference', 'ref-a', '<empty>'],
['Reference', 'ref-a', 'a', '<empty>'],
]);
});
it('is correct for references via data-ref-', () => {
expectFromHtml('<div ref-a></div>').toEqual([
['Element', '<div ref-a></div>', '<div ref-a>', '</div>'],
['Reference', 'ref-a', '<empty>'],
['Reference', 'ref-a', 'a', '<empty>'],
]);
});
});

View File

@ -235,7 +235,7 @@ describe('definitions', () => {
it('should work for element reference declarations', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `<div #cha¦rt></div>{{chart}}`,
expectedSpanText: '#chart',
expectedSpanText: 'chart',
});
// We're already at the definition, so nothing is returned
expect(definitions).toEqual([]);
@ -249,7 +249,7 @@ describe('definitions', () => {
expect(definitions!.length).toEqual(1);
const [varDef] = definitions;
expect(varDef.textSpan).toEqual('#chart');
expect(varDef.textSpan).toEqual('chart');
});
});

View File

@ -196,7 +196,7 @@ describe('quick info', () => {
it('should work for element reference declarations', () => {
const {documentation} = expectQuickInfo({
templateOverride: `<div #¦chart></div>`,
expectedSpanText: '#chart',
expectedSpanText: 'chart',
expectedDisplayString: '(reference) chart: HTMLDivElement'
});
expect(toText(documentation))
@ -205,15 +205,23 @@ describe('quick info', () => {
'interface it also has available to it by inheritance) for manipulating <div> elements.');
});
it('should work for directive references', () => {
expectQuickInfo({
templateOverride: `<div string-model #dir¦Ref="stringModel"></div>`,
expectedSpanText: 'dirRef',
expectedDisplayString: '(reference) dirRef: StringModel'
});
});
it('should work for ref- syntax', () => {
expectQuickInfo({
templateOverride: `<div ref-ch¦art></div>`,
expectedSpanText: 'ref-chart',
expectedSpanText: 'chart',
expectedDisplayString: '(reference) chart: HTMLDivElement'
});
expectQuickInfo({
templateOverride: `<div data-ref-ch¦art></div>`,
expectedSpanText: 'data-ref-chart',
expectedSpanText: 'chart',
expectedDisplayString: '(reference) chart: HTMLDivElement'
});
});