Skip to content

Commit ff5d38a

Browse files
ahejlsbergahejlsberg
authored
Merge pull request #33622 from microsoft/fix33580
Error when assertion function calls aren't CFA'd
2 parents 20e2be1 + e5af71e commit ff5d38a

14 files changed

+366
-99
lines changed

‎src/compiler/binder.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,12 +1348,6 @@ namespace ts {
13481348
activeLabels!.pop();
13491349
}
13501350

1351-
function isDottedName(node: Expression): boolean {
1352-
return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword ||
1353-
node.kind === SyntaxKind.PropertyAccessExpression && isDottedName((<PropertyAccessExpression>node).expression) ||
1354-
node.kind === SyntaxKind.ParenthesizedExpression && isDottedName((<ParenthesizedExpression>node).expression);
1355-
}
1356-
13571351
function bindExpressionStatement(node: ExpressionStatement): void {
13581352
bind(node.expression);
13591353
// A top level call expression with a dotted function name and at least one argument

‎src/compiler/checker.ts

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18552,30 +18552,38 @@ namespace ts {
1855218552
getEffectiveTypeAnnotationNode(declaration as VariableDeclaration | ParameterDeclaration | PropertyDeclaration | PropertySignature));
1855318553
}
1855418554

18555-
function getExplicitTypeOfSymbol(symbol: Symbol) {
18556-
return symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.ValueModule) ||
18557-
symbol.flags & (SymbolFlags.Variable | SymbolFlags.Property) && isDeclarationWithExplicitTypeAnnotation(symbol.valueDeclaration) ?
18558-
getTypeOfSymbol(symbol) : undefined;
18555+
function getExplicitTypeOfSymbol(symbol: Symbol, diagnostic?: Diagnostic) {
18556+
if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.ValueModule)) {
18557+
return getTypeOfSymbol(symbol);
18558+
}
18559+
if (symbol.flags & (SymbolFlags.Variable | SymbolFlags.Property)) {
18560+
if (isDeclarationWithExplicitTypeAnnotation(symbol.valueDeclaration)) {
18561+
return getTypeOfSymbol(symbol);
18562+
}
18563+
if (diagnostic && symbol.valueDeclaration) {
18564+
addRelatedInfo(diagnostic, createDiagnosticForNode(symbol.valueDeclaration, Diagnostics._0_is_declared_here, symbolToString(symbol)));
18565+
}
18566+
}
1855918567
}
1856018568

1856118569
// We require the dotted function name in an assertion expression to be comprised of identifiers
1856218570
// that reference function, method, class or value module symbols; or variable, property or
1856318571
// parameter symbols with declarations that have explicit type annotations. Such references are
1856418572
// resolvable with no possibility of triggering circularities in control flow analysis.
18565-
function getTypeOfDottedName(node: Expression): Type | undefined {
18573+
function getTypeOfDottedName(node: Expression, diagnostic: Diagnostic | undefined): Type | undefined {
1856618574
if (!(node.flags & NodeFlags.InWithStatement)) {
1856718575
switch (node.kind) {
1856818576
case SyntaxKind.Identifier:
18569-
const symbol = getResolvedSymbol(<Identifier>node);
18570-
return getExplicitTypeOfSymbol(symbol.flags & SymbolFlags.Alias ? resolveAlias(symbol) : symbol);
18577+
const symbol = getExportSymbolOfValueSymbolIfExported(getResolvedSymbol(<Identifier>node));
18578+
return getExplicitTypeOfSymbol(symbol.flags & SymbolFlags.Alias ? resolveAlias(symbol) : symbol, diagnostic);
1857118579
case SyntaxKind.ThisKeyword:
1857218580
return getExplicitThisType(node);
1857318581
case SyntaxKind.PropertyAccessExpression:
18574-
const type = getTypeOfDottedName((<PropertyAccessExpression>node).expression);
18582+
const type = getTypeOfDottedName((<PropertyAccessExpression>node).expression, diagnostic);
1857518583
const prop = type && getPropertyOfType(type, (<PropertyAccessExpression>node).name.escapedText);
18576-
return prop && getExplicitTypeOfSymbol(prop);
18584+
return prop && getExplicitTypeOfSymbol(prop, diagnostic);
1857718585
case SyntaxKind.ParenthesizedExpression:
18578-
return getTypeOfDottedName((<ParenthesizedExpression>node).expression);
18586+
return getTypeOfDottedName((<ParenthesizedExpression>node).expression, diagnostic);
1857918587
}
1858018588
}
1858118589
}
@@ -18588,7 +18596,7 @@ namespace ts {
1858818596
// expressions are potential type predicate function calls. In order to avoid triggering
1858918597
// circularities in control flow analysis, we use getTypeOfDottedName when resolving the call
1859018598
// target expression of an assertion.
18591-
const funcType = node.parent.kind === SyntaxKind.ExpressionStatement ? getTypeOfDottedName(node.expression) :
18599+
const funcType = node.parent.kind === SyntaxKind.ExpressionStatement ? getTypeOfDottedName(node.expression, /*diagnostic*/ undefined) :
1859218600
node.expression.kind !== SyntaxKind.SuperKeyword ? checkNonNullExpression(node.expression) :
1859318601
undefined;
1859418602
const signatures = getSignaturesOfType(funcType && getApparentType(funcType) || unknownType, SignatureKind.Call);
@@ -24777,6 +24785,16 @@ namespace ts {
2477724785
if (returnType.flags & TypeFlags.ESSymbolLike && isSymbolOrSymbolForCall(node)) {
2477824786
return getESSymbolLikeTypeForNode(walkUpParenthesizedExpressions(node.parent));
2477924787
}
24788+
if (node.kind === SyntaxKind.CallExpression && node.parent.kind === SyntaxKind.ExpressionStatement &&
24789+
returnType.flags & TypeFlags.Void && getTypePredicateOfSignature(signature)) {
24790+
if (!isDottedName(node.expression)) {
24791+
error(node.expression, Diagnostics.Assertions_require_the_call_target_to_be_an_identifier_or_qualified_name);
24792+
}
24793+
else if (!getEffectsSignature(node)) {
24794+
const diagnostic = error(node.expression, Diagnostics.Assertions_require_every_name_in_the_call_target_to_be_declared_with_an_explicit_type_annotation);
24795+
getTypeOfDottedName(node.expression, diagnostic);
24796+
}
24797+
}
2478024798
let jsAssignmentType: Type | undefined;
2478124799
if (isInJSFile(node)) {
2478224800
const decl = getDeclarationOfExpando(node);

‎src/compiler/diagnosticMessages.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2739,6 +2739,14 @@
27392739
"category": "Error",
27402740
"code": 2774
27412741
},
2742+
"Assertions require every name in the call target to be declared with an explicit type annotation.": {
2743+
"category": "Error",
2744+
"code": 2775
2745+
},
2746+
"Assertions require the call target to be an identifier or qualified name.": {
2747+
"category": "Error",
2748+
"code": 2776
2749+
},
27422750
"Import declaration '{0}' is using private name '{1}'.": {
27432751
"category": "Error",
27442752
"code": 4000

‎src/compiler/utilities.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4110,6 +4110,12 @@ namespace ts {
41104110
return node.kind === SyntaxKind.Identifier || isPropertyAccessEntityNameExpression(node);
41114111
}
41124112

4113+
export function isDottedName(node: Expression): boolean {
4114+
return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword ||
4115+
node.kind === SyntaxKind.PropertyAccessExpression && isDottedName((<PropertyAccessExpression>node).expression) ||
4116+
node.kind === SyntaxKind.ParenthesizedExpression && isDottedName((<ParenthesizedExpression>node).expression);
4117+
}
4118+
41134119
export function isPropertyAccessEntityNameExpression(node: Node): node is PropertyAccessEntityNameExpression {
41144120
return isPropertyAccessExpression(node) && isEntityNameExpression(node.expression);
41154121
}

‎tests/baselines/reference/assertionTypePredicates1.errors.txt

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(121,15): error T
55
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(122,15): error TS1228: A type predicate is only allowed in return type position for functions and methods.
66
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(123,15): error TS1228: A type predicate is only allowed in return type position for functions and methods.
77
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(124,15): error TS1228: A type predicate is only allowed in return type position for functions and methods.
8+
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.
9+
tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(131,5): error TS2776: Assertions require the call target to be an identifier or qualified name.
10+
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.
811

912

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

@@ -147,4 +150,23 @@ tests/cases/conformance/controlFlow/assertionTypePredicates1.ts(124,15): error T
147150
~~~~~~~~~~~~~~~~~~~~~~
148151
!!! error TS1228: A type predicate is only allowed in return type position for functions and methods.
149152
}
153+
154+
function f20(x: unknown) {
155+
const assert = (value: unknown): asserts value => {}
156+
assert(typeof x === "string"); // Error
157+
~~~~~~
158+
!!! error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
159+
!!! related TS2728 tests/cases/conformance/controlFlow/assertionTypePredicates1.ts:128:11: 'assert' is declared here.
160+
const a = [assert];
161+
a[0](typeof x === "string"); // Error
162+
~~~~
163+
!!! error TS2776: Assertions require the call target to be an identifier or qualified name.
164+
const t1 = new Test();
165+
t1.assert(typeof x === "string"); // Error
166+
~~~~~~~~~
167+
!!! error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
168+
!!! related TS2728 tests/cases/conformance/controlFlow/assertionTypePredicates1.ts:132:11: 't1' is declared here.
169+
const t2: Test = new Test();
170+
t2.assert(typeof x === "string");
171+
}
150172

‎tests/baselines/reference/assertionTypePredicates1.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,17 @@ declare class Wat {
124124
get p2(): asserts this is string;
125125
set p2(x: asserts this is string);
126126
}
127+
128+
function f20(x: unknown) {
129+
const assert = (value: unknown): asserts value => {}
130+
assert(typeof x === "string"); // Error
131+
const a = [assert];
132+
a[0](typeof x === "string"); // Error
133+
const t1 = new Test();
134+
t1.assert(typeof x === "string"); // Error
135+
const t2: Test = new Test();
136+
t2.assert(typeof x === "string");
137+
}
127138

128139

129140
//// [assertionTypePredicates1.js]
@@ -250,6 +261,16 @@ var Test2 = /** @class */ (function (_super) {
250261
}
251262
return Test2;
252263
}(Test));
264+
function f20(x) {
265+
var assert = function (value) { };
266+
assert(typeof x === "string"); // Error
267+
var a = [assert];
268+
a[0](typeof x === "string"); // Error
269+
var t1 = new Test();
270+
t1.assert(typeof x === "string"); // Error
271+
var t2 = new Test();
272+
t2.assert(typeof x === "string");
273+
}
253274

254275

255276
//// [assertionTypePredicates1.d.ts]
@@ -287,3 +308,4 @@ declare class Wat {
287308
get p2(): asserts this is string;
288309
set p2(x: asserts this is string);
289310
}
311+
declare function f20(x: unknown): void;

‎tests/baselines/reference/assertionTypePredicates1.symbols

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,3 +357,46 @@ declare class Wat {
357357
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 123, 11))
358358
}
359359

360+
function f20(x: unknown) {
361+
>f20 : Symbol(f20, Decl(assertionTypePredicates1.ts, 124, 1))
362+
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))
363+
364+
const assert = (value: unknown): asserts value => {}
365+
>assert : Symbol(assert, Decl(assertionTypePredicates1.ts, 127, 9))
366+
>value : Symbol(value, Decl(assertionTypePredicates1.ts, 127, 20))
367+
>value : Symbol(value, Decl(assertionTypePredicates1.ts, 127, 20))
368+
369+
assert(typeof x === "string"); // Error
370+
>assert : Symbol(assert, Decl(assertionTypePredicates1.ts, 127, 9))
371+
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))
372+
373+
const a = [assert];
374+
>a : Symbol(a, Decl(assertionTypePredicates1.ts, 129, 9))
375+
>assert : Symbol(assert, Decl(assertionTypePredicates1.ts, 127, 9))
376+
377+
a[0](typeof x === "string"); // Error
378+
>a : Symbol(a, Decl(assertionTypePredicates1.ts, 129, 9))
379+
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))
380+
381+
const t1 = new Test();
382+
>t1 : Symbol(t1, Decl(assertionTypePredicates1.ts, 131, 9))
383+
>Test : Symbol(Test, Decl(assertionTypePredicates1.ts, 76, 1))
384+
385+
t1.assert(typeof x === "string"); // Error
386+
>t1.assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
387+
>t1 : Symbol(t1, Decl(assertionTypePredicates1.ts, 131, 9))
388+
>assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
389+
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))
390+
391+
const t2: Test = new Test();
392+
>t2 : Symbol(t2, Decl(assertionTypePredicates1.ts, 133, 9))
393+
>Test : Symbol(Test, Decl(assertionTypePredicates1.ts, 76, 1))
394+
>Test : Symbol(Test, Decl(assertionTypePredicates1.ts, 76, 1))
395+
396+
t2.assert(typeof x === "string");
397+
>t2.assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
398+
>t2 : Symbol(t2, Decl(assertionTypePredicates1.ts, 133, 9))
399+
>assert : Symbol(Test.assert, Decl(assertionTypePredicates1.ts, 78, 12))
400+
>x : Symbol(x, Decl(assertionTypePredicates1.ts, 126, 13))
401+
}
402+

‎tests/baselines/reference/assertionTypePredicates1.types

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,3 +436,66 @@ declare class Wat {
436436
>x : void
437437
}
438438

439+
function f20(x: unknown) {
440+
>f20 : (x: unknown) => void
441+
>x : unknown
442+
443+
const assert = (value: unknown): asserts value => {}
444+
>assert : (value: unknown) => asserts value
445+
>(value: unknown): asserts value => {} : (value: unknown) => asserts value
446+
>value : unknown
447+
448+
assert(typeof x === "string"); // Error
449+
>assert(typeof x === "string") : void
450+
>assert : (value: unknown) => asserts value
451+
>typeof x === "string" : boolean
452+
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
453+
>x : unknown
454+
>"string" : "string"
455+
456+
const a = [assert];
457+
>a : ((value: unknown) => asserts value)[]
458+
>[assert] : ((value: unknown) => asserts value)[]
459+
>assert : (value: unknown) => asserts value
460+
461+
a[0](typeof x === "string"); // Error
462+
>a[0](typeof x === "string") : void
463+
>a[0] : (value: unknown) => asserts value
464+
>a : ((value: unknown) => asserts value)[]
465+
>0 : 0
466+
>typeof x === "string" : boolean
467+
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
468+
>x : unknown
469+
>"string" : "string"
470+
471+
const t1 = new Test();
472+
>t1 : Test
473+
>new Test() : Test
474+
>Test : typeof Test
475+
476+
t1.assert(typeof x === "string"); // Error
477+
>t1.assert(typeof x === "string") : void
478+
>t1.assert : (value: unknown) => asserts value
479+
>t1 : Test
480+
>assert : (value: unknown) => asserts value
481+
>typeof x === "string" : boolean
482+
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
483+
>x : unknown
484+
>"string" : "string"
485+
486+
const t2: Test = new Test();
487+
>t2 : Test
488+
>new Test() : Test
489+
>Test : typeof Test
490+
491+
t2.assert(typeof x === "string");
492+
>t2.assert(typeof x === "string") : void
493+
>t2.assert : (value: unknown) => asserts value
494+
>t2 : Test
495+
>assert : (value: unknown) => asserts value
496+
>typeof x === "string" : boolean
497+
>typeof x : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
498+
>x : unknown
499+
>"string" : "string"
500+
}
501+

‎tests/baselines/reference/neverReturningFunctions1.errors.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,14 @@ tests/cases/conformance/controlFlow/neverReturningFunctions1.ts(153,5): error TS
225225
!!! error TS7027: Unreachable code detected.
226226
}
227227

228+
function f43() {
229+
const fail = (): never => { throw new Error(); };
230+
const f = [fail];
231+
fail(); // No effect (missing type annotation)
232+
f[0](); // No effect (not a dotted name)
233+
f;
234+
}
235+
228236
// Repro from #33582
229237

230238
export interface Component<T extends object = any> {

‎tests/baselines/reference/neverReturningFunctions1.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,14 @@ function f42(x: number) {
154154
x; // Unreachable
155155
}
156156

157+
function f43() {
158+
const fail = (): never => { throw new Error(); };
159+
const f = [fail];
160+
fail(); // No effect (missing type annotation)
161+
f[0](); // No effect (not a dotted name)
162+
f;
163+
}
164+
157165
// Repro from #33582
158166

159167
export interface Component<T extends object = any> {
@@ -375,6 +383,13 @@ function f42(x) {
375383
}
376384
x; // Unreachable
377385
}
386+
function f43() {
387+
var fail = function () { throw new Error(); };
388+
var f = [fail];
389+
fail(); // No effect (missing type annotation)
390+
f[0](); // No effect (not a dotted name)
391+
f;
392+
}
378393
var Component = registerComponent('test-component', {
379394
schema: {
380395
myProperty: {

0 commit comments

Comments
 (0)