feat(compiler): support unary operators for more accurate type checking (#37918)

Prior to this change, the unary + and - operators would be parsed as `x - 0`
and `0 - x` respectively. The runtime semantics of these expressions are
equivalent, however they may introduce inaccurate template type checking
errors as the literal type is lost, for example:

```ts
@Component({
  template: `<button [disabled]="isAdjacent(-1)"></button>`
})
export class Example {
  isAdjacent(direction: -1 | 1): boolean { return false; }
}
```

would incorrectly report a type-check error:

> error TS2345: Argument of type 'number' is not assignable to parameter
  of type '-1 | 1'.

Additionally, the translated expression for the unary + operator would be
considered as arithmetic expression with an incompatible left-hand side:

> error TS2362: The left-hand side of an arithmetic operation must be of
  type 'any', 'number', 'bigint' or an enum type.

To resolve this issues, the implicit transformation should be avoided.
This commit adds a new unary AST node to represent these expressions,
allowing for more accurate type-checking.

Fixes #20845
Fixes #36178

PR Close #37918
This commit is contained in:
JoostK
2020-07-04 01:52:40 +02:00
committed by Misko Hevery
parent e7da4040d6
commit 874792dc43
23 changed files with 313 additions and 23 deletions

View File

@ -7,7 +7,7 @@
*/
import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeofExpr, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler';
import {LocalizedString} from '@angular/compiler/src/output/output_ast';
import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast';
import * as ts from 'typescript';
import {DefaultImportRecorder, ImportRewriter, NOOP_DEFAULT_IMPORT_RECORDER, NoopImportRewriter} from '../../imports';
@ -24,6 +24,11 @@ export class Context {
}
}
const UNARY_OPERATORS = new Map<UnaryOperator, ts.PrefixUnaryOperator>([
[UnaryOperator.Minus, ts.SyntaxKind.MinusToken],
[UnaryOperator.Plus, ts.SyntaxKind.PlusToken],
]);
const BINARY_OPERATORS = new Map<BinaryOperator, ts.BinaryOperator>([
[BinaryOperator.And, ts.SyntaxKind.AmpersandAmpersandToken],
[BinaryOperator.Bigger, ts.SyntaxKind.GreaterThanToken],
@ -361,6 +366,14 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
undefined, ts.createBlock(ast.statements.map(stmt => stmt.visitStatement(this, context))));
}
visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: Context): ts.Expression {
if (!UNARY_OPERATORS.has(ast.operator)) {
throw new Error(`Unknown unary operator: ${UnaryOperator[ast.operator]}`);
}
return ts.createPrefix(
UNARY_OPERATORS.get(ast.operator)!, ast.expr.visitExpression(this, context));
}
visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: Context): ts.Expression {
if (!BINARY_OPERATORS.has(ast.operator)) {
throw new Error(`Unknown binary operator: ${BinaryOperator[ast.operator]}`);
@ -567,6 +580,10 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
throw new Error('Method not implemented.');
}
visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: Context) {
throw new Error('Method not implemented.');
}
visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: Context) {
throw new Error('Method not implemented.');
}

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler';
import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, Unary} from '@angular/compiler';
import * as ts from 'typescript';
import {TypeCheckingConfig} from '../api';
@ -17,7 +17,12 @@ export const NULL_AS_ANY =
ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
const UNDEFINED = ts.createIdentifier('undefined');
const BINARY_OPS = new Map<string, ts.SyntaxKind>([
const UNARY_OPS = new Map<string, ts.PrefixUnaryOperator>([
['+', ts.SyntaxKind.PlusToken],
['-', ts.SyntaxKind.MinusToken],
]);
const BINARY_OPS = new Map<string, ts.BinaryOperator>([
['+', ts.SyntaxKind.PlusToken],
['-', ts.SyntaxKind.MinusToken],
['<', ts.SyntaxKind.LessThanToken],
@ -74,6 +79,17 @@ class AstTranslator implements AstVisitor {
return ast.visit(this);
}
visitUnary(ast: Unary): ts.Expression {
const expr = this.translate(ast.expr);
const op = UNARY_OPS.get(ast.operator);
if (op === undefined) {
throw new Error(`Unsupported Unary.operator: ${ast.operator}`);
}
const node = wrapForDiagnostics(ts.createPrefix(op, expr));
addParseSpanInfo(node, ast.sourceSpan);
return node;
}
visitBinary(ast: Binary): ts.Expression {
const lhs = wrapForDiagnostics(this.translate(ast.left));
const rhs = wrapForDiagnostics(this.translate(ast.right));
@ -81,7 +97,7 @@ class AstTranslator implements AstVisitor {
if (op === undefined) {
throw new Error(`Unsupported Binary.operation: ${ast.operation}`);
}
const node = ts.createBinary(lhs, op as any, rhs);
const node = ts.createBinary(lhs, op, rhs);
addParseSpanInfo(node, ast.sourceSpan);
return node;
}
@ -314,6 +330,9 @@ class VeSafeLhsInferenceBugDetector implements AstVisitor {
return ast.receiver.visit(VeSafeLhsInferenceBugDetector.SINGLETON);
}
visitUnary(ast: Unary): boolean {
return ast.expr.visit(this);
}
visitBinary(ast: Binary): boolean {
return ast.left.visit(this) || ast.right.visit(this);
}

View File

@ -248,6 +248,17 @@ runInEachFileSystem(() => {
expect(messages).toEqual([]);
});
it('should treat unary operators as literal types', () => {
const messages = diagnose(`{{ test(-1) + test(+1) + test(-2) }}`, `
class TestComponent {
test(value: -1 | 1): number { return value; }
}`);
expect(messages).toEqual([
`TestComponent.html(1, 31): Argument of type '-2' is not assignable to parameter of type '1 | -1'.`,
]);
});
describe('outputs', () => {
it('should produce a diagnostic for directive outputs', () => {
const messages = diagnose(

View File

@ -10,6 +10,10 @@ import {tcb, TestDeclaration} from './test_utils';
describe('type check blocks diagnostics', () => {
describe('parse spans', () => {
it('should annotate unary ops', () => {
expect(tcbWithSpans('{{ -a }}')).toContain('(-((ctx).a /*4,5*/) /*4,5*/) /*3,5*/');
});
it('should annotate binary ops', () => {
expect(tcbWithSpans('{{ a + b }}'))
.toContain('(((ctx).a /*3,4*/) /*3,4*/) + (((ctx).b /*7,8*/) /*7,8*/) /*3,8*/');

View File

@ -31,6 +31,11 @@ describe('type check blocks', () => {
expect(tcb(TEMPLATE)).toContain('((((ctx).a))!);');
});
it('should handle unary - operator', () => {
const TEMPLATE = `{{-1}}`;
expect(tcb(TEMPLATE)).toContain('(-1);');
});
it('should handle keyed property access', () => {
const TEMPLATE = `{{a[b]}}`;
expect(tcb(TEMPLATE)).toContain('(((ctx).a))[((ctx).b)];');

View File

@ -7,7 +7,7 @@
*/
import {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ParseSourceFile, ParseSourceSpan, PartialModule, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler';
import {LocalizedString} from '@angular/compiler/src/output/output_ast';
import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast';
import * as ts from 'typescript';
import {error} from './util';
@ -622,6 +622,23 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
/* type */ undefined, this._visitStatements(expr.statements)));
}
visitUnaryOperatorExpr(expr: UnaryOperatorExpr):
RecordedNode<ts.UnaryExpression|ts.ParenthesizedExpression> {
let unaryOperator: ts.BinaryOperator;
switch (expr.operator) {
case UnaryOperator.Minus:
unaryOperator = ts.SyntaxKind.MinusToken;
break;
case UnaryOperator.Plus:
unaryOperator = ts.SyntaxKind.PlusToken;
break;
default:
throw new Error(`Unknown operator: ${expr.operator}`);
}
const binary = ts.createPrefix(unaryOperator, expr.expr.visitExpression(this, null));
return this.record(expr, expr.parens ? ts.createParen(binary) : binary);
}
visitBinaryOperatorExpr(expr: BinaryOperatorExpr):
RecordedNode<ts.BinaryExpression|ts.ParenthesizedExpression> {
let binaryOperator: ts.BinaryOperator;