Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow truthiness checks on certain literal kinds #59003

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43948,6 +43948,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

function helper(condExpr: Expression, body: Expression | Statement | undefined) {
const location = isLogicalOrCoalescingBinaryExpression(condExpr) ? skipParentheses(condExpr.right) : condExpr;

if (isModuleExportsAccessExpression(location)) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aware this is still a draft, but function expressions will almost certainly be a mistyped IIFE, so it might be helpful to separate out a specific error message for these – maybe some sort of "Did you mean to call..." message, à la 2774?

Expand Down Expand Up @@ -44077,9 +44078,32 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function checkTruthinessOfType(type: Type, node: Node) {
node = skipParentheses(node);
if (type.flags & TypeFlags.Void) {
error(node, Diagnostics.An_expression_of_type_void_cannot_be_tested_for_truthiness);
}

switch (node.kind) {
case SyntaxKind.NumericLiteral:
// Allow e.g. while(0) or while(1)
if ((node as NumericLiteral).text === "0" || (node as NumericLiteral).text === "1") break;
// falls through
case SyntaxKind.ArrayLiteralExpression:
case SyntaxKind.ArrowFunction:
case SyntaxKind.BigIntLiteral:
case SyntaxKind.ClassExpression:
case SyntaxKind.FunctionExpression:
case SyntaxKind.JsxElement:
case SyntaxKind.JsxSelfClosingElement:
case SyntaxKind.NoSubstitutionTemplateLiteral:
case SyntaxKind.ObjectLiteralExpression:
case SyntaxKind.RegularExpressionLiteral:
case SyntaxKind.StringLiteral:
case SyntaxKind.TemplateExpression:
case SyntaxKind.VoidExpression:
error(node, Diagnostics.Truthiness_test_of_this_kind_of_expression_appears_unintentional);
}

return type;
}

Expand Down
5 changes: 4 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3907,7 +3907,10 @@
"category": "Error",
"code": 2868
},

"Truthiness test of this kind of expression appears unintentional": {
"category": "Error",
"code": 2869
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
28 changes: 28 additions & 0 deletions tests/baselines/reference/checkJsdocReturnTag1.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
returns.js(20,12): error TS2869: Truthiness test of this kind of expression appears unintentional


==== returns.js (1 errors) ====
// @ts-check
/**
* @returns {string} This comment is not currently exposed
*/
function f() {
return "hello";
}

/**
* @returns {string=} This comment is not currently exposed
*/
function f1() {
return "hello world";
}

/**
* @returns {string|number} This comment is not currently exposed
*/
function f2() {
return 5 || "hello";
~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
}

5 changes: 4 additions & 1 deletion tests/baselines/reference/checkJsdocReturnTag2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
returns.js(6,5): error TS2322: Type 'number' is not assignable to type 'string'.
returns.js(13,5): error TS2322: Type 'number | boolean' is not assignable to type 'string | number'.
Type 'boolean' is not assignable to type 'string | number'.
returns.js(13,12): error TS2869: Truthiness test of this kind of expression appears unintentional


==== returns.js (2 errors) ====
==== returns.js (3 errors) ====
// @ts-check
/**
* @returns {string} This comment is not currently exposed
Expand All @@ -22,5 +23,7 @@ returns.js(13,5): error TS2322: Type 'number | boolean' is not assignable to typ
~~~~~~
!!! error TS2322: Type 'number | boolean' is not assignable to type 'string | number'.
!!! error TS2322: Type 'boolean' is not assignable to type 'string | number'.
~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
computedPropertyNames46_ES5.ts(2,6): error TS2869: Truthiness test of this kind of expression appears unintentional


==== computedPropertyNames46_ES5.ts (1 errors) ====
var o = {
["" || 0]: 0
~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
computedPropertyNames46_ES6.ts(2,6): error TS2869: Truthiness test of this kind of expression appears unintentional


==== computedPropertyNames46_ES6.ts (1 errors) ====
var o = {
["" || 0]: 0
~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
};
23 changes: 23 additions & 0 deletions tests/baselines/reference/computedPropertyNames48_ES5.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
computedPropertyNames48_ES5.ts(16,6): error TS2869: Truthiness test of this kind of expression appears unintentional


==== computedPropertyNames48_ES5.ts (1 errors) ====
declare function extractIndexer<T>(p: { [n: number]: T }): T;

enum E { x }

var a: any;

extractIndexer({
[a]: ""
}); // Should return string

extractIndexer({
[E.x]: ""
}); // Should return string

extractIndexer({
["" || 0]: ""
~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
}); // Should return any (widened form of undefined)
2 changes: 2 additions & 0 deletions tests/baselines/reference/computedPropertyNames48_ES5.types
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ enum E { x }

var a: any;
>a : any
> : ^^^

extractIndexer({
>extractIndexer({ [a]: ""}) : string
Expand All @@ -30,6 +31,7 @@ extractIndexer({
>[a] : string
> : ^^^^^^
>a : any
> : ^^^
>"" : ""
> : ^^

Expand Down
23 changes: 23 additions & 0 deletions tests/baselines/reference/computedPropertyNames48_ES6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
computedPropertyNames48_ES6.ts(16,6): error TS2869: Truthiness test of this kind of expression appears unintentional


==== computedPropertyNames48_ES6.ts (1 errors) ====
declare function extractIndexer<T>(p: { [n: number]: T }): T;

enum E { x }

var a: any;

extractIndexer({
[a]: ""
}); // Should return string

extractIndexer({
[E.x]: ""
}); // Should return string

extractIndexer({
["" || 0]: ""
~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
}); // Should return any (widened form of undefined)
2 changes: 2 additions & 0 deletions tests/baselines/reference/computedPropertyNames48_ES6.types
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ enum E { x }

var a: any;
>a : any
> : ^^^

extractIndexer({
>extractIndexer({ [a]: ""}) : string
Expand All @@ -30,6 +31,7 @@ extractIndexer({
>[a] : string
> : ^^^^^^
>a : any
> : ^^^
>"" : ""
> : ^^

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
conditionalOperatorConditionIsNumberType.ts(27,1): error TS2869: Truthiness test of this kind of expression appears unintentional
conditionalOperatorConditionIsNumberType.ts(29,1): error TS2869: Truthiness test of this kind of expression appears unintentional
conditionalOperatorConditionIsNumberType.ts(30,1): error TS2869: Truthiness test of this kind of expression appears unintentional
conditionalOperatorConditionIsNumberType.ts(53,23): error TS2869: Truthiness test of this kind of expression appears unintentional
conditionalOperatorConditionIsNumberType.ts(55,23): error TS2869: Truthiness test of this kind of expression appears unintentional
conditionalOperatorConditionIsNumberType.ts(56,32): error TS2869: Truthiness test of this kind of expression appears unintentional


==== conditionalOperatorConditionIsNumberType.ts (6 errors) ====
//Cond ? Expr1 : Expr2, Cond is of number type, Expr1 and Expr2 have the same type
var condNumber: number;

var exprAny1: any;
var exprBoolean1: boolean;
var exprNumber1: number;
var exprString1: string;
var exprIsObject1: Object;

var exprAny2: any;
var exprBoolean2: boolean;
var exprNumber2: number;
var exprString2: string;
var exprIsObject2: Object;

//Cond is a number type variable
condNumber ? exprAny1 : exprAny2;
condNumber ? exprBoolean1 : exprBoolean2;
condNumber ? exprNumber1 : exprNumber2;
condNumber ? exprString1 : exprString2;
condNumber ? exprIsObject1 : exprIsObject2;
condNumber ? exprString1 : exprBoolean1; // Union

//Cond is a number type literal
1 ? exprAny1 : exprAny2;
0 ? exprBoolean1 : exprBoolean2;
0.123456789 ? exprNumber1 : exprNumber2;
~~~~~~~~~~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
- 10000000000000 ? exprString1 : exprString2;
1000000000000 ? exprIsObject1 : exprIsObject2;
~~~~~~~~~~~~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
10000 ? exprString1 : exprBoolean1; // Union
~~~~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional

//Cond is a number type expression
function foo() { return 1 };
var array = [1, 2, 3];

1 * 0 ? exprAny1 : exprAny2;
1 + 1 ? exprBoolean1 : exprBoolean2;
"string".length ? exprNumber1 : exprNumber2;
foo() ? exprString1 : exprString2;
foo() / array[1] ? exprIsObject1 : exprIsObject2;
foo() ? exprString1 : exprBoolean1; // Union

//Results shoud be same as Expr1 and Expr2
var resultIsAny1 = condNumber ? exprAny1 : exprAny2;
var resultIsBoolean1 = condNumber ? exprBoolean1 : exprBoolean2;
var resultIsNumber1 = condNumber ? exprNumber1 : exprNumber2;
var resultIsString1 = condNumber ? exprString1 : exprString2;
var resultIsObject1 = condNumber ? exprIsObject1 : exprIsObject2;
var resultIsStringOrBoolean1 = condNumber ? exprString1 : exprBoolean1; // Union

var resultIsAny2 = 1 ? exprAny1 : exprAny2;
var resultIsBoolean2 = 0 ? exprBoolean1 : exprBoolean2;
var resultIsNumber2 = 0.123456789 ? exprNumber1 : exprNumber2;
~~~~~~~~~~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
var resultIsString2 = - 10000000000000 ? exprString1 : exprString2;
var resultIsObject2 = 1000000000000 ? exprIsObject1 : exprIsObject2;
~~~~~~~~~~~~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional
var resultIsStringOrBoolean2 = 10000 ? exprString1 : exprBoolean1; // Union
~~~~~
!!! error TS2869: Truthiness test of this kind of expression appears unintentional

var resultIsAny3 = 1 * 0 ? exprAny1 : exprAny2;
var resultIsBoolean3 = 1 + 1 ? exprBoolean1 : exprBoolean2;
var resultIsNumber3 = "string".length ? exprNumber1 : exprNumber2;
var resultIsString3 = foo() ? exprString1 : exprString2;
var resultIsObject3 = foo() / array[1] ? exprIsObject1 : exprIsObject2;
var resultIsStringOrBoolean3 = foo() / array[1] ? exprString1 : exprBoolean1; // Union
Loading