honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(compiler-cli): add keySpan to text attributes (#39613) 

Similar to #39609 and #38898, though we currently have the knowledge of where the key for an
attribute appears during parsing, we do not propagate this
information to the output AST. This means that once we produce the
template AST, we have no way of mapping a template position to the key
span alone. The best we can currently do is map back to the
sourceSpan. This presents problems downstream, specifically for the
language service, where we cannot provide correct information about a
position in a template because the AST is not granular enough.

PR Close #39613
This commit is contained in:
Andrew Scott 2020-11-09 10:25:25 -08:00 committed by atscott
parent ade6da95e4
commit 21651d362d
9 changed files with 53 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
packages/compiler/src/i18n/extractor_merger.ts
View File

@ -394,10 +394,14 @@ class _Visitor implements html.Visitor {
const nodes = this._translations.get(message);
if (nodes) {
if (nodes.length == 0) {
translatedAttributes.push(new html.Attribute(attr.name, '', attr.sourceSpan));
translatedAttributes.push(new html.Attribute(
attr.name, '', attr.sourceSpan, undefined /* keySpan */, undefined /* valueSpan */,
undefined /* i18n */));
} else if (nodes[0] instanceof html.Text) {
const value = (nodes[0] as html.Text).value;
translatedAttributes.push(new html.Attribute(attr.name, value, attr.sourceSpan));
translatedAttributes.push(new html.Attribute(
attr.name, value, attr.sourceSpan, undefined /* keySpan */,
undefined /* valueSpan */, undefined /* i18n */));
} else {
this._reportError(
el,

3
packages/compiler/src/ml_parser/ast.ts
View File

@ -53,7 +53,8 @@ export class ExpansionCase implements Node {
export class Attribute extends NodeWithI18n {
constructor(
public name: string, public value: string, sourceSpan: ParseSourceSpan,
public valueSpan?: ParseSourceSpan, i18n?: I18nMeta) {
readonly keySpan: ParseSourceSpan|undefined, public valueSpan?: ParseSourceSpan,
i18n?: I18nMeta) {
super(sourceSpan, i18n);
}
visit(visitor: Visitor, context: any): any {

20
packages/compiler/src/ml_parser/icu_ast_expander.ts
View File

@ -102,10 +102,14 @@ function _expandPluralForm(ast: html.Expansion, errors: ParseError[]): html.Elem
errors.push(...expansionResult.errors);
return new html.Element(
`ng-template`, [new html.Attribute('ngPluralCase', `${c.value}`, c.valueSourceSpan)],
`ng-template`, [new html.Attribute(
'ngPluralCase', `${c.value}`, c.valueSourceSpan, undefined /* keySpan */,
undefined /* valueSpan */, undefined /* i18n */)],
expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan);
});
const switchAttr = new html.Attribute('[ngPlural]', ast.switchValue, ast.switchValueSourceSpan);
const switchAttr = new html.Attribute(
'[ngPlural]', ast.switchValue, ast.switchValueSourceSpan, undefined /* keySpan */,
undefined /* valueSpan */, undefined /* i18n */);
return new html.Element(
'ng-container', [switchAttr], children, ast.sourceSpan, ast.sourceSpan, ast.sourceSpan);
}
@ -119,15 +123,21 @@ function _expandDefaultForm(ast: html.Expansion, errors: ParseError[]): html.Ele
if (c.value === 'other') {
// other is the default case when no values match
return new html.Element(
`ng-template`, [new html.Attribute('ngSwitchDefault', '', c.valueSourceSpan)],
`ng-template`, [new html.Attribute(
'ngSwitchDefault', '', c.valueSourceSpan, undefined /* keySpan */,
undefined /* valueSpan */, undefined /* i18n */)],
expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan);
}
return new html.Element(
`ng-template`, [new html.Attribute('ngSwitchCase', `${c.value}`, c.valueSourceSpan)],
`ng-template`, [new html.Attribute(
'ngSwitchCase', `${c.value}`, c.valueSourceSpan, undefined /* keySpan */,
undefined /* valueSpan */, undefined /* i18n */)],
expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan);
});
const switchAttr = new html.Attribute('[ngSwitch]', ast.switchValue, ast.switchValueSourceSpan);
const switchAttr = new html.Attribute(
'[ngSwitch]', ast.switchValue, ast.switchValueSourceSpan, undefined /* keySpan */,
undefined /* valueSpan */, undefined /* i18n */);
return new html.Element(
'ng-container', [switchAttr], children, ast.sourceSpan, ast.sourceSpan, ast.sourceSpan);
}

3
packages/compiler/src/ml_parser/parser.ts
View File

@ -351,9 +351,10 @@ class _TreeBuilder {
const quoteToken = this._advance();
end = quoteToken.sourceSpan.end;
}
const keySpan = new ParseSourceSpan(attrName.sourceSpan.start, attrName.sourceSpan.end);
return new html.Attribute(
fullName, value,
new ParseSourceSpan(attrName.sourceSpan.start, end, attrName.sourceSpan.fullStart),
new ParseSourceSpan(attrName.sourceSpan.start, end, attrName.sourceSpan.fullStart), keySpan,
valueSpan);
}

9
packages/compiler/src/render3/r3_ast.ts
View File

@ -30,10 +30,17 @@ export class BoundText implements Node {
}
}
/**
* Represents a text attribute in the template.
*
* `valueSpan` may not be present in cases where there is no value `<div a></div>`.
* `keySpan` may also not be present for synthetic attributes from ICU expansions.
*/
export class TextAttribute implements Node {
constructor(
public name: string, public value: string, public sourceSpan: ParseSourceSpan,
public valueSpan?: ParseSourceSpan, public i18n?: I18nMeta) {}
readonly keySpan: ParseSourceSpan|undefined, public valueSpan?: ParseSourceSpan,
public i18n?: I18nMeta) {}
visit<Result>(visitor: Visitor<Result>): Result {
return visitor.visitTextAttribute(this);
}

11
packages/compiler/src/render3/r3_template_transform.ts
View File

@ -166,7 +166,7 @@ class HtmlAstToIvyAst implements html.Visitor {
if (!hasBinding && !isTemplateBinding) {
// don't include the bindings as attributes as well in the AST
attributes.push(this.visitAttribute(attribute) as t.TextAttribute);
attributes.push(this.visitAttribute(attribute));
}
}
@ -238,7 +238,8 @@ class HtmlAstToIvyAst implements html.Visitor {
visitAttribute(attribute: html.Attribute): t.TextAttribute {
return new t.TextAttribute(
attribute.name, attribute.value, attribute.sourceSpan, attribute.valueSpan, attribute.i18n);
attribute.name, attribute.value, attribute.sourceSpan, attribute.keySpan,
attribute.valueSpan, attribute.i18n);
}
visitText(text: html.Text): t.Node {
@ -301,7 +302,8 @@ class HtmlAstToIvyAst implements html.Visitor {
const i18n = i18nPropsMeta[prop.name];
if (prop.isLiteral) {
literal.push(new t.TextAttribute(
prop.name, prop.expression.source || '', prop.sourceSpan, undefined, i18n));
prop.name, prop.expression.source || '', prop.sourceSpan, prop.keySpan, prop.valueSpan,
i18n));
} else {
// Note that validation is skipped and property mapping is disabled
// due to the fact that we need to make sure a given prop is not an
@ -501,7 +503,8 @@ class NonBindableVisitor implements html.Visitor {
visitAttribute(attribute: html.Attribute): t.TextAttribute {
return new t.TextAttribute(
attribute.name, attribute.value, attribute.sourceSpan, undefined, attribute.i18n);
attribute.name, attribute.value, attribute.sourceSpan, attribute.keySpan,
attribute.valueSpan, attribute.i18n);
}
visitText(text: html.Text): t.Text {

20
packages/compiler/test/render3/r3_ast_spans_spec.ts
View File

@ -64,8 +64,10 @@ class R3AstSourceSpans implements t.Visitor<void> {
}
visitTextAttribute(attribute: t.TextAttribute) {
this.result.push(
['TextAttribute', humanizeSpan(attribute.sourceSpan), humanizeSpan(attribute.valueSpan)]);
this.result.push([
'TextAttribute', humanizeSpan(attribute.sourceSpan), humanizeSpan(attribute.keySpan),
humanizeSpan(attribute.valueSpan)
]);
}
visitBoundAttribute(attribute: t.BoundAttribute) {
@ -132,14 +134,14 @@ describe('R3 AST source spans', () => {
it('is correct for elements with attributes', () => {
expectFromHtml('<div a="b"></div>').toEqual([
['Element', '<div a="b"></div>', '<div a="b">', '</div>'],
['TextAttribute', 'a="b"', 'b'],
['TextAttribute', 'a="b"', 'a', 'b'],
]);
});
it('is correct for elements with attributes without value', () => {
expectFromHtml('<div a></div>').toEqual([
['Element', '<div a></div>', '<div a>', '</div>'],
['TextAttribute', 'a', '<empty>'],
['TextAttribute', 'a', 'a', '<empty>'],
]);
});
});
@ -193,7 +195,7 @@ describe('R3 AST source spans', () => {
it('is correct for * directives', () => {
expectFromHtml('<div *ngIf></div>').toEqual([
['Template', '<div *ngIf></div>', '<div *ngIf>', '</div>'],
['TextAttribute', 'ngIf', '<empty>'],
['TextAttribute', 'ngIf', 'ngIf', '<empty>'],
['Element', '<div *ngIf></div>', '<div *ngIf>', '</div>'],
]);
});
@ -263,7 +265,7 @@ describe('R3 AST source spans', () => {
'Template', '<ng-template k1="v1"></ng-template>', '<ng-template k1="v1">',
'</ng-template>'
],
['TextAttribute', 'k1="v1"', 'v1'],
['TextAttribute', 'k1="v1"', 'k1', 'v1'],
]);
});
@ -290,7 +292,7 @@ describe('R3 AST source spans', () => {
'Template', '<div *ngFor="let item of items"></div>', '<div *ngFor="let item of items">',
'</div>'
],
['TextAttribute', 'ngFor', '<empty>'],
['TextAttribute', 'ngFor', 'ngFor', '<empty>'],
['BoundAttribute', 'of items', 'of', 'items'],
['Variable', 'let item ', 'item', '<empty>'],
[
@ -319,7 +321,7 @@ describe('R3 AST source spans', () => {
'Template', '<div *ngFor="let item of items; trackBy: trackByFn"></div>',
'<div *ngFor="let item of items; trackBy: trackByFn">', '</div>'
],
['TextAttribute', 'ngFor', '<empty>'],
['TextAttribute', 'ngFor', 'ngFor', '<empty>'],
['BoundAttribute', 'of items; ', 'of', 'items'],
['BoundAttribute', 'trackBy: trackByFn', 'trackBy', 'trackByFn'],
['Variable', 'let item ', 'item', '<empty>'],
@ -334,7 +336,7 @@ describe('R3 AST source spans', () => {
it('is correct for variables via let ...', () => {
expectFromHtml('<div *ngIf="let a=b"></div>').toEqual([
['Template', '<div *ngIf="let a=b"></div>', '<div *ngIf="let a=b">', '</div>'],
['TextAttribute', 'ngIf', '<empty>'],
['TextAttribute', 'ngIf', 'ngIf', '<empty>'],
['Variable', 'let a=b', 'a', 'b'],
['Element', '<div *ngIf="let a=b"></div>', '<div *ngIf="let a=b">', '</div>'],
]);

2
packages/language-service/ivy/test/definitions_spec.ts
View File

@ -135,7 +135,7 @@ describe('definitions', () => {
it('should work for text inputs', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `<test-comp tcN¦ame="name"></test-comp>`,
expectedSpanText: 'tcName="name"',
expectedSpanText: 'tcName',
});
expect(definitions!.length).toEqual(1);

2
packages/language-service/ivy/test/quick_info_spec.ts
View File

@ -295,7 +295,7 @@ describe('quick info', () => {
it('should find input binding on text attribute', () => {
expectQuickInfo({
templateOverride: `<test-comp tcN¦ame="title"></test-comp>`,
expectedSpanText: 'tcName="title"',
expectedSpanText: 'tcName',
expectedDisplayString: '(property) TestComponent.name: string'
});
});