fix(compiler): markup lexer should not capture quotes in attribute value (#28055)

When tokenizing markup (e.g. HTML) element attributes
can have quoted or unquoted values (e.g. `a=b` or `a="b"`).
The `ATTR_VALUE` tokens were capturing the quotes, which
was inconsistent and also affected source-mapping.

Now the tokenizer captures additional `ATTR_QUOTE` tokens,
which the HTML related parsers understand and factor into their
token parsing.

PR Close #28055
This commit is contained in:
Pete Bacon Darwin 2019-02-08 22:10:20 +00:00 committed by Misko Hevery
parent e6a00be014
commit c0dac184cd
7 changed files with 52 additions and 8 deletions

View File

@ -272,8 +272,7 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
const path = findNode(this.info.htmlAst, ast.sourceSpan.start.offset);
const last = path.tail;
if (last instanceof Attribute && last.valueSpan) {
// Add 1 for the quote.
return last.valueSpan.start.offset + 1;
return last.valueSpan.start.offset;
}
return ast.sourceSpan.start.offset;
}

View File

@ -25,6 +25,7 @@ export enum TokenType {
CDATA_START,
CDATA_END,
ATTR_NAME,
ATTR_QUOTE,
ATTR_VALUE,
DOC_TYPE,
EXPANSION_FORM_START,
@ -709,23 +710,29 @@ class _Tokenizer {
}
private _consumeAttributeValue() {
this._beginToken(TokenType.ATTR_VALUE);
let value: string;
if (this._peek === chars.$SQ || this._peek === chars.$DQ) {
this._beginToken(TokenType.ATTR_QUOTE);
const quoteChar = this._peek;
this._advance();
this._endToken([String.fromCodePoint(quoteChar)]);
this._beginToken(TokenType.ATTR_VALUE);
const parts: string[] = [];
while (this._peek !== quoteChar) {
parts.push(this._readChar(true));
}
value = parts.join('');
this._endToken([this._processCarriageReturns(value)]);
this._beginToken(TokenType.ATTR_QUOTE);
this._advance();
this._endToken([String.fromCodePoint(quoteChar)]);
} else {
this._beginToken(TokenType.ATTR_VALUE);
const valueStart = this._index;
this._requireCharCodeUntilFn(isNameEnd, 1);
value = this._input.substring(valueStart, this._index);
this._endToken([this._processCarriageReturns(value)]);
}
this._endToken([this._processCarriageReturns(value)]);
}
private _consumeTagOpenEnd() {

View File

@ -326,12 +326,19 @@ class _TreeBuilder {
let end = attrName.sourceSpan.end;
let value = '';
let valueSpan: ParseSourceSpan = undefined !;
if (this._peek.type === lex.TokenType.ATTR_QUOTE) {
this._advance();
}
if (this._peek.type === lex.TokenType.ATTR_VALUE) {
const valueToken = this._advance();
value = valueToken.parts[0];
end = valueToken.sourceSpan.end;
valueSpan = valueToken.sourceSpan;
}
if (this._peek.type === lex.TokenType.ATTR_QUOTE) {
const quoteToken = this._advance();
end = quoteToken.sourceSpan.end;
}
return new html.Attribute(
fullName, value, new ParseSourceSpan(attrName.sourceSpan.start, end), valueSpan);
}

View File

@ -429,8 +429,15 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn} from './ast_spe
it('should report a value span for an attribute with a value', () => {
const ast = parser.parse('<div bar="12"></div>', 'TestComp');
const attr = (ast.rootNodes[0] as html.Element).attrs[0];
expect(attr.valueSpan !.start.offset).toEqual(10);
expect(attr.valueSpan !.end.offset).toEqual(12);
});
it('should report a value span for an unquoted attribute value', () => {
const ast = parser.parse('<div bar=12></div>', 'TestComp');
const attr = (ast.rootNodes[0] as html.Element).attrs[0];
expect(attr.valueSpan !.start.offset).toEqual(9);
expect(attr.valueSpan !.end.offset).toEqual(13);
expect(attr.valueSpan !.end.offset).toEqual(11);
});
});

View File

@ -238,11 +238,17 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a="{{v}}" b="s{{m}}e" c="s{{m//c}}e">')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, '{{v}}'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_NAME, null, 'b'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 's{{m}}e'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_NAME, null, 'c'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 's{{m//c}}e'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -270,7 +276,9 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a=\'b\'>')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '\''],
[lex.TokenType.ATTR_VALUE, 'b'],
[lex.TokenType.ATTR_QUOTE, '\''],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -280,7 +288,9 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a="b">')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 'b'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -310,7 +320,9 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a="&#65;&#x41;">')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 'AA'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -320,9 +332,13 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a="&amp" b="c&&d">')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, '&amp'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_NAME, null, 'b'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 'c&&d'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -332,7 +348,9 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a="b && c &">')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 'b && c &'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -342,7 +360,9 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
expect(tokenizeAndHumanizeParts('<t a=\'t\ne\rs\r\nt\'>')).toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '\''],
[lex.TokenType.ATTR_VALUE, 't\ne\ns\nt'],
[lex.TokenType.ATTR_QUOTE, '\''],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);
@ -1001,9 +1021,13 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
.toEqual([
[lex.TokenType.TAG_OPEN_START, null, 't'],
[lex.TokenType.ATTR_NAME, null, 'a'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_VALUE, 'b'],
[lex.TokenType.ATTR_QUOTE, '"'],
[lex.TokenType.ATTR_NAME, null, 'c'],
[lex.TokenType.ATTR_QUOTE, '\''],
[lex.TokenType.ATTR_VALUE, 'd'],
[lex.TokenType.ATTR_QUOTE, '\''],
[lex.TokenType.TAG_OPEN_END],
[lex.TokenType.EOF],
]);

View File

@ -316,7 +316,7 @@ class ExpressionVisitor extends NullTemplateVisitor {
// find the template binding that contains the position
if (!this.attr.valueSpan) return;
const valueRelativePosition = this.position - this.attr.valueSpan.start.offset - 1;
const valueRelativePosition = this.position - this.attr.valueSpan.start.offset;
const bindings = templateBindingResult.templateBindings;
const binding =
bindings.find(
@ -401,7 +401,7 @@ class ExpressionVisitor extends NullTemplateVisitor {
private get attributeValuePosition() {
if (this.attr && this.attr.valueSpan) {
return this.position - this.attr.valueSpan.start.offset - 1;
return this.position - this.attr.valueSpan.start.offset;
}
return 0;
}

View File

@ -33,7 +33,7 @@ export function locateSymbol(info: TemplateInfo): SymbolInfo|undefined {
const dinfo = diagnosticInfoFromTemplateInfo(info);
const scope = getExpressionScope(dinfo, path, inEvent);
if (attribute.valueSpan) {
const expressionOffset = attribute.valueSpan.start.offset + 1;
const expressionOffset = attribute.valueSpan.start.offset;
const result = getExpressionSymbol(
scope, ast, templatePosition - expressionOffset, info.template.query);
if (result) {