diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index cfad47b653..f639db8b97 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27930,7 +27930,25 @@ namespace ts { // Grammar checking checkGrammarStatementInAmbientContext(node); - checkTruthinessExpression(node.expression); + const type = checkTruthinessExpression(node.expression); + + if (strictNullChecks && + (node.expression.kind === SyntaxKind.Identifier || + node.expression.kind === SyntaxKind.PropertyAccessExpression || + node.expression.kind === SyntaxKind.ElementAccessExpression)) { + const possiblyFalsy = !!getFalsyFlags(type); + if (!possiblyFalsy) { + // While it technically should be invalid for any known-truthy value + // to be tested, we de-scope to functions 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); + if (callSignatures.length > 0) { + error(node.expression, Diagnostics.This_condition_will_always_return_true_since_the_function_is_always_defined_Did_you_mean_to_call_it_instead); + } + } + } + checkSourceElement(node.thenStatement); if (node.thenStatement.kind === SyntaxKind.EmptyStatement) { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index b296c224d0..4d31022546 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -2689,7 +2689,10 @@ "category": "Error", "code": 2773 }, - + "This condition will always return true since the function is always defined. Did you mean to call it instead?": { + "category": "Error", + "code": 2774 + }, "Import declaration '{0}' is using private name '{1}'.": { "category": "Error", "code": 4000 diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt b/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt new file mode 100644 index 0000000000..0bd5a0cbb9 --- /dev/null +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.errors.txt @@ -0,0 +1,69 @@ +tests/cases/compiler/truthinessCallExpressionCoercion.ts(3,5): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? +tests/cases/compiler/truthinessCallExpressionCoercion.ts(7,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? +tests/cases/compiler/truthinessCallExpressionCoercion.ts(24,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? +tests/cases/compiler/truthinessCallExpressionCoercion.ts(27,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? +tests/cases/compiler/truthinessCallExpressionCoercion.ts(44,13): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + + +==== tests/cases/compiler/truthinessCallExpressionCoercion.ts (5 errors) ==== + function func() { return Math.random() > 0.5; } + + if (func) { // error + ~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + } + + function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) { + if (required) { // error + ~~~~~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + } + + if (required()) { // ok + } + + if (optional) { // ok + } + } + + function checksPropertyAndElementAccess() { + const x = { + foo: { + bar() { } + } + } + + if (x.foo.bar) { // error + ~~~~~~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + } + + if (x.foo['bar']) { // error + ~~~~~~~~~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + } + } + + function maybeBoolean(param: boolean | (() => boolean)) { + if (param) { // ok + } + } + + class Foo { + maybeIsUser?: () => boolean; + + isUser() { + return true; + } + + test() { + if (this.isUser) { // error + ~~~~~~~~~~~ +!!! error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? + } + + if (this.maybeIsUser) { // ok + } + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.js b/tests/baselines/reference/truthinessCallExpressionCoercion.js new file mode 100644 index 0000000000..87c010f350 --- /dev/null +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.js @@ -0,0 +1,94 @@ +//// [truthinessCallExpressionCoercion.ts] +function func() { return Math.random() > 0.5; } + +if (func) { // error +} + +function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) { + if (required) { // error + } + + if (required()) { // ok + } + + if (optional) { // ok + } +} + +function checksPropertyAndElementAccess() { + const x = { + foo: { + bar() { } + } + } + + if (x.foo.bar) { // error + } + + if (x.foo['bar']) { // error + } +} + +function maybeBoolean(param: boolean | (() => boolean)) { + if (param) { // ok + } +} + +class Foo { + maybeIsUser?: () => boolean; + + isUser() { + return true; + } + + test() { + if (this.isUser) { // error + } + + if (this.maybeIsUser) { // ok + } + } +} + + +//// [truthinessCallExpressionCoercion.js] +function func() { return Math.random() > 0.5; } +if (func) { // error +} +function onlyErrorsWhenNonNullable(required, optional) { + if (required) { // error + } + if (required()) { // ok + } + if (optional) { // ok + } +} +function checksPropertyAndElementAccess() { + var x = { + foo: { + bar: function () { } + } + }; + if (x.foo.bar) { // error + } + if (x.foo['bar']) { // error + } +} +function maybeBoolean(param) { + if (param) { // ok + } +} +var Foo = /** @class */ (function () { + function Foo() { + } + Foo.prototype.isUser = function () { + return true; + }; + Foo.prototype.test = function () { + if (this.isUser) { // error + } + if (this.maybeIsUser) { // ok + } + }; + return Foo; +}()); diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.symbols b/tests/baselines/reference/truthinessCallExpressionCoercion.symbols new file mode 100644 index 0000000000..d6141c4394 --- /dev/null +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.symbols @@ -0,0 +1,97 @@ +=== tests/cases/compiler/truthinessCallExpressionCoercion.ts === +function func() { return Math.random() > 0.5; } +>func : Symbol(func, Decl(truthinessCallExpressionCoercion.ts, 0, 0)) +>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) +>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) + +if (func) { // error +>func : Symbol(func, Decl(truthinessCallExpressionCoercion.ts, 0, 0)) +} + +function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) { +>onlyErrorsWhenNonNullable : Symbol(onlyErrorsWhenNonNullable, Decl(truthinessCallExpressionCoercion.ts, 3, 1)) +>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35)) +>optional : Symbol(optional, Decl(truthinessCallExpressionCoercion.ts, 5, 59)) + + if (required) { // error +>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35)) + } + + if (required()) { // ok +>required : Symbol(required, Decl(truthinessCallExpressionCoercion.ts, 5, 35)) + } + + if (optional) { // ok +>optional : Symbol(optional, Decl(truthinessCallExpressionCoercion.ts, 5, 59)) + } +} + +function checksPropertyAndElementAccess() { +>checksPropertyAndElementAccess : Symbol(checksPropertyAndElementAccess, Decl(truthinessCallExpressionCoercion.ts, 14, 1)) + + const x = { +>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9)) + + foo: { +>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15)) + + bar() { } +>bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14)) + } + } + + if (x.foo.bar) { // error +>x.foo.bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14)) +>x.foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15)) +>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9)) +>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15)) +>bar : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14)) + } + + if (x.foo['bar']) { // error +>x.foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15)) +>x : Symbol(x, Decl(truthinessCallExpressionCoercion.ts, 17, 9)) +>foo : Symbol(foo, Decl(truthinessCallExpressionCoercion.ts, 17, 15)) +>'bar' : Symbol(bar, Decl(truthinessCallExpressionCoercion.ts, 18, 14)) + } +} + +function maybeBoolean(param: boolean | (() => boolean)) { +>maybeBoolean : Symbol(maybeBoolean, Decl(truthinessCallExpressionCoercion.ts, 28, 1)) +>param : Symbol(param, Decl(truthinessCallExpressionCoercion.ts, 30, 22)) + + if (param) { // ok +>param : Symbol(param, Decl(truthinessCallExpressionCoercion.ts, 30, 22)) + } +} + +class Foo { +>Foo : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1)) + + maybeIsUser?: () => boolean; +>maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11)) + + isUser() { +>isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32)) + + return true; + } + + test() { +>test : Symbol(Foo.test, Decl(truthinessCallExpressionCoercion.ts, 40, 5)) + + if (this.isUser) { // error +>this.isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32)) +>this : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1)) +>isUser : Symbol(Foo.isUser, Decl(truthinessCallExpressionCoercion.ts, 36, 32)) + } + + if (this.maybeIsUser) { // ok +>this.maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11)) +>this : Symbol(Foo, Decl(truthinessCallExpressionCoercion.ts, 33, 1)) +>maybeIsUser : Symbol(Foo.maybeIsUser, Decl(truthinessCallExpressionCoercion.ts, 35, 11)) + } + } +} + diff --git a/tests/baselines/reference/truthinessCallExpressionCoercion.types b/tests/baselines/reference/truthinessCallExpressionCoercion.types new file mode 100644 index 0000000000..478c398406 --- /dev/null +++ b/tests/baselines/reference/truthinessCallExpressionCoercion.types @@ -0,0 +1,105 @@ +=== tests/cases/compiler/truthinessCallExpressionCoercion.ts === +function func() { return Math.random() > 0.5; } +>func : () => boolean +>Math.random() > 0.5 : boolean +>Math.random() : number +>Math.random : () => number +>Math : Math +>random : () => number +>0.5 : 0.5 + +if (func) { // error +>func : () => boolean +} + +function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) { +>onlyErrorsWhenNonNullable : (required: () => boolean, optional?: (() => boolean) | undefined) => void +>required : () => boolean +>optional : (() => boolean) | undefined + + if (required) { // error +>required : () => boolean + } + + if (required()) { // ok +>required() : boolean +>required : () => boolean + } + + if (optional) { // ok +>optional : (() => boolean) | undefined + } +} + +function checksPropertyAndElementAccess() { +>checksPropertyAndElementAccess : () => void + + const x = { +>x : { foo: { bar(): void; }; } +>{ foo: { bar() { } } } : { foo: { bar(): void; }; } + + foo: { +>foo : { bar(): void; } +>{ bar() { } } : { bar(): void; } + + bar() { } +>bar : () => void + } + } + + if (x.foo.bar) { // error +>x.foo.bar : () => void +>x.foo : { bar(): void; } +>x : { foo: { bar(): void; }; } +>foo : { bar(): void; } +>bar : () => void + } + + if (x.foo['bar']) { // error +>x.foo['bar'] : () => void +>x.foo : { bar(): void; } +>x : { foo: { bar(): void; }; } +>foo : { bar(): void; } +>'bar' : "bar" + } +} + +function maybeBoolean(param: boolean | (() => boolean)) { +>maybeBoolean : (param: boolean | (() => boolean)) => void +>param : boolean | (() => boolean) + + if (param) { // ok +>param : boolean | (() => boolean) + } +} + +class Foo { +>Foo : Foo + + maybeIsUser?: () => boolean; +>maybeIsUser : (() => boolean) | undefined + + isUser() { +>isUser : () => boolean + + return true; +>true : true + } + + test() { +>test : () => void + + if (this.isUser) { // error +>this.isUser : () => boolean +>this : this +>isUser : () => boolean + } + + if (this.maybeIsUser) { // ok +>this.maybeIsUser : (() => boolean) | undefined +>this : this +>maybeIsUser : (() => boolean) | undefined + } + } +} + diff --git a/tests/cases/compiler/truthinessCallExpressionCoercion.ts b/tests/cases/compiler/truthinessCallExpressionCoercion.ts new file mode 100644 index 0000000000..1fa3105b9e --- /dev/null +++ b/tests/cases/compiler/truthinessCallExpressionCoercion.ts @@ -0,0 +1,52 @@ +// @strictNullChecks:true + +function func() { return Math.random() > 0.5; } + +if (func) { // error +} + +function onlyErrorsWhenNonNullable(required: () => boolean, optional?: () => boolean) { + if (required) { // error + } + + if (required()) { // ok + } + + if (optional) { // ok + } +} + +function checksPropertyAndElementAccess() { + const x = { + foo: { + bar() { } + } + } + + if (x.foo.bar) { // error + } + + if (x.foo['bar']) { // error + } +} + +function maybeBoolean(param: boolean | (() => boolean)) { + if (param) { // ok + } +} + +class Foo { + maybeIsUser?: () => boolean; + + isUser() { + return true; + } + + test() { + if (this.isUser) { // error + } + + if (this.maybeIsUser) { // ok + } + } +}