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

refactor(compiler): make `keySpan` available for `BoundAttributes` (#38898) 

Though we currently have the knowledge of where the `key` for an
attribute binding 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 #38898
This commit is contained in:
Andrew Scott 2020-09-17 13:01:32 -07:00 committed by Misko Hevery
parent c8f056beb6
commit ba3f4c26bb
6 changed files with 162 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
packages/compiler/src/expression_parser/ast.ts
View File

@ -841,7 +841,10 @@ export class ParsedProperty {
constructor(
public name: string, public expression: ASTWithSource, public type: ParsedPropertyType,
public sourceSpan: ParseSourceSpan, public valueSpan?: ParseSourceSpan) {
// TODO(atscott): `keySpan` should really be required but allows `undefined` so VE does
// not need to be updated. Make `keySpan` required when VE is removed.
public sourceSpan: ParseSourceSpan, readonly keySpan: ParseSourceSpan|undefined,
public valueSpan: ParseSourceSpan|undefined) {
this.isLiteral = this.type === ParsedPropertyType.LITERAL_ATTR;
this.isAnimation = this.type === ParsedPropertyType.ANIMATION;
}
@ -896,5 +899,5 @@ export class BoundElementProperty {
constructor(
public name: string, public type: BindingType, public securityContext: SecurityContext,
public value: ASTWithSource, public unit: string|null, public sourceSpan: ParseSourceSpan,
public valueSpan?: ParseSourceSpan) {}
readonly keySpan: ParseSourceSpan|undefined, public valueSpan: ParseSourceSpan|undefined) {}
}

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

@ -43,12 +43,18 @@ export class BoundAttribute implements Node {
constructor(
public name: string, public type: BindingType, public securityContext: SecurityContext,
public value: AST, public unit: string|null, public sourceSpan: ParseSourceSpan,
public valueSpan?: ParseSourceSpan, public i18n?: I18nMeta) {}
readonly keySpan: ParseSourceSpan, public valueSpan: ParseSourceSpan|undefined,
public i18n: I18nMeta|undefined) {}
static fromBoundElementProperty(prop: BoundElementProperty, i18n?: I18nMeta) {
static fromBoundElementProperty(prop: BoundElementProperty, i18n?: I18nMeta): BoundAttribute {
if (prop.keySpan === undefined) {
throw new Error(
`Unexpected state: keySpan must be defined for bound attributes but was not for ${
prop.name}: ${prop.sourceSpan}`);
}
return new BoundAttribute(
prop.name, prop.type, prop.securityContext, prop.value, prop.unit, prop.sourceSpan,
prop.valueSpan, i18n);
prop.keySpan, prop.valueSpan, i18n);
}
visit<Result>(visitor: Visitor<Result>): Result {

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

@ -332,15 +332,26 @@ class HtmlAstToIvyAst implements html.Visitor {
const absoluteOffset =
attribute.valueSpan ? attribute.valueSpan.start.offset : srcSpan.start.offset;
function createKeySpan(srcSpan: ParseSourceSpan, prefix: string, identifier: string) {
// We need to adjust the start location for the keySpan to account for the removed 'data-'
// prefix from `normalizeAttributeName`.
const normalizationAdjustment = attribute.name.length - name.length;
const keySpanStart = srcSpan.start.moveBy(prefix.length + normalizationAdjustment);
const keySpanEnd = keySpanStart.moveBy(identifier.length);
return new ParseSourceSpan(keySpanStart, keySpanEnd, identifier);
}
const bindParts = name.match(BIND_NAME_REGEXP);
let hasBinding = false;
if (bindParts) {
hasBinding = true;
if (bindParts[KW_BIND_IDX] != null) {
const identifier = bindParts[IDENT_KW_IDX];
const keySpan = createKeySpan(srcSpan, bindParts[KW_BIND_IDX], identifier);
this.bindingParser.parsePropertyBinding(
bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, attribute.valueSpan,
matchableAttributes, parsedProperties);
identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
matchableAttributes, parsedProperties, keySpan);
} else if (bindParts[KW_LET_IDX]) {
if (isTemplateElement) {
@ -353,37 +364,41 @@ 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);
} else if (bindParts[KW_ON_IDX]) {
const events: ParsedEvent[] = [];
const identifier = bindParts[IDENT_KW_IDX];
this.bindingParser.parseEvent(
bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan || srcSpan,
matchableAttributes, events);
identifier, value, srcSpan, attribute.valueSpan || srcSpan, matchableAttributes,
events);
addEvents(events, boundEvents);
} else if (bindParts[KW_BINDON_IDX]) {
const identifier = bindParts[IDENT_KW_IDX];
const keySpan = createKeySpan(srcSpan, bindParts[KW_BINDON_IDX], identifier);
this.bindingParser.parsePropertyBinding(
bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, attribute.valueSpan,
matchableAttributes, parsedProperties);
identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
matchableAttributes, parsedProperties, keySpan);
this.parseAssignmentEvent(
bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan, matchableAttributes,
boundEvents);
identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents);
} else if (bindParts[KW_AT_IDX]) {
const keySpan = createKeySpan(srcSpan, '', name);
this.bindingParser.parseLiteralAttr(
name, value, srcSpan, absoluteOffset, attribute.valueSpan, matchableAttributes,
parsedProperties);
parsedProperties, keySpan);
} else if (bindParts[IDENT_BANANA_BOX_IDX]) {
const keySpan = createKeySpan(srcSpan, '[(', bindParts[IDENT_BANANA_BOX_IDX]);
this.bindingParser.parsePropertyBinding(
bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, absoluteOffset,
attribute.valueSpan, matchableAttributes, parsedProperties);
attribute.valueSpan, matchableAttributes, parsedProperties, keySpan);
this.parseAssignmentEvent(
bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, attribute.valueSpan,
matchableAttributes, boundEvents);
} else if (bindParts[IDENT_PROPERTY_IDX]) {
const keySpan = createKeySpan(srcSpan, '[', bindParts[IDENT_PROPERTY_IDX]);
this.bindingParser.parsePropertyBinding(
bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, absoluteOffset,
attribute.valueSpan, matchableAttributes, parsedProperties);
attribute.valueSpan, matchableAttributes, parsedProperties, keySpan);
} else if (bindParts[IDENT_EVENT_IDX]) {
const events: ParsedEvent[] = [];
@ -393,8 +408,10 @@ class HtmlAstToIvyAst implements html.Visitor {
addEvents(events, boundEvents);
}
} else {
const keySpan = createKeySpan(srcSpan, '' /* prefix */, name);
hasBinding = this.bindingParser.parsePropertyInterpolation(
name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties);
name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties,
keySpan);
}
return hasBinding;

5
packages/compiler/src/render3/view/compiler.ts
View File

@ -582,9 +582,8 @@ function createHostBindingsFunction(
// bindings with pipes. These calculates happen after this block.
let totalHostVarsCount = 0;
bindings && bindings.forEach((binding: ParsedProperty) => {
const name = binding.name;
const stylingInputWasSet =
styleBuilder.registerInputBasedOnName(name, binding.expression, binding.sourceSpan);
const stylingInputWasSet = styleBuilder.registerInputBasedOnName(
binding.name, binding.expression, hostBindingSourceSpan);
if (stylingInputWasSet) {
totalHostVarsCount += MIN_STYLING_BINDING_SLOTS_REQUIRED;
} else {

54
packages/compiler/src/template_parser/binding_parser.ts
View File

@ -62,7 +62,13 @@ export class BindingParser {
if (typeof expression === 'string') {
this.parsePropertyBinding(
propName, expression, true, sourceSpan, sourceSpan.start.offset, undefined, [],
boundProps);
// Use the `sourceSpan` for `keySpan`. This isn't really accurate, but neither is the
// sourceSpan, as it represents the sourceSpan of the host itself rather than the
// source of the host binding (which doesn't exist in the template). Regardless,
// neither of these values are used in Ivy but are only here to satisfy the function
// signature. This should likely be refactored in the future so that `sourceSpan`
// isn't being used inaccurately.
boundProps, sourceSpan);
} else {
this._reportError(
`Value of the host property binding "${
@ -155,14 +161,14 @@ export class BindingParser {
} else if (binding.value) {
const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan);
this._parsePropertyAst(
key, binding.value, sourceSpan, valueSpan, targetMatchableAttrs, targetProps);
key, binding.value, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
} else {
targetMatchableAttrs.push([key, '' /* value */]);
// Since this is a literal attribute with no RHS, source span should be
// just the key span.
this.parseLiteralAttr(
key, null /* value */, keySpan, absoluteValueOffset, undefined /* valueSpan */,
targetMatchableAttrs, targetProps);
targetMatchableAttrs, targetProps, keySpan);
}
}
}
@ -206,7 +212,9 @@ export class BindingParser {
parseLiteralAttr(
name: string, value: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number,
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
targetProps: ParsedProperty[]) {
// TODO(atscott): keySpan is only optional here so VE template parser implementation does not
// have to change This should be required when VE is removed.
targetProps: ParsedProperty[], keySpan?: ParseSourceSpan) {
if (isAnimationLabel(name)) {
name = name.substring(1);
if (value) {
@ -216,18 +224,21 @@ export class BindingParser {
sourceSpan, ParseErrorLevel.ERROR);
}
this._parseAnimation(
name, value, sourceSpan, absoluteOffset, valueSpan, targetMatchableAttrs, targetProps);
name, value, sourceSpan, absoluteOffset, keySpan, valueSpan, targetMatchableAttrs,
targetProps);
} else {
targetProps.push(new ParsedProperty(
name, this._exprParser.wrapLiteralPrimitive(value, '', absoluteOffset),
ParsedPropertyType.LITERAL_ATTR, sourceSpan, valueSpan));
ParsedPropertyType.LITERAL_ATTR, sourceSpan, keySpan, valueSpan));
}
}
parsePropertyBinding(
name: string, expression: string, isHost: boolean, sourceSpan: ParseSourceSpan,
absoluteOffset: number, valueSpan: ParseSourceSpan|undefined,
targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) {
// TODO(atscott): keySpan is only optional here so VE template parser implementation does not
// have to change This should be required when VE is removed.
targetMatchableAttrs: string[][], targetProps: ParsedProperty[], keySpan?: ParseSourceSpan) {
if (name.length === 0) {
this._reportError(`Property name is missing in binding`, sourceSpan);
}
@ -243,22 +254,25 @@ export class BindingParser {
if (isAnimationProp) {
this._parseAnimation(
name, expression, sourceSpan, absoluteOffset, valueSpan, targetMatchableAttrs,
name, expression, sourceSpan, absoluteOffset, keySpan, valueSpan, targetMatchableAttrs,
targetProps);
} else {
this._parsePropertyAst(
name, this._parseBinding(expression, isHost, valueSpan || sourceSpan, absoluteOffset),
sourceSpan, valueSpan, targetMatchableAttrs, targetProps);
sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
}
}
parsePropertyInterpolation(
name: string, value: string, sourceSpan: ParseSourceSpan,
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
targetProps: ParsedProperty[]): boolean {
// TODO(atscott): keySpan is only optional here so VE template parser implementation does not
// have to change This should be required when VE is removed.
targetProps: ParsedProperty[], keySpan?: ParseSourceSpan): boolean {
const expr = this.parseInterpolation(value, valueSpan || sourceSpan);
if (expr) {
this._parsePropertyAst(name, expr, sourceSpan, valueSpan, targetMatchableAttrs, targetProps);
this._parsePropertyAst(
name, expr, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
return true;
}
return false;
@ -266,17 +280,17 @@ export class BindingParser {
private _parsePropertyAst(
name: string, ast: ASTWithSource, sourceSpan: ParseSourceSpan,
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
targetProps: ParsedProperty[]) {
keySpan: ParseSourceSpan|undefined, valueSpan: ParseSourceSpan|undefined,
targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) {
targetMatchableAttrs.push([name, ast.source!]);
targetProps.push(
new ParsedProperty(name, ast, ParsedPropertyType.DEFAULT, sourceSpan, valueSpan));
new ParsedProperty(name, ast, ParsedPropertyType.DEFAULT, sourceSpan, keySpan, valueSpan));
}
private _parseAnimation(
name: string, expression: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number,
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
targetProps: ParsedProperty[]) {
keySpan: ParseSourceSpan|undefined, valueSpan: ParseSourceSpan|undefined,
targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) {
if (name.length === 0) {
this._reportError('Animation trigger is missing', sourceSpan);
}
@ -287,8 +301,8 @@ export class BindingParser {
const ast = this._parseBinding(
expression || 'undefined', false, valueSpan || sourceSpan, absoluteOffset);
targetMatchableAttrs.push([name, ast.source!]);
targetProps.push(
new ParsedProperty(name, ast, ParsedPropertyType.ANIMATION, sourceSpan, valueSpan));
targetProps.push(new ParsedProperty(
name, ast, ParsedPropertyType.ANIMATION, sourceSpan, keySpan, valueSpan));
}
private _parseBinding(
@ -317,7 +331,7 @@ export class BindingParser {
if (boundProp.isAnimation) {
return new BoundElementProperty(
boundProp.name, BindingType.Animation, SecurityContext.NONE, boundProp.expression, null,
boundProp.sourceSpan, boundProp.valueSpan);
boundProp.sourceSpan, boundProp.keySpan, boundProp.valueSpan);
}
let unit: string|null = null;
@ -370,7 +384,7 @@ export class BindingParser {
return new BoundElementProperty(
boundPropertyName, bindingType, securityContexts[0], boundProp.expression, unit,
boundProp.sourceSpan, boundProp.valueSpan);
boundProp.sourceSpan, boundProp.keySpan, boundProp.valueSpan);
}
parseEvent(

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

@ -65,8 +65,10 @@ class R3AstSourceSpans implements t.Visitor<void> {
}
visitBoundAttribute(attribute: t.BoundAttribute) {
this.result.push(
['BoundAttribute', humanizeSpan(attribute.sourceSpan), humanizeSpan(attribute.valueSpan)]);
this.result.push([
'BoundAttribute', humanizeSpan(attribute.sourceSpan), humanizeSpan(attribute.keySpan),
humanizeSpan(attribute.valueSpan)
]);
}
visitBoundEvent(event: t.BoundEvent) {
@ -144,28 +146,35 @@ describe('R3 AST source spans', () => {
it('is correct for bound properties', () => {
expectFromHtml('<div [someProp]="v"></div>').toEqual([
['Element', '<div [someProp]="v"></div>', '<div [someProp]="v">', '</div>'],
['BoundAttribute', '[someProp]="v"', 'v'],
['BoundAttribute', '[someProp]="v"', 'someProp', 'v'],
]);
});
it('is correct for bound properties without value', () => {
expectFromHtml('<div [someProp]></div>').toEqual([
['Element', '<div [someProp]></div>', '<div [someProp]>', '</div>'],
['BoundAttribute', '[someProp]', '<empty>'],
['BoundAttribute', '[someProp]', 'someProp', '<empty>'],
]);
});
it('is correct for bound properties via bind- ', () => {
expectFromHtml('<div bind-prop="v"></div>').toEqual([
['Element', '<div bind-prop="v"></div>', '<div bind-prop="v">', '</div>'],
['BoundAttribute', 'bind-prop="v"', 'v'],
['BoundAttribute', 'bind-prop="v"', 'prop', 'v'],
]);
});
it('is correct for bound properties via {{...}}', () => {
expectFromHtml('<div prop="{{v}}"></div>').toEqual([
['Element', '<div prop="{{v}}"></div>', '<div prop="{{v}}">', '</div>'],
['BoundAttribute', 'prop="{{v}}"', '{{v}}'],
['BoundAttribute', 'prop="{{v}}"', 'prop', '{{v}}'],
]);
});
it('is correct for bound properties via data-', () => {
expectFromHtml('<div data-prop="{{v}}"></div>').toEqual([
['Element', '<div data-prop="{{v}}"></div>', '<div data-prop="{{v}}">', '</div>'],
['BoundAttribute', 'data-prop="{{v}}"', 'prop', '{{v}}'],
]);
});
});
@ -208,6 +217,16 @@ describe('R3 AST source spans', () => {
]);
});
it('is correct for reference via data-ref-...', () => {
expectFromHtml('<ng-template data-ref-a></ng-template>').toEqual([
[
'Template', '<ng-template data-ref-a></ng-template>', '<ng-template data-ref-a>',
'</ng-template>'
],
['Reference', 'data-ref-a', '<empty>'],
]);
});
it('is correct for variables via let-...', () => {
expectFromHtml('<ng-template let-a="b"></ng-template>').toEqual([
[
@ -218,6 +237,16 @@ describe('R3 AST source spans', () => {
]);
});
it('is correct for variables via data-let-...', () => {
expectFromHtml('<ng-template data-let-a="b"></ng-template>').toEqual([
[
'Template', '<ng-template data-let-a="b"></ng-template>', '<ng-template data-let-a="b">',
'</ng-template>'
],
['Variable', 'data-let-a="b"', 'b'],
]);
});
it('is correct for attributes', () => {
expectFromHtml('<ng-template k1="v1"></ng-template>').toEqual([
[
@ -234,7 +263,7 @@ describe('R3 AST source spans', () => {
'Template', '<ng-template [k1]="v1"></ng-template>', '<ng-template [k1]="v1">',
'</ng-template>'
],
['BoundAttribute', '[k1]="v1"', 'v1'],
['BoundAttribute', '[k1]="v1"', 'k1', 'v1'],
]);
});
});
@ -252,7 +281,7 @@ describe('R3 AST source spans', () => {
'</div>'
],
['TextAttribute', 'ngFor', '<empty>'],
['BoundAttribute', '*ngFor="let item of items"', 'items'],
['BoundAttribute', '*ngFor="let item of items"', 'of', 'items'],
['Variable', 'let item ', '<empty>'],
[
'Element', '<div *ngFor="let item of items"></div>', '<div *ngFor="let item of items">',
@ -270,10 +299,28 @@ describe('R3 AST source spans', () => {
[
'Template', '<div *ngFor="item of items"></div>', '<div *ngFor="item of items">', '</div>'
],
['BoundAttribute', '*ngFor="item of items"', 'item'],
['BoundAttribute', '*ngFor="item of items"', 'items'],
['BoundAttribute', '*ngFor="item of items"', 'ngFor', 'item'],
['BoundAttribute', '*ngFor="item of items"', 'of', 'items'],
['Element', '<div *ngFor="item of items"></div>', '<div *ngFor="item of items">', '</div>'],
]);
expectFromHtml('<div *ngFor="let item of items; trackBy: trackByFn"></div>').toEqual([
[
'Template', '<div *ngFor="let item of items; trackBy: trackByFn"></div>',
'<div *ngFor="let item of items; trackBy: trackByFn">', '</div>'
],
['TextAttribute', 'ngFor', '<empty>'],
['BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'of', 'items'],
[
'BoundAttribute', '*ngFor="let item of items; trackBy: trackByFn"', 'trackBy', 'trackByFn'
],
['Variable', 'let item ', '<empty>'],
[
'Element', '<div *ngFor="let item of items; trackBy: trackByFn"></div>',
'<div *ngFor="let item of items; trackBy: trackByFn">', '</div>'
],
]);
});
it('is correct for variables via let ...', () => {
@ -288,7 +335,7 @@ describe('R3 AST source spans', () => {
it('is correct for variables via as ...', () => {
expectFromHtml('<div *ngIf="expr as local"></div>').toEqual([
['Template', '<div *ngIf="expr as local"></div>', '<div *ngIf="expr as local">', '</div>'],
['BoundAttribute', '*ngIf="expr as local"', 'expr'],
['BoundAttribute', '*ngIf="expr as local"', 'ngIf', 'expr'],
['Variable', 'ngIf="expr as local', 'ngIf'],
['Element', '<div *ngIf="expr as local"></div>', '<div *ngIf="expr as local">', '</div>'],
]);
@ -310,10 +357,17 @@ describe('R3 AST source spans', () => {
]);
});
it('is correct for bound events via data-on-', () => {
expectFromHtml('<div data-on-event="v"></div>').toEqual([
['Element', '<div data-on-event="v"></div>', '<div data-on-event="v">', '</div>'],
['BoundEvent', 'data-on-event="v"', 'v'],
]);
});
it('is correct for bound events and properties via [(...)]', () => {
expectFromHtml('<div [(prop)]="v"></div>').toEqual([
['Element', '<div [(prop)]="v"></div>', '<div [(prop)]="v">', '</div>'],
['BoundAttribute', '[(prop)]="v"', 'v'],
['BoundAttribute', '[(prop)]="v"', 'prop', 'v'],
['BoundEvent', '[(prop)]="v"', 'v'],
]);
});
@ -321,10 +375,18 @@ describe('R3 AST source spans', () => {
it('is correct for bound events and properties via bindon-', () => {
expectFromHtml('<div bindon-prop="v"></div>').toEqual([
['Element', '<div bindon-prop="v"></div>', '<div bindon-prop="v">', '</div>'],
['BoundAttribute', 'bindon-prop="v"', 'v'],
['BoundAttribute', 'bindon-prop="v"', 'prop', 'v'],
['BoundEvent', 'bindon-prop="v"', 'v'],
]);
});
it('is correct for bound events and properties via data-bindon-', () => {
expectFromHtml('<div data-bindon-prop="v"></div>').toEqual([
['Element', '<div data-bindon-prop="v"></div>', '<div data-bindon-prop="v">', '</div>'],
['BoundAttribute', 'data-bindon-prop="v"', 'prop', 'v'],
['BoundEvent', 'data-bindon-prop="v"', 'v'],
]);
});
});
describe('references', () => {
@ -348,5 +410,12 @@ describe('R3 AST source spans', () => {
['Reference', 'ref-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>'],
]);
});
});
});