Uncalled function checks only works with single conditional (#42835)

* Uncalled function checks only works with single conditional

* fix type errors in compiler

* remove uncalled function checks with negations

* review

* fix test

* Cleanup after merge, accept baselines

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
This commit is contained in:
Jonas Hübotter
2022-02-17 11:37:55 -08:00
committed by GitHub
co-authored by Nathan Shively-Sanders
parent 53809d8ed1
commit fb1066e5d9
12 changed files with 498 additions and 47 deletions
+50 -41
View File
@@ -33203,7 +33203,7 @@ namespace ts {
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken) {
if (operator === SyntaxKind.AmpersandAmpersandToken) {
const parent = walkUpParenthesizedExpressions(node.parent);
checkTestingKnownTruthyCallableOrAwaitableType(node.left, leftType, isIfStatement(parent) ? parent.thenStatement : undefined);
checkTestingKnownTruthyCallableOrAwaitableType(node.left, isIfStatement(parent) ? parent.thenStatement : undefined);
}
checkTruthinessOfType(leftType, node.left);
}
@@ -33776,8 +33776,8 @@ namespace ts {
}
function checkConditionalExpression(node: ConditionalExpression, checkMode?: CheckMode): Type {
const type = checkTruthinessExpression(node.condition);
checkTestingKnownTruthyCallableOrAwaitableType(node.condition, type, node.whenTrue);
checkTruthinessExpression(node.condition);
checkTestingKnownTruthyCallableOrAwaitableType(node.condition, node.whenTrue);
const type1 = checkExpression(node.whenTrue, checkMode);
const type2 = checkExpression(node.whenFalse, checkMode);
return getUnionType([type1, type2], UnionReduction.Subtype);
@@ -37274,8 +37274,8 @@ namespace ts {
function checkIfStatement(node: IfStatement) {
// Grammar checking
checkGrammarStatementInAmbientContext(node);
const type = checkTruthinessExpression(node.expression);
checkTestingKnownTruthyCallableOrAwaitableType(node.expression, type, node.thenStatement);
checkTruthinessExpression(node.expression);
checkTestingKnownTruthyCallableOrAwaitableType(node.expression, node.thenStatement);
checkSourceElement(node.thenStatement);
if (node.thenStatement.kind === SyntaxKind.EmptyStatement) {
@@ -37285,48 +37285,57 @@ namespace ts {
checkSourceElement(node.elseStatement);
}
function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, type: Type, body?: Statement | Expression) {
function checkTestingKnownTruthyCallableOrAwaitableType(condExpr: Expression, body?: Statement | Expression) {
if (!strictNullChecks) return;
if (getFalsyFlags(type)) return;
const location = isBinaryExpression(condExpr) ? condExpr.right : condExpr;
if (isPropertyAccessExpression(location) && isTypeAssertion(location.expression)) {
return;
helper(condExpr, body);
while (isBinaryExpression(condExpr) && condExpr.operatorToken.kind === SyntaxKind.BarBarToken) {
condExpr = condExpr.left;
helper(condExpr, body);
}
const testedNode = isIdentifier(location) ? location
: isPropertyAccessExpression(location) ? location.name
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
: undefined;
function helper(condExpr: Expression, body: Expression | Statement | undefined) {
const location = isBinaryExpression(condExpr) &&
(condExpr.operatorToken.kind === SyntaxKind.BarBarToken || condExpr.operatorToken.kind === SyntaxKind.AmpersandAmpersandToken)
? condExpr.right
: condExpr;
const type = checkTruthinessExpression(location);
const isPropertyExpressionCast = isPropertyAccessExpression(location) && isTypeAssertion(location.expression);
if (getFalsyFlags(type) || isPropertyExpressionCast) return;
// While it technically should be invalid for any known-truthy value
// to be tested, we de-scope to functions and Promises unreferenced in
// the block as a heuristic to identify the most common bugs. There
// are too many false positives for values sourced from type
// definitions without strictNullChecks otherwise.
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
const isPromise = !!getAwaitedTypeOfPromise(type);
if (callSignatures.length === 0 && !isPromise) {
return;
}
const testedSymbol = testedNode && getSymbolAtLocation(testedNode);
if (!testedSymbol && !isPromise) {
return;
}
const isUsed = testedSymbol && isBinaryExpression(condExpr.parent) && isSymbolUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
|| testedSymbol && body && isSymbolUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
if (!isUsed) {
if (isPromise) {
errorAndMaybeSuggestAwait(
location,
/*maybeMissingAwait*/ true,
Diagnostics.This_condition_will_always_return_true_since_this_0_is_always_defined,
getTypeNameForErrorDisplay(type));
// While it technically should be invalid for any known-truthy value
// to be tested, we de-scope to functions and Promises unreferenced in
// the block as a heuristic to identify the most common bugs. There
// are too many false positives for values sourced from type
// definitions without strictNullChecks otherwise.
const callSignatures = getSignaturesOfType(type, SignatureKind.Call);
const isPromise = !!getAwaitedTypeOfPromise(type);
if (callSignatures.length === 0 && !isPromise) {
return;
}
else {
error(location, Diagnostics.This_condition_will_always_return_true_since_this_function_is_always_defined_Did_you_mean_to_call_it_instead);
const testedNode = isIdentifier(location) ? location
: isPropertyAccessExpression(location) ? location.name
: isBinaryExpression(location) && isIdentifier(location.right) ? location.right
: undefined;
const testedSymbol = testedNode && getSymbolAtLocation(testedNode);
if (!testedSymbol && !isPromise) {
return;
}
const isUsed = testedSymbol && isBinaryExpression(condExpr.parent) && isSymbolUsedInBinaryExpressionChain(condExpr.parent, testedSymbol)
|| testedSymbol && body && isSymbolUsedInConditionBody(condExpr, body, testedNode, testedSymbol);
if (!isUsed) {
if (isPromise) {
errorAndMaybeSuggestAwait(
location,
/*maybeMissingAwait*/ true,
Diagnostics.This_condition_will_always_return_true_since_this_0_is_always_defined,
getTypeNameForErrorDisplay(type));
}
else {
error(location, Diagnostics.This_condition_will_always_return_true_since_this_function_is_always_defined_Did_you_mean_to_call_it_instead);
}
}
}
}
+1 -1
View File
@@ -185,7 +185,7 @@ namespace ts {
}
export function assertNever(member: never, message = "Illegal value:", stackCrawlMark?: AnyFunction): never {
const detail = typeof member === "object" && hasProperty(member, "kind") && hasProperty(member, "pos") && formatSyntaxKind ? "SyntaxKind: " + formatSyntaxKind((member as Node).kind) : JSON.stringify(member);
const detail = typeof member === "object" && hasProperty(member, "kind") && hasProperty(member, "pos") ? "SyntaxKind: " + formatSyntaxKind((member as Node).kind) : JSON.stringify(member);
return fail(`${message} ${detail}`, stackCrawlMark || assertNever);
}
@@ -0,0 +1,81 @@
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(5,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(9,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(9,14): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(13,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(32,10): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(36,5): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(40,22): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(44,16): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
tests/cases/compiler/uncalledFunctionChecksInConditional.ts(48,22): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
==== tests/cases/compiler/uncalledFunctionChecksInConditional.ts (9 errors) ====
declare function isFoo(): boolean;
declare function isBar(): boolean;
declare const isUndefinedFoo: (() => boolean) | undefined;
if (isFoo) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo
}
if (isFoo || isBar) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo, isBar
}
if (isFoo || isFoo()) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo
}
if (isUndefinedFoo || isFoo()) {
// no error
}
if (isFoo && isFoo()) {
// no error
}
declare const x: boolean;
declare const ux: boolean | undefined;
declare const y: boolean;
declare const uy: boolean | undefined;
declare function z(): boolean;
declare const uz: (() => boolean) | undefined;
if (x || isFoo) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo
}
if (isFoo || x) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo
}
if (x || y || z() || isFoo) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo
}
if (x || uy || z || isUndefinedFoo) {
~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on z
}
if (ux || y || uz || isFoo) {
~~~~~
!!! error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
// error on isFoo
}
@@ -0,0 +1,84 @@
//// [uncalledFunctionChecksInConditional.ts]
declare function isFoo(): boolean;
declare function isBar(): boolean;
declare const isUndefinedFoo: (() => boolean) | undefined;
if (isFoo) {
// error on isFoo
}
if (isFoo || isBar) {
// error on isFoo, isBar
}
if (isFoo || isFoo()) {
// error on isFoo
}
if (isUndefinedFoo || isFoo()) {
// no error
}
if (isFoo && isFoo()) {
// no error
}
declare const x: boolean;
declare const ux: boolean | undefined;
declare const y: boolean;
declare const uy: boolean | undefined;
declare function z(): boolean;
declare const uz: (() => boolean) | undefined;
if (x || isFoo) {
// error on isFoo
}
if (isFoo || x) {
// error on isFoo
}
if (x || y || z() || isFoo) {
// error on isFoo
}
if (x || uy || z || isUndefinedFoo) {
// error on z
}
if (ux || y || uz || isFoo) {
// error on isFoo
}
//// [uncalledFunctionChecksInConditional.js]
if (isFoo) {
// error on isFoo
}
if (isFoo || isBar) {
// error on isFoo, isBar
}
if (isFoo || isFoo()) {
// error on isFoo
}
if (isUndefinedFoo || isFoo()) {
// no error
}
if (isFoo && isFoo()) {
// no error
}
if (x || isFoo) {
// error on isFoo
}
if (isFoo || x) {
// error on isFoo
}
if (x || y || z() || isFoo) {
// error on isFoo
}
if (x || uy || z || isUndefinedFoo) {
// error on z
}
if (ux || y || uz || isFoo) {
// error on isFoo
}
@@ -0,0 +1,103 @@
=== tests/cases/compiler/uncalledFunctionChecksInConditional.ts ===
declare function isFoo(): boolean;
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
declare function isBar(): boolean;
>isBar : Symbol(isBar, Decl(uncalledFunctionChecksInConditional.ts, 0, 34))
declare const isUndefinedFoo: (() => boolean) | undefined;
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
if (isFoo) {
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// error on isFoo
}
if (isFoo || isBar) {
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
>isBar : Symbol(isBar, Decl(uncalledFunctionChecksInConditional.ts, 0, 34))
// error on isFoo, isBar
}
if (isFoo || isFoo()) {
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// error on isFoo
}
if (isUndefinedFoo || isFoo()) {
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// no error
}
if (isFoo && isFoo()) {
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// no error
}
declare const x: boolean;
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
declare const ux: boolean | undefined;
>ux : Symbol(ux, Decl(uncalledFunctionChecksInConditional.ts, 25, 13))
declare const y: boolean;
>y : Symbol(y, Decl(uncalledFunctionChecksInConditional.ts, 26, 13))
declare const uy: boolean | undefined;
>uy : Symbol(uy, Decl(uncalledFunctionChecksInConditional.ts, 27, 13))
declare function z(): boolean;
>z : Symbol(z, Decl(uncalledFunctionChecksInConditional.ts, 27, 38))
declare const uz: (() => boolean) | undefined;
>uz : Symbol(uz, Decl(uncalledFunctionChecksInConditional.ts, 29, 13))
if (x || isFoo) {
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// error on isFoo
}
if (isFoo || x) {
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
// error on isFoo
}
if (x || y || z() || isFoo) {
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
>y : Symbol(y, Decl(uncalledFunctionChecksInConditional.ts, 26, 13))
>z : Symbol(z, Decl(uncalledFunctionChecksInConditional.ts, 27, 38))
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// error on isFoo
}
if (x || uy || z || isUndefinedFoo) {
>x : Symbol(x, Decl(uncalledFunctionChecksInConditional.ts, 24, 13))
>uy : Symbol(uy, Decl(uncalledFunctionChecksInConditional.ts, 27, 13))
>z : Symbol(z, Decl(uncalledFunctionChecksInConditional.ts, 27, 38))
>isUndefinedFoo : Symbol(isUndefinedFoo, Decl(uncalledFunctionChecksInConditional.ts, 2, 13))
// error on z
}
if (ux || y || uz || isFoo) {
>ux : Symbol(ux, Decl(uncalledFunctionChecksInConditional.ts, 25, 13))
>y : Symbol(y, Decl(uncalledFunctionChecksInConditional.ts, 26, 13))
>uz : Symbol(uz, Decl(uncalledFunctionChecksInConditional.ts, 29, 13))
>isFoo : Symbol(isFoo, Decl(uncalledFunctionChecksInConditional.ts, 0, 0))
// error on isFoo
}
@@ -0,0 +1,122 @@
=== tests/cases/compiler/uncalledFunctionChecksInConditional.ts ===
declare function isFoo(): boolean;
>isFoo : () => boolean
declare function isBar(): boolean;
>isBar : () => boolean
declare const isUndefinedFoo: (() => boolean) | undefined;
>isUndefinedFoo : (() => boolean) | undefined
if (isFoo) {
>isFoo : () => boolean
// error on isFoo
}
if (isFoo || isBar) {
>isFoo || isBar : () => boolean
>isFoo : () => boolean
>isBar : () => boolean
// error on isFoo, isBar
}
if (isFoo || isFoo()) {
>isFoo || isFoo() : () => boolean
>isFoo : () => boolean
>isFoo() : boolean
>isFoo : () => boolean
// error on isFoo
}
if (isUndefinedFoo || isFoo()) {
>isUndefinedFoo || isFoo() : boolean | (() => boolean)
>isUndefinedFoo : (() => boolean) | undefined
>isFoo() : boolean
>isFoo : () => boolean
// no error
}
if (isFoo && isFoo()) {
>isFoo && isFoo() : boolean
>isFoo : () => boolean
>isFoo() : boolean
>isFoo : () => boolean
// no error
}
declare const x: boolean;
>x : boolean
declare const ux: boolean | undefined;
>ux : boolean | undefined
declare const y: boolean;
>y : boolean
declare const uy: boolean | undefined;
>uy : boolean | undefined
declare function z(): boolean;
>z : () => boolean
declare const uz: (() => boolean) | undefined;
>uz : (() => boolean) | undefined
if (x || isFoo) {
>x || isFoo : true | (() => boolean)
>x : boolean
>isFoo : () => boolean
// error on isFoo
}
if (isFoo || x) {
>isFoo || x : () => boolean
>isFoo : () => boolean
>x : boolean
// error on isFoo
}
if (x || y || z() || isFoo) {
>x || y || z() || isFoo : true | (() => boolean)
>x || y || z() : boolean
>x || y : boolean
>x : boolean
>y : boolean
>z() : boolean
>z : () => boolean
>isFoo : () => boolean
// error on isFoo
}
if (x || uy || z || isUndefinedFoo) {
>x || uy || z || isUndefinedFoo : true | (() => boolean)
>x || uy || z : true | (() => boolean)
>x || uy : boolean | undefined
>x : boolean
>uy : boolean | undefined
>z : () => boolean
>isUndefinedFoo : (() => boolean) | undefined
// error on z
}
if (ux || y || uz || isFoo) {
>ux || y || uz || isFoo : true | (() => boolean)
>ux || y || uz : true | (() => boolean) | undefined
>ux || y : boolean
>ux : boolean | undefined
>y : boolean
>uz : (() => boolean) | undefined
>isFoo : () => boolean
// error on isFoo
}
@@ -248,7 +248,7 @@ tests/cases/conformance/types/unknown/unknownType2.ts(216,13): error TS2322: Typ
else {
const a: NumberEnum.A = u;
}
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
else {
+1 -1
View File
@@ -241,7 +241,7 @@ function notNotEquals(u: unknown) {
else {
const a: NumberEnum.A = u;
}
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
else {
@@ -627,7 +627,7 @@ function notNotEquals(u: unknown) {
>A : Symbol(NumberEnum.A, Decl(unknownType2.ts, 92, 17))
>u : Symbol(u, Decl(unknownType2.ts, 232, 22))
}
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
>u : Symbol(u, Decl(unknownType2.ts, 232, 22))
+1 -1
View File
@@ -679,7 +679,7 @@ function notNotEquals(u: unknown) {
>NumberEnum : any
>u : NumberEnum.A
}
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
>u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A : boolean
@@ -0,0 +1,52 @@
// @strictNullChecks: true
declare function isFoo(): boolean;
declare function isBar(): boolean;
declare const isUndefinedFoo: (() => boolean) | undefined;
if (isFoo) {
// error on isFoo
}
if (isFoo || isBar) {
// error on isFoo, isBar
}
if (isFoo || isFoo()) {
// error on isFoo
}
if (isUndefinedFoo || isFoo()) {
// no error
}
if (isFoo && isFoo()) {
// no error
}
declare const x: boolean;
declare const ux: boolean | undefined;
declare const y: boolean;
declare const uy: boolean | undefined;
declare function z(): boolean;
declare const uz: (() => boolean) | undefined;
if (x || isFoo) {
// error on isFoo
}
if (isFoo || x) {
// error on isFoo
}
if (x || y || z() || isFoo) {
// error on isFoo
}
if (x || uy || z || isUndefinedFoo) {
// error on z
}
if (ux || y || uz || isFoo) {
// error on isFoo
}
@@ -242,7 +242,7 @@ function notNotEquals(u: unknown) {
else {
const a: NumberEnum.A = u;
}
if (u !== NumberEnum.A && u !== NumberEnum.B && u !== StringEnum.A) { }
else {