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

fix(compiler): ensure that event handlers have the correct source spans (#28055) 

When template bindings are being parsed the event handlers
were receiving a source span that included the whole attribute.

Now they get a span that is focussed on the handler itself.

PR Close #28055
This commit is contained in:
Pete Bacon Darwin 2019-02-08 22:10:20 +00:00 committed by Misko Hevery
parent 497619f25d
commit cffd86260a
8 changed files with 58 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -698,7 +698,8 @@ export class ParsedEvent {
// Animation events have a phase
constructor(
public name: string, public targetOrPhase: string, public type: ParsedEventType,
public handler: AST, public sourceSpan: ParseSourceSpan) {}
public handler: AST, public sourceSpan: ParseSourceSpan,
public handlerSpan: ParseSourceSpan) {}
}
export class ParsedVariable {

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

@ -50,13 +50,15 @@ export class BoundAttribute implements Node {
export class BoundEvent implements Node {
constructor(
public name: string, public type: ParsedEventType, public handler: AST,
public target: string|null, public phase: string|null, public sourceSpan: ParseSourceSpan) {}
public target: string|null, public phase: string|null, public sourceSpan: ParseSourceSpan,
public handlerSpan: ParseSourceSpan) {}
static fromParsedEvent(event: ParsedEvent) {
const target: string|null = event.type === ParsedEventType.Regular ? event.targetOrPhase : null;
const phase: string|null =
event.type === ParsedEventType.Animation ? event.targetOrPhase : null;
return new BoundEvent(event.name, event.type, event.handler, target, phase, event.sourceSpan);
return new BoundEvent(
event.name, event.type, event.handler, target, phase, event.sourceSpan, event.handlerSpan);
}
visit<Result>(visitor: Visitor<Result>): Result { return visitor.visitBoundEvent(this); }
@ -67,7 +69,12 @@ export class Element implements Node {
public name: string, public attributes: TextAttribute[], public inputs: BoundAttribute[],
public outputs: BoundEvent[], public children: Node[], public references: Reference[],
public sourceSpan: ParseSourceSpan, public startSourceSpan: ParseSourceSpan|null,
public endSourceSpan: ParseSourceSpan|null, public i18n?: I18nAST) {}
public endSourceSpan: ParseSourceSpan|null, public i18n?: I18nAST) {
// If the element is empty then the source span should include any closing tag
if (children.length === 0 && startSourceSpan && endSourceSpan) {
this.sourceSpan = {...sourceSpan, end: endSourceSpan.end};
}
}
visit<Result>(visitor: Visitor<Result>): Result { return visitor.visitElement(this); }
}

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

@ -284,13 +284,15 @@ class HtmlAstToIvyAst implements html.Visitor {
} else if (bindParts[KW_ON_IDX]) {
const events: ParsedEvent[] = [];
this.bindingParser.parseEvent(
bindParts[IDENT_KW_IDX], value, srcSpan, matchableAttributes, events);
bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan || srcSpan,
matchableAttributes, events);
addEvents(events, boundEvents);
} else if (bindParts[KW_BINDON_IDX]) {
this.bindingParser.parsePropertyBinding(
bindParts[IDENT_KW_IDX], value, false, srcSpan, matchableAttributes, parsedProperties);
this.parseAssignmentEvent(
bindParts[IDENT_KW_IDX], value, srcSpan, matchableAttributes, boundEvents);
bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan, matchableAttributes,
boundEvents);
} else if (bindParts[KW_AT_IDX]) {
this.bindingParser.parseLiteralAttr(
name, value, srcSpan, matchableAttributes, parsedProperties);
@ -300,7 +302,8 @@ class HtmlAstToIvyAst implements html.Visitor {
bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, matchableAttributes,
parsedProperties);
this.parseAssignmentEvent(
bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, matchableAttributes, boundEvents);
bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, attribute.valueSpan,
matchableAttributes, boundEvents);
} else if (bindParts[IDENT_PROPERTY_IDX]) {
this.bindingParser.parsePropertyBinding(
@ -310,7 +313,8 @@ class HtmlAstToIvyAst implements html.Visitor {
} else if (bindParts[IDENT_EVENT_IDX]) {
const events: ParsedEvent[] = [];
this.bindingParser.parseEvent(
bindParts[IDENT_EVENT_IDX], value, srcSpan, matchableAttributes, events);
bindParts[IDENT_EVENT_IDX], value, srcSpan, attribute.valueSpan || srcSpan,
matchableAttributes, events);
addEvents(events, boundEvents);
}
} else {
@ -347,10 +351,12 @@ class HtmlAstToIvyAst implements html.Visitor {
private parseAssignmentEvent(
name: string, expression: string, sourceSpan: ParseSourceSpan,
targetMatchableAttrs: string[][], boundEvents: t.BoundEvent[]) {
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
boundEvents: t.BoundEvent[]) {
const events: ParsedEvent[] = [];
this.bindingParser.parseEvent(
`${name}Change`, `${expression}=$event`, sourceSpan, targetMatchableAttrs, events);
`${name}Change`, `${expression}=$event`, sourceSpan, valueSpan || sourceSpan,
targetMatchableAttrs, events);
addEvents(events, boundEvents);
}

3
packages/compiler/src/render3/view/template.ts
View File

@ -79,7 +79,8 @@ export function prepareEventListenerParameters(
}
const bindingExpr = convertActionBinding(
scope, bindingContext, handler, 'b', () => error('Unexpected interpolation'));
scope, bindingContext, handler, 'b', () => error('Unexpected interpolation'),
eventAst.handlerSpan);
const statements = [];
if (scope) {

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

@ -83,7 +83,8 @@ export class BindingParser {
Object.keys(dirMeta.hostListeners).forEach(propName => {
const expression = dirMeta.hostListeners[propName];
if (typeof expression === 'string') {
this.parseEvent(propName, expression, sourceSpan, [], targetEvents);
// TODO: pass a more accurate handlerSpan for this event.
this.parseEvent(propName, expression, sourceSpan, sourceSpan, [], targetEvents);
} else {
this._reportError(
`Value of the host listener "${propName}" needs to be a string representing an expression but got "${expression}" (${typeof expression})`,
@ -300,13 +301,14 @@ export class BindingParser {
}
parseEvent(
name: string, expression: string, sourceSpan: ParseSourceSpan,
name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan,
targetMatchableAttrs: string[][], targetEvents: ParsedEvent[]) {
if (isAnimationLabel(name)) {
name = name.substr(1);
this._parseAnimationEvent(name, expression, sourceSpan, targetEvents);
this._parseAnimationEvent(name, expression, sourceSpan, handlerSpan, targetEvents);
} else {
this._parseRegularEvent(name, expression, sourceSpan, targetMatchableAttrs, targetEvents);
this._parseRegularEvent(
name, expression, sourceSpan, handlerSpan, targetMatchableAttrs, targetEvents);
}
}
@ -317,7 +319,8 @@ export class BindingParser {
}
private _parseAnimationEvent(
name: string, expression: string, sourceSpan: ParseSourceSpan, targetEvents: ParsedEvent[]) {
name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan,
targetEvents: ParsedEvent[]) {
const matches = splitAtPeriod(name, [name, '']);
const eventName = matches[0];
const phase = matches[1].toLowerCase();
@ -325,9 +328,9 @@ export class BindingParser {
switch (phase) {
case 'start':
case 'done':
const ast = this._parseAction(expression, sourceSpan);
targetEvents.push(
new ParsedEvent(eventName, phase, ParsedEventType.Animation, ast, sourceSpan));
const ast = this._parseAction(expression, handlerSpan);
targetEvents.push(new ParsedEvent(
eventName, phase, ParsedEventType.Animation, ast, sourceSpan, handlerSpan));
break;
default:
@ -344,13 +347,14 @@ export class BindingParser {
}
private _parseRegularEvent(
name: string, expression: string, sourceSpan: ParseSourceSpan,
name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan,
targetMatchableAttrs: string[][], targetEvents: ParsedEvent[]) {
// long format: 'target: eventName'
const [target, eventName] = splitAtColon(name, [null !, name]);
const ast = this._parseAction(expression, sourceSpan);
const ast = this._parseAction(expression, handlerSpan);
targetMatchableAttrs.push([name !, ast.source !]);
targetEvents.push(new ParsedEvent(eventName, target, ParsedEventType.Regular, ast, sourceSpan));
targetEvents.push(
new ParsedEvent(eventName, target, ParsedEventType.Regular, ast, sourceSpan, handlerSpan));
// Don't detect directives for event names for now,
// so don't add the event name to the matchableAttrs
}

6
packages/compiler/src/template_parser/template_ast.ts
View File

@ -114,7 +114,8 @@ export class BoundEventAst implements TemplateAst {
constructor(
public name: string, public target: string|null, public phase: string|null,
public handler: AST, public sourceSpan: ParseSourceSpan) {
public handler: AST, public sourceSpan: ParseSourceSpan,
public handlerSpan: ParseSourceSpan) {
this.fullName = BoundEventAst.calcFullName(this.name, this.target, this.phase);
this.isAnimation = !!this.phase;
}
@ -134,7 +135,8 @@ export class BoundEventAst implements TemplateAst {
const target: string|null = event.type === ParsedEventType.Regular ? event.targetOrPhase : null;
const phase: string|null =
event.type === ParsedEventType.Animation ? event.targetOrPhase : null;
return new BoundEventAst(event.name, target, phase, event.handler, event.sourceSpan);
return new BoundEventAst(
event.name, target, phase, event.handler, event.sourceSpan, event.handlerSpan);
}
visit(visitor: TemplateAstVisitor, context: any): any {

17
packages/compiler/src/template_parser/template_parser.ts
View File

@ -441,13 +441,15 @@ class TemplateParseVisitor implements html.Visitor {
} else if (bindParts[KW_ON_IDX]) {
this._bindingParser.parseEvent(
bindParts[IDENT_KW_IDX], value, srcSpan, targetMatchableAttrs, boundEvents);
bindParts[IDENT_KW_IDX], value, srcSpan, attr.valueSpan || srcSpan,
targetMatchableAttrs, boundEvents);
} else if (bindParts[KW_BINDON_IDX]) {
this._bindingParser.parsePropertyBinding(
bindParts[IDENT_KW_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps);
this._parseAssignmentEvent(
bindParts[IDENT_KW_IDX], value, srcSpan, targetMatchableAttrs, boundEvents);
bindParts[IDENT_KW_IDX], value, srcSpan, attr.valueSpan || srcSpan,
targetMatchableAttrs, boundEvents);
} else if (bindParts[KW_AT_IDX]) {
this._bindingParser.parseLiteralAttr(
@ -458,7 +460,8 @@ class TemplateParseVisitor implements html.Visitor {
bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, targetMatchableAttrs,
targetProps);
this._parseAssignmentEvent(
bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, targetMatchableAttrs, boundEvents);
bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, attr.valueSpan || srcSpan,
targetMatchableAttrs, boundEvents);
} else if (bindParts[IDENT_PROPERTY_IDX]) {
this._bindingParser.parsePropertyBinding(
@ -467,7 +470,8 @@ class TemplateParseVisitor implements html.Visitor {
} else if (bindParts[IDENT_EVENT_IDX]) {
this._bindingParser.parseEvent(
bindParts[IDENT_EVENT_IDX], value, srcSpan, targetMatchableAttrs, boundEvents);
bindParts[IDENT_EVENT_IDX], value, srcSpan, attr.valueSpan || srcSpan,
targetMatchableAttrs, boundEvents);
}
} else {
hasBinding = this._bindingParser.parsePropertyInterpolation(
@ -507,10 +511,11 @@ class TemplateParseVisitor implements html.Visitor {
}
private _parseAssignmentEvent(
name: string, expression: string, sourceSpan: ParseSourceSpan,
name: string, expression: string, sourceSpan: ParseSourceSpan, valueSpan: ParseSourceSpan,
targetMatchableAttrs: string[][], targetEvents: ParsedEvent[]) {
this._bindingParser.parseEvent(
`${name}Change`, `${expression}=$event`, sourceSpan, targetMatchableAttrs, targetEvents);
`${name}Change`, `${expression}=$event`, sourceSpan, valueSpan, targetMatchableAttrs,
targetEvents);
}
private _parseDirectives(selectorMatcher: SelectorMatcher, elementCssSelector: CssSelector):

4
packages/compiler/test/template_parser/template_parser_spec.ts
View File

@ -420,7 +420,7 @@ class ArrayConsole implements Console {
expectVisitedNode(
new class extends
NullVisitor{visitEvent(ast: BoundEventAst, context: any): any{return ast;}},
new BoundEventAst('foo', 'bar', 'goo', null !, null !));
new BoundEventAst('foo', 'bar', 'goo', null !, null !, null !));
});
it('should visit BoundElementPropertyAst', () => {
@ -474,7 +474,7 @@ class ArrayConsole implements Console {
new EmbeddedTemplateAst([], [], [], [], [], [], false, [], [], 0, null !),
new ElementAst('foo', [], [], [], [], [], [], false, [], [], 0, null !, null !),
new ReferenceAst('foo', null !, 'bar', null !), new VariableAst('foo', 'bar', null !),
new BoundEventAst('foo', 'bar', 'goo', null !, null !),
new BoundEventAst('foo', 'bar', 'goo', null !, null !, null !),
new BoundElementPropertyAst('foo', null !, null !, null !, 'bar', null !),
new AttrAst('foo', 'bar', null !), new BoundTextAst(null !, 0, null !),
new TextAst('foo', 0, null !), new DirectiveAst(null !, [], [], [], 0, null !),