Skip to content

Commit 6314116

Browse files
jonhuejonhue
committed
Uncalled function checks don't work with negation
1 parent f0a72e2 commit 6314116

11 files changed

Lines changed: 208 additions & 15 deletions

‎src/compiler/checker.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34867,7 +34867,6 @@ namespace ts {
3486734867

3486834868
function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
3486934869
if (!strictNullChecks) return;
34870-
if (getFalsyFlags(type)) return;
3487134870

3487234871
if (getAwaitedTypeOfPromise(type)) {
3487334872
errorAndMaybeSuggestAwait(
@@ -34878,7 +34877,9 @@ namespace ts {
3487834877
return;
3487934878
}
3488034879

34881-
const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
34880+
const location = isBinaryExpression(condExpr) ? condExpr.right
34881+
: isPrefixUnaryExpression(condExpr) ? condExpr.operand
34882+
: condExpr;
3488234883
const testedNode = isIdentifier(location) ? location
3488334884
: isPropertyAccessExpression(location) ? location.name
3488434885
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
@@ -34889,12 +34890,17 @@ namespace ts {
3488934890
return;
3489034891
}
3489134892

34893+
const testedType = isPrefixUnaryExpression(condExpr) ? checkExpression(location) : type;
34894+
if (typeToString(testedType).includes("undefined")) { // maybeTypeOfKind(testedType, TypeFlags.Undefined)
34895+
return;
34896+
}
34897+
3489234898
// While it technically should be invalid for any known-truthy value
3489334899
// to be tested, we de-scope to functions unrefenced in the block as a
3489434900
// heuristic to identify the most common bugs. There are too many
3489534901
// false positives for values sourced from type definitions without
3489634902
// strictNullChecks otherwise.
34897-
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
34903+
const callSignatures = getSignaturesOfType(testedType, SignatureKind.Call);
3489834904
if (callSignatures.length === 0) {
3489934905
return;
3490034906
}
@@ -34907,7 +34913,12 @@ namespace ts {
3490734913
const isUsed = isBinaryExpression(condExpr.parent) && isFunctionUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
3490834914
|| body && isFunctionUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
3490934915
if (!isUsed) {
34910-
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34916+
if (getFalsyFlags(type)) {
34917+
error(location, Diagnostics.This_condition_will_always_return_false_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34918+
}
34919+
else {
34920+
error(location, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead);
34921+
}
3491134922
}
3491234923
}
3491334924

‎src/compiler/diagnosticMessages.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3264,6 +3264,10 @@
32643264
"category": "Error",
32653265
"code": 2802
32663266
},
3267+
"This condition will always return false since the function is always defined. Did you mean to call it instead?": {
3268+
"category": "Error",
3269+
"code": 2803
3270+
},
32673271

32683272
"Import declaration '{0}' is using private name '{1}'.": {
32693273
"category": "Error",

‎src/compiler/moduleSpecifiers.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,6 @@ namespace ts.moduleSpecifiers {
469469
}
470470

471471
function tryGetModuleNameAsNodeModule({ path, isRedirect }: ModulePath, { getCanonicalFileName, sourceDirectory }: Info, host: ModuleSpecifierResolutionHost, options: CompilerOptions, packageNameOnly?: boolean): string | undefined {
472-
if (!host.fileExists || !host.readFile) {
473-
return undefined;
474-
}
475472
const parts: NodeModulePathParts = getNodeModulePathParts(path)!;
476473
if (!parts) {
477474
return undefined;
@@ -569,7 +566,6 @@ namespace ts.moduleSpecifiers {
569566
}
570567

571568
function tryGetAnyFileFromPath(host: ModuleSpecifierResolutionHost, path: string) {
572-
if (!host.fileExists) return;
573569
// We check all js, `node` and `json` extensions in addition to TS, since node module resolution would also choose those over the directory
574570
const extensions = getSupportedExtensions({ allowJs: true }, [{ extension: "node", isMixedContent: false }, { extension: "json", isMixedContent: false, scriptKind: ScriptKind.JSON }]);
575571
for (const e of extensions) {

‎src/compiler/sys.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,6 @@ namespace ts {
8383

8484
/* @internal */
8585
export function setCustomPollingValues(system: System) {
86-
if (!system.getEnvironmentVariable) {
87-
return;
88-
}
8986
const pollingIntervalChanged = setCustomLevels("TSC_WATCH_POLLINGINTERVAL", PollingInterval);
9087
pollingChunkSize = getCustomPollingBasedLevels("TSC_WATCH_POLLINGCHUNKSIZE", defaultChunkLevels) || pollingChunkSize;
9188
unchangedPollThresholds = getCustomPollingBasedLevels("TSC_WATCH_UNCHANGEDPOLLTHRESHOLDS", defaultChunkLevels) || unchangedPollThresholds;

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
tests/cases/compiler/three.ts(28,14): error TS2803: This condition will always return false since the function is always defined. Did you mean to call it instead?
2+
3+
4+
==== tests/cases/compiler/one.ts (0 errors) ====
5+
declare const y: never[] | string[];
6+
export const yThen = y.map(item => item.length);
7+
==== tests/cases/compiler/two.ts (0 errors) ====
8+
declare const y: number[][] | string[];
9+
export const yThen = y.map(item => item.length);
10+
==== tests/cases/compiler/three.ts (1 errors) ====
11+
// #42504
12+
interface ResizeObserverCallback {
13+
(entries: ResizeObserverEntry[], observer: ResizeObserver): void;
14+
}
15+
interface ResizeObserverCallback { // duplicate for effect
16+
(entries: ResizeObserverEntry[], observer: ResizeObserver): void;
17+
}
18+
19+
const resizeObserver = new ResizeObserver(([entry]) => {
20+
entry
21+
});
22+
// comment in #35501
23+
interface Callback<T> {
24+
(error: null, result: T): unknown
25+
(error: Error, result: null): unknown
26+
}
27+
28+
interface Task<T> {
29+
(callback: Callback<T>): unknown
30+
}
31+
32+
export function series<T>(tasks: Task<T>[], callback: Callback<T[]>): void {
33+
let index = 0
34+
let results: T[] = []
35+
36+
function next() {
37+
let task = tasks[index]
38+
if (!task) {
39+
~~~~
40+
!!! error TS2803: This condition will always return false since the function is always defined. Did you mean to call it instead?
41+
callback(null, results)
42+
} else {
43+
task((error, result) => {
44+
if (error) {
45+
callback(error, null)
46+
} else {
47+
// must use postfix-!, since `error` and `result` don't have a
48+
// causal relationship when the overloads are combined
49+
results.push(result!)
50+
next()
51+
}
52+
})
53+
}
54+
}
55+
next()
56+
}
57+
58+
series([
59+
cb => setTimeout(() => cb(null, 1), 300),
60+
cb => setTimeout(() => cb(null, 2), 200),
61+
cb => setTimeout(() => cb(null, 3), 100),
62+
], (error, results) => {
63+
if (error) {
64+
console.error(error)
65+
} else {
66+
console.log(results)
67+
}
68+
})
69+

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@ tests/cases/compiler/truthinessCallExpressionCoercion2.ts(14,10): error TS2774:
33
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(41,18): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
44
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(44,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
55
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(48,11): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
6-
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(65,46): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
76
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(76,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
87
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(79,10): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
98
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(99,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
109
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(109,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
1110
tests/cases/compiler/truthinessCallExpressionCoercion2.ts(112,14): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
1211

1312

14-
==== tests/cases/compiler/truthinessCallExpressionCoercion2.ts (11 errors) ====
13+
==== tests/cases/compiler/truthinessCallExpressionCoercion2.ts (10 errors) ====
1514
declare class A {
1615
static from(): string;
1716
}
@@ -87,8 +86,6 @@ tests/cases/compiler/truthinessCallExpressionCoercion2.ts(112,14): error TS2774:
8786
// error
8887
typeof window !== 'undefined' && window.console &&
8988
((window.console as any).firebug || (window.console.exception && window.console.table));
90-
~~~~~~~~~~~~~~~~~~~~~~~~
91-
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
9289
}
9390

9491
function checksPropertyAccess() {

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
tests/cases/compiler/uncalledFunctionChecksDontWorkWithNegation.ts(4,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
2+
tests/cases/compiler/uncalledFunctionChecksDontWorkWithNegation.ts(8,6): error TS2803: This condition will always return false since the function is always defined. Did you mean to call it instead?
3+
4+
5+
==== tests/cases/compiler/uncalledFunctionChecksDontWorkWithNegation.ts (2 errors) ====
6+
declare function isFoo(): boolean;
7+
declare const isUndefinedFoo: (() => boolean) | undefined;
8+
9+
if (isFoo) {
10+
~~~~~
11+
!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?
12+
// error
13+
}
14+
15+
if (!isFoo) {
16+
~~~~~
17+
!!! error TS2803: This condition will always return false since the function is always defined. Did you mean to call it instead?
18+
// error
19+
}
20+
21+
if (!isUndefinedFoo) {
22+
// no error
23+
}
24+

‎tests/baselines/reference/uncalledFunctionChecksDontWorkWithNegation.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//// [uncalledFunctionChecksDontWorkWithNegation.ts]
2+
declare function isFoo(): boolean;
3+
declare const isUndefinedFoo: (() => boolean) | undefined;
4+
5+
if (isFoo) {
6+
// error
7+
}
8+
9+
if (!isFoo) {
10+
// error
11+
}
12+
13+
if (!isUndefinedFoo) {
14+
// no error
15+
}
16+
17+
18+
//// [uncalledFunctionChecksDontWorkWithNegation.js]
19+
if (isFoo) {
20+
// error
21+
}
22+
if (!isFoo) {
23+
// error
24+
}
25+
if (!isUndefinedFoo) {
26+
// no error
27+
}

‎tests/baselines/reference/uncalledFunctionChecksDontWorkWithNegation.symbols‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
=== tests/cases/compiler/uncalledFunctionChecksDontWorkWithNegation.ts ===
2+
declare function isFoo(): boolean;
3+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksDontWorkWithNegation.ts, 0, 0))
4+
5+
declare const isUndefinedFoo: (() => boolean) | undefined;
6+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksDontWorkWithNegation.ts, 1, 13))
7+
8+
if (isFoo) {
9+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksDontWorkWithNegation.ts, 0, 0))
10+
11+
// error
12+
}
13+
14+
if (!isFoo) {
15+
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksDontWorkWithNegation.ts, 0, 0))
16+
17+
// error
18+
}
19+
20+
if (!isUndefinedFoo) {
21+
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksDontWorkWithNegation.ts, 1, 13))
22+
23+
// no error
24+
}
25+

‎tests/baselines/reference/uncalledFunctionChecksDontWorkWithNegation.types‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/uncalledFunctionChecksDontWorkWithNegation.ts ===
2+
declare function isFoo(): boolean;
3+
>isFoo : () => boolean
4+
5+
declare const isUndefinedFoo: (() => boolean) | undefined;
6+
>isUndefinedFoo : (() => boolean) | undefined
7+
8+
if (isFoo) {
9+
>isFoo : () => boolean
10+
11+
// error
12+
}
13+
14+
if (!isFoo) {
15+
>!isFoo : false
16+
>isFoo : () => boolean
17+
18+
// error
19+
}
20+
21+
if (!isUndefinedFoo) {
22+
>!isUndefinedFoo : boolean
23+
>isUndefinedFoo : (() => boolean) | undefined
24+
25+
// no error
26+
}
27+

0 commit comments

Comments
 (0)