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

fix(compiler): unclear lexer error when using private identifier in expressions (#42027) 

TypeScript supports ECMAScript private identifiers. It can happen that
developers intend to access such members from within an expression.

This currently results in an unclear error from the lexer. e.g.

```
'Parser Error: Unexpected token # at column 1 in [{{#myField}}] in C:/test.ts@5:2
```

We could improve such errors by tokenizing private identifiers similar to
how the TypeScript scanner processes them. Later we can report better
errors in the expression parser or in the typecheck block. This commit
causes all private identifier tokens to be disallowed, so it never
reaches the type checker. This is done intentionally as private
identifiers should not be considered valid Angular syntax, especially
because private fields are not guaranteed to be accessible from within
a component/directive definition (e.g. there cases where a template
function is generated outside of the class; which results in private
members not being accessible; and this results in mixed/confusing
behavior).

Fixes #36003.

PR Close #42027
This commit is contained in:
Paul Gschwendtner 2021-05-10 18:16:45 +02:00 committed by atscott
parent 63383c186d
commit 3c726c3516
5 changed files with 97 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
packages/compiler/src/expression_parser/lexer.ts
View File

@ -11,6 +11,7 @@ import * as chars from '../chars';
export enum TokenType {
Character,
Identifier,
PrivateIdentifier,
Keyword,
String,
Operator,
@ -58,6 +59,10 @@ export class Token {
return this.type == TokenType.Identifier;
}
isPrivateIdentifier(): boolean {
return this.type == TokenType.PrivateIdentifier;
}
isKeyword(): boolean {
return this.type == TokenType.Keyword;
}
@ -104,6 +109,7 @@ export class Token {
case TokenType.Identifier:
case TokenType.Keyword:
case TokenType.Operator:
case TokenType.PrivateIdentifier:
case TokenType.String:
case TokenType.Error:
return this.strValue;
@ -123,6 +129,10 @@ function newIdentifierToken(index: number, end: number, text: string): Token {
return new Token(index, end, TokenType.Identifier, 0, text);
}
function newPrivateIdentifierToken(index: number, end: number, text: string): Token {
return new Token(index, end, TokenType.PrivateIdentifier, 0, text);
}
function newKeywordToken(index: number, end: number, text: string): Token {
return new Token(index, end, TokenType.Keyword, 0, text);
}
@ -204,6 +214,7 @@ class _Scanner {
case chars.$DQ:
return this.scanString();
case chars.$HASH:
return this.scanPrivateIdentifier();
case chars.$PLUS:
case chars.$MINUS:
case chars.$STAR:
@ -279,6 +290,18 @@ class _Scanner {
newIdentifierToken(start, this.index, str);
}
/** Scans an ECMAScript private identifier. */
scanPrivateIdentifier(): Token {
const start: number = this.index;
this.advance();
if (!isIdentifierStart(this.peek)) {
return this.error('Invalid character [#]', -1);
}
while (isIdentifierPart(this.peek)) this.advance();
const identifierName: string = this.input.substring(start, this.index);
return newPrivateIdentifierToken(start, this.index, identifierName);
}
scanNumber(start: number): Token {
let simple: boolean = (this.index === start);
this.advance(); // Skip initial digit.

31
packages/compiler/src/expression_parser/parser.ts
View File

@ -548,7 +548,11 @@ export class _ParseAST {
expectIdentifierOrKeyword(): string|null {
const n = this.next;
if (!n.isIdentifier() && !n.isKeyword()) {
this.error(`Unexpected ${this.prettyPrintToken(n)}, expected identifier or keyword`);
if (n.isPrivateIdentifier()) {
this._reportErrorForPrivateIdentifier(n, 'expected identifier or keyword');
} else {
this.error(`Unexpected ${this.prettyPrintToken(n)}, expected identifier or keyword`);
}
return null;
}
this.advance();
@ -558,7 +562,12 @@ export class _ParseAST {
expectIdentifierOrKeywordOrString(): string {
const n = this.next;
if (!n.isIdentifier() && !n.isKeyword() && !n.isString()) {
this.error(`Unexpected ${this.prettyPrintToken(n)}, expected identifier, keyword, or string`);
if (n.isPrivateIdentifier()) {
this._reportErrorForPrivateIdentifier(n, 'expected identifier, keyword or string');
} else {
this.error(
`Unexpected ${this.prettyPrintToken(n)}, expected identifier, keyword, or string`);
}
return '';
}
this.advance();
@ -899,6 +908,10 @@ export class _ParseAST {
this.advance();
return new LiteralPrimitive(this.span(start), this.sourceSpan(start), literalValue);
} else if (this.next.isPrivateIdentifier()) {
this._reportErrorForPrivateIdentifier(this.next, null);
return new EmptyExpr(this.span(start), this.sourceSpan(start));
} else if (this.index >= this.tokens.length) {
this.error(`Unexpected end of expression: ${this.input}`);
return new EmptyExpr(this.span(start), this.sourceSpan(start));
@ -1209,6 +1222,20 @@ export class _ParseAST {
`at the end of the expression`;
}
/**
* Records an error for an unexpected private identifier being discovered.
* @param token Token representing a private identifier.
* @param extraMessage Optional additional message being appended to the error.
*/
private _reportErrorForPrivateIdentifier(token: Token, extraMessage: string|null) {
let errorMessage =
`Private identifiers are not supported. Unexpected private identifier: ${token}`;
if (extraMessage !== null) {
errorMessage += `, ${extraMessage}`;
}
this.error(errorMessage);
}
/**
* Error recovery should skip tokens until it encounters a recovery point.
*

36
packages/compiler/test/expression_parser/lexer_spec.ts
View File

@ -47,6 +47,13 @@ function expectIdentifierToken(token: any, index: number, end: number, identifie
expect(token.toString()).toEqual(identifier);
}
function expectPrivateIdentifierToken(token: any, index: number, end: number, identifier: string) {
expectToken(token, index, end);
expect(token.isPrivateIdentifier()).toBe(true);
expect(token.toString()).toEqual(identifier);
}
function expectKeywordToken(token: any, index: number, end: number, keyword: string) {
expectToken(token, index, end);
expect(token.isKeyword()).toBe(true);
@ -82,6 +89,31 @@ function expectErrorToken(token: Token, index: any, end: number, message: string
expectIdentifierToken(tokens[2], 2, 3, 'k');
});
it('should tokenize a private identifier', () => {
const tokens: number[] = lex('#a');
expect(tokens.length).toEqual(1);
expectPrivateIdentifierToken(tokens[0], 0, 2, '#a');
});
it('should tokenize a property access with private identifier', () => {
const tokens: number[] = lex('j.#k');
expect(tokens.length).toEqual(3);
expectIdentifierToken(tokens[0], 0, 1, 'j');
expectCharacterToken(tokens[1], 1, 2, '.');
expectPrivateIdentifierToken(tokens[2], 2, 4, '#k');
});
it('should throw an invalid character error when a hash character is discovered but ' +
'not indicating a private identifier',
() => {
expectErrorToken(
lex('#')[0], 0, 1,
`Lexer Error: Invalid character [#] at column 0 in expression [#]`);
expectErrorToken(
lex('#0')[0], 0, 1,
`Lexer Error: Invalid character [#] at column 0 in expression [#0]`);
});
it('should tokenize an operator', () => {
const tokens: number[] = lex('j-k');
expect(tokens.length).toEqual(3);
@ -248,10 +280,6 @@ function expectErrorToken(token: Token, index: any, end: number, message: string
'Lexer Error: Invalid unicode escape [\\u1\'\'b] at column 2 in expression [\'\\u1\'\'bla\']');
});
it('should tokenize hash as operator', () => {
expectOperatorToken(lex('#')[0], 0, 1, '#');
});
it('should tokenize ?. as operator', () => {
expectOperatorToken(lex('?.')[0], 0, 2, '?.');
});

11
packages/compiler/test/expression_parser/parser_spec.ts
View File

@ -120,6 +120,7 @@ describe('parser', () => {
it('should only allow identifier, string, or keyword as map key', () => {
expectActionError('{(:0}', 'expected identifier, keyword, or string');
expectActionError('{1234:0}', 'expected identifier, keyword, or string');
expectActionError('{#myField:0}', 'expected identifier, keyword or string');
});
});
@ -130,11 +131,20 @@ describe('parser', () => {
checkAction('a.a');
});
it('should error for private identifiers with implicit receiver', () => {
checkActionWithError(
'#privateField', '',
'Private identifiers are not supported. Unexpected private identifier: #privateField at column 1');
});
it('should only allow identifier or keyword as member names', () => {
checkActionWithError('x.', 'x.', 'identifier or keyword');
checkActionWithError('x.(', 'x.', 'identifier or keyword');
checkActionWithError('x. 1234', 'x.', 'identifier or keyword');
checkActionWithError('x."foo"', 'x.', 'identifier or keyword');
checkActionWithError(
'x.#privateField', 'x.',
'Private identifiers are not supported. Unexpected private identifier: #privateField, expected identifier or keyword');
});
it('should parse safe field access', () => {
@ -511,6 +521,7 @@ describe('parser', () => {
expectBindingError('"Foo"|(', 'identifier or keyword');
expectBindingError('"Foo"|1234', 'identifier or keyword');
expectBindingError('"Foo"|"uppercase"', 'identifier or keyword');
expectBindingError('"Foo"|#privateIdentifier"', 'identifier or keyword');
});
it('should parse quoted expressions', () => {

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

@ -1622,7 +1622,8 @@ Reference "#a" is defined several times ("<div #a></div><div [ERROR ->]#a></div>
describe('inline templates', () => {
it('should report an error on variables declared with #', () => {
expect(() => humanizeTplAst(parse('<div *ngIf="#a=b">', [])))
.toThrowError(/Parser Error: Unexpected token # at column 1/);
.toThrowError(
/Parser Error: Private identifiers are not supported\. Unexpected private identifier: #a at column 1/);
});
it('should parse variables via let ...', () => {