Skip to content

Commit

Permalink
Merge pull request #33622 from microsoft/fix33580
Browse files Browse the repository at this point in the history
Error when assertion function calls aren't CFA'd
  • Loading branch information
ahejlsberg authored Sep 30, 2019
2 parents 20e2be1 + e5af71e commit ff5d38a
Show file tree
Hide file tree
Showing 14 changed files with 366 additions and 99 deletions.
6 changes: 0 additions & 6 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1348,12 +1348,6 @@ namespace ts {
activeLabels!.pop();
}

function isDottedName(node: Expression): boolean {
return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword ||
node.kind === SyntaxKind.PropertyAccessExpression && isDottedName((<PropertyAccessExpression>node).expression) ||
node.kind === SyntaxKind.ParenthesizedExpression && isDottedName((<ParenthesizedExpression>node).expression);
}

function bindExpressionStatement(node: ExpressionStatement): void {
bind(node.expression);
// A top level call expression with a dotted function name and at least one argument
Expand Down
40 changes: 29 additions & 11 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18552,30 +18552,38 @@ namespace ts {
getEffectiveTypeAnnotationNode(declaration as VariableDeclaration | ParameterDeclaration | PropertyDeclaration | PropertySignature));
}

function getExplicitTypeOfSymbol(symbol: Symbol) {
return symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.ValueModule) ||
symbol.flags & (SymbolFlags.Variable | SymbolFlags.Property) && isDeclarationWithExplicitTypeAnnotation(symbol.valueDeclaration) ?
getTypeOfSymbol(symbol) : undefined;
function getExplicitTypeOfSymbol(symbol: Symbol, diagnostic?: Diagnostic) {
if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.ValueModule)) {
return getTypeOfSymbol(symbol);
}
if (symbol.flags & (SymbolFlags.Variable | SymbolFlags.Property)) {
if (isDeclarationWithExplicitTypeAnnotation(symbol.valueDeclaration)) {
return getTypeOfSymbol(symbol);
}
if (diagnostic && symbol.valueDeclaration) {
addRelatedInfo(diagnostic, createDiagnosticForNode(symbol.valueDeclaration, Diagnostics._0_is_declared_here, symbolToString(symbol)));
}
}
}

// We require the dotted function name in an assertion expression to be comprised of identifiers
// that reference function, method, class or value module symbols; or variable, property or
// parameter symbols with declarations that have explicit type annotations. Such references are
// resolvable with no possibility of triggering circularities in control flow analysis.
function getTypeOfDottedName(node: Expression): Type | undefined {
function getTypeOfDottedName(node: Expression, diagnostic: Diagnostic | undefined): Type | undefined {
if (!(node.flags & NodeFlags.InWithStatement)) {
switch (node.kind) {
case SyntaxKind.Identifier:
const symbol = getResolvedSymbol(<Identifier>node);
return getExplicitTypeOfSymbol(symbol.flags & SymbolFlags.Alias ? resolveAlias(symbol) : symbol);
const symbol = getExportSymbolOfValueSymbolIfExported(getResolvedSymbol(<Identifier>node));
return getExplicitTypeOfSymbol(symbol.flags & SymbolFlags.Alias ? resolveAlias(symbol) : symbol, diagnostic);
case SyntaxKind.ThisKeyword:
return getExplicitThisType(node);
case SyntaxKind.PropertyAccessExpression:
const type = getTypeOfDottedName((<PropertyAccessExpression>node).expression);
const type = getTypeOfDottedName((<PropertyAccessExpression>node).expression, diagnostic);
const prop = type && getPropertyOfType(type, (<PropertyAccessExpression>node).name.escapedText);
return prop && getExplicitTypeOfSymbol(prop);
return prop && getExplicitTypeOfSymbol(prop, diagnostic);
case SyntaxKind.ParenthesizedExpression:
return getTypeOfDottedName((<ParenthesizedExpression>node).expression);
return getTypeOfDottedName((<ParenthesizedExpression>node).expression, diagnostic);
}
}
}
Expand All @@ -18588,7 +18596,7 @@ namespace ts {
// expressions are potential type predicate function calls. In order to avoid triggering
// circularities in control flow analysis, we use getTypeOfDottedName when resolving the call
// target expression of an assertion.
const funcType = node.parent.kind === SyntaxKind.ExpressionStatement ? getTypeOfDottedName(node.expression) :
const funcType = node.parent.kind === SyntaxKind.ExpressionStatement ? getTypeOfDottedName(node.expression, /*diagnostic*/ undefined) :
node.expression.kind !== SyntaxKind.SuperKeyword ? checkNonNullExpression(node.expression) :
undefined;
const signatures = getSignaturesOfType(funcType && getApparentType(funcType) || unknownType, SignatureKind.Call);
Expand Down Expand Up @@ -24777,6 +24785,16 @@ namespace ts {
if (returnType.flags & TypeFlags.ESSymbolLike && isSymbolOrSymbolForCall(node)) {
return getESSymbolLikeTypeForNode(walkUpParenthesizedExpressions(node.parent));
}
if (node.kind === SyntaxKind.CallExpression && node.parent.kind === SyntaxKind.ExpressionStatement &&
returnType.flags & TypeFlags.Void && getTypePredicateOfSignature(signature)) {
if (!isDottedName(node.expression)) {
error(node.expression, Diagnostics.Assertions_require_the_call_target_to_be_an_identifier_or_qualified_name);
}
else if (!getEffectsSignature(node)) {
const diagnostic = error(node.expression, Diagnostics.Assertions_require_every_name_in_the_call_target_to_be_declared_with_an_explicit_type_annotation);
getTypeOfDottedName(node.expression, diagnostic);
}
}
let jsAssignmentType: Type | undefined;
if (isInJSFile(node)) {
const decl = getDeclarationOfExpando(node);
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2739,6 +2739,14 @@
"category": "Error",
"code": 2774
},
"Assertions require every name in the call target to be declared with an explicit type annotation.": {
"category": "Error",
"code": 2775
},
"Assertions require the call target to be an identifier or qualified name.": {
"category": "Error",
"code": 2776
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4110,6 +4110,12 @@ namespace ts {
return node.kind === SyntaxKind.Identifier || isPropertyAccessEntityNameExpression(node);
}

export function isDottedName(node: Expression): boolean {
return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword ||
node.kind === SyntaxKind.PropertyAccessExpression && isDottedName((<PropertyAccessExpression>node).expression) ||
node.kind === SyntaxKind.ParenthesizedExpression && isDottedName((<ParenthesizedExpression>node).expression);
}

export function isPropertyAccessEntityNameExpression(node: Node): node is PropertyAccessEntityNameExpression {
return isPropertyAccessExpression(node) && isEntityNameExpression(node.expression);
}
Expand Down
24 changes: 23 additions & 1 deletion tests/baselines/reference/assertionTypePredicates1.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(121,15): error T
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(122,15): error TS1228: A type predicate is only allowed in return type position for functions and methods.
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(123,15): error TS1228: A type predicate is only allowed in return type position for functions and methods.
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(124,15): error TS1228: A type predicate is only allowed in return type position for functions and methods.
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(129,5): error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(131,5): error TS2776: Assertions require the call target to be an identifier or qualified name.
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(133,5): error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.


==== tests/cases/conformance/controlFlow/assertionTypePredicates1.ts (7 errors) ====
==== tests/cases/conformance/controlFlow/assertionTypePredicates1.ts (10 errors) ====
declare function isString(value: unknown): value is string;
declare function isArrayOfStrings(value: unknown): value is string[];

Expand Down Expand Up @@ -147,4 +150,23 @@ tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(124,15): error T
~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1228: A type predicate is only allowed in return type position for functions and methods.
}

function f20(x: unknown) {
const assert = (value: unknown): asserts value => {}
assert(typeof x === "string"); // Error
~~~~~~
!!! error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
!!! related TS2728 tests/cases/conformance/controlFlow/assertionTypePredicates1.ts:128:11: 'assert' is declared here.
const a = [assert];
a[0](typeof x === "string"); // Error
~~~~
!!! error TS2776: Assertions require the call target to be an identifier or qualified name.
const t1 = new Test();
t1.assert(typeof x === "string"); // Error
~~~~~~~~~
!!! error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
!!! related TS2728 tests/cases/conformance/controlFlow/assertionTypePredicates1.ts:132:11: 't1' is declared here.
const t2: Test = new Test();
t2.assert(typeof x === "string");
}

22 changes: 22 additions & 0 deletions tests/baselines/reference/assertionTypePredicates1.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,17 @@ declare class Wat {
get p2(): asserts this is string;
set p2(x: asserts this is string);
}

function f20(x: unknown) {
const assert = (value: unknown): asserts value => {}
assert(typeof x === "string"); // Error
const a = [assert];
a[0](typeof x === "string"); // Error
const t1 = new Test();
t1.assert(typeof x === "string"); // Error
const t2: Test = new Test();
t2.assert(typeof x === "string");
}


//// [assertionTypePredicates1.js]
Expand Down Expand Up @@ -250,6 +261,16 @@ var Test2 = /** @class */ (function (_super) {
}
return Test2;
}(Test));
function f20(x) {
var assert = function (value) { };
assert(typeof x === "string"); // Error
var a = [assert];
a[0](typeof x === "string"); // Error
var t1 = new Test();
t1.assert(typeof x === "string"); // Error
var t2 = new Test();
t2.assert(typeof x === "string");
}


//// [assertionTypePredicates1.d.ts]
Expand Down Expand Up @@ -287,3 +308,4 @@ declare class Wat {
get p2(): asserts this is string;
set p2(x: asserts this is string);
}
declare function f20(x: unknown): void;
43 changes: 43 additions & 0 deletions tests/baselines/reference/assertionTypePredicates1.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,46 @@ declare class Wat {
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 123, 11))
}

function f20(x: unknown) {
>f20 : Symbol(f20, Decl(assertionTypePredicates1.ts, 124, 1))
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))

const assert = (value: unknown): asserts value => {}
>assert : Symbol(assert, Decl(assertionTypePredicates1.ts, 127, 9))
>value : Symbol(value, Decl(assertionTypePredicates1.ts, 127, 20))
>value : Symbol(value, Decl(assertionTypePredicates1.ts, 127, 20))

assert(typeof x === "string"); // Error
>assert : Symbol(assert, Decl(assertionTypePredicates1.ts, 127, 9))
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))

const a = [assert];
>a : Symbol(a, Decl(assertionTypePredicates1.ts, 129, 9))
>assert : Symbol(assert, Decl(assertionTypePredicates1.ts, 127, 9))

a[0](typeof x === "string"); // Error
>a : Symbol(a, Decl(assertionTypePredicates1.ts, 129, 9))
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))

const t1 = new Test();
>t1 : Symbol(t1, Decl(assertionTypePredicates1.ts, 131, 9))
>Test : Symbol(Test, Decl(assertionTypePredicates1.ts, 76, 1))

t1.assert(typeof x === "string"); // Error
>t1.assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
>t1 : Symbol(t1, Decl(assertionTypePredicates1.ts, 131, 9))
>assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))

const t2: Test = new Test();
>t2 : Symbol(t2, Decl(assertionTypePredicates1.ts, 133, 9))
>Test : Symbol(Test, Decl(assertionTypePredicates1.ts, 76, 1))
>Test : Symbol(Test, Decl(assertionTypePredicates1.ts, 76, 1))

t2.assert(typeof x === "string");
>t2.assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
>t2 : Symbol(t2, Decl(assertionTypePredicates1.ts, 133, 9))
>assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))
}

63 changes: 63 additions & 0 deletions tests/baselines/reference/assertionTypePredicates1.types
Original file line number Diff line number Diff line change
Expand Up @@ -436,3 +436,66 @@ declare class Wat {
>x : void
}

function f20(x: unknown) {
>f20 : (x: unknown) => void
>x : unknown

const assert = (value: unknown): asserts value => {}
>assert : (value: unknown) => asserts value
>(value: unknown): asserts value => {} : (value: unknown) => asserts value
>value : unknown

assert(typeof x === "string"); // Error
>assert(typeof x === "string") : void
>assert : (value: unknown) => asserts value
>typeof x === "string" : boolean
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
>x : unknown
>"string" : "string"

const a = [assert];
>a : ((value: unknown) => asserts value)[]
>[assert] : ((value: unknown) => asserts value)[]
>assert : (value: unknown) => asserts value

a[0](typeof x === "string"); // Error
>a[0](typeof x === "string") : void
>a[0] : (value: unknown) => asserts value
>a : ((value: unknown) => asserts value)[]
>0 : 0
>typeof x === "string" : boolean
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
>x : unknown
>"string" : "string"

const t1 = new Test();
>t1 : Test
>new Test() : Test
>Test : typeof Test

t1.assert(typeof x === "string"); // Error
>t1.assert(typeof x === "string") : void
>t1.assert : (value: unknown) => asserts value
>t1 : Test
>assert : (value: unknown) => asserts value
>typeof x === "string" : boolean
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
>x : unknown
>"string" : "string"

const t2: Test = new Test();
>t2 : Test
>new Test() : Test
>Test : typeof Test

t2.assert(typeof x === "string");
>t2.assert(typeof x === "string") : void
>t2.assert : (value: unknown) => asserts value
>t2 : Test
>assert : (value: unknown) => asserts value
>typeof x === "string" : boolean
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
>x : unknown
>"string" : "string"
}

8 changes: 8 additions & 0 deletions tests/baselines/reference/neverReturningFunctions1.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ tests/cases/conformance/controlFlow/neverReturningFunctions1.ts(153,5): error TS
!!! error TS7027: Unreachable code detected.
}

function f43() {
const fail = (): never => { throw new Error(); };
const f = [fail];
fail(); // No effect (missing type annotation)
f[0](); // No effect (not a dotted name)
f;
}

// Repro from #33582

export interface Component<T extends object = any> {
Expand Down
15 changes: 15 additions & 0 deletions tests/baselines/reference/neverReturningFunctions1.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ function f42(x: number) {
x; // Unreachable
}

function f43() {
const fail = (): never => { throw new Error(); };
const f = [fail];
fail(); // No effect (missing type annotation)
f[0](); // No effect (not a dotted name)
f;
}

// Repro from #33582

export interface Component<T extends object = any> {
Expand Down Expand Up @@ -375,6 +383,13 @@ function f42(x) {
}
x; // Unreachable
}
function f43() {
var fail = function () { throw new Error(); };
var f = [fail];
fail(); // No effect (missing type annotation)
f[0](); // No effect (not a dotted name)
f;
}
var Component = registerComponent('test-component', {
schema: {
myProperty: {
Expand Down
Loading

0 comments on commit ff5d38a

Please sign in to comment.