From cc8a8824cf3371478932e2011576357ee6d918fc Mon Sep 17 00:00:00 2001 From: Renuga Saraswathy Date: Mon, 10 Aug 2020 18:13:42 +0530 Subject: [PATCH 1/4] fix(eslint-plugin): [no-unnecessary-condition] added handling for unary negation expression --- .../src/rules/no-unnecessary-condition.ts | 32 +++++++++++++++++ .../rules/no-unnecessary-condition.test.ts | 34 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 0103919e14ae..c723599e5581 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -166,6 +166,14 @@ export default createRule({ * if the type of the node is always true or always false, it's not necessary. */ function checkNode(node: TSESTree.Expression): void { + // Check if the node is Unary Negation expression and handle it + if ( + node.type === AST_NODE_TYPES.UnaryExpression && + node.operator === '!' + ) { + return checkIfUnaryNegationExpressionIsNecessaryConditional(node); + } + // Since typescript array index signature types don't represent the // possibility of out-of-bounds access, if we're indexing into an array // just skip the check, to avoid false positives @@ -206,6 +214,30 @@ export default createRule({ } } + /** + * If it is Unary Negation Expression, check the argument for alwaysTruthy / alwaysFalsy + */ + function checkIfUnaryNegationExpressionIsNecessaryConditional( + node: TSESTree.UnaryExpression, + ): void { + if (isArrayIndexExpression(node.argument)) { + return; + } + const type = getNodeType(node.argument); + const messageId = isTypeFlagSet(type, ts.TypeFlags.Never) + ? 'never' + : isPossiblyTruthy(type) + ? 'alwaysFalsy' + : isPossiblyFalsy(type) + ? 'alwaysTruthy' + : undefined; + + if (messageId) { + context.report({ node, messageId }); + } + return; + } + function checkNodeForNullish(node: TSESTree.Expression): void { // Since typescript array index signature types don't represent the // possibility of out-of-bounds access, if we're indexing into an array diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index ce65e717fa0c..a876d54ba324 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -445,11 +445,45 @@ declare const key: Key; foo?.[key]?.trim(); `, + `let latencies: number[][] = []; + +function recordData(): void { + if (!latencies[0]) + latencies[0] = []; + latencies[0].push(4); +} + +recordData();`, + `let latencies: number[][] = []; + +function recordData(): void { + if (latencies[0]) + latencies[0] = []; + latencies[0].push(4); +} + +recordData();`, ], invalid: [ // Ensure that it's checking in all the right places { code: ` +const a=null; +if(!a) { +} + `, + errors: [ruleError(3, 10, 'alwaysTruthy')], + }, + { + code: ` +const a=true; +if(!a) { +} + `, + errors: [ruleError(3, 10, 'alwaysFalsy')], + }, + { + code: ` const b1 = true; declare const b2: boolean; const t1 = b1 && b2; From b8edd34f5ca8808df117e6085431cce653c87dd0 Mon Sep 17 00:00:00 2001 From: Renuga Saraswathy Date: Mon, 10 Aug 2020 18:34:26 +0530 Subject: [PATCH 2/4] fix(eslint-plugin): [no-unnecessary-condition] fixed lint issues and tests --- .../rules/no-unnecessary-condition.test.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index a876d54ba324..811cad447e3f 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -445,42 +445,44 @@ declare const key: Key; foo?.[key]?.trim(); `, - `let latencies: number[][] = []; + ` +let latencies: number[][] = []; function recordData(): void { - if (!latencies[0]) - latencies[0] = []; - latencies[0].push(4); + if (!latencies[0]) latencies[0] = []; + latencies[0].push(4); } -recordData();`, - `let latencies: number[][] = []; +recordData(); + `, + ` +let latencies: number[][] = []; function recordData(): void { - if (latencies[0]) - latencies[0] = []; + if (latencies[0]) latencies[0] = []; latencies[0].push(4); } -recordData();`, +recordData(); + `, ], invalid: [ // Ensure that it's checking in all the right places { code: ` -const a=null; -if(!a) { +const a = null; +if (!a) { } `, - errors: [ruleError(3, 10, 'alwaysTruthy')], + errors: [ruleError(3, 5, 'alwaysTruthy')], }, { code: ` -const a=true; -if(!a) { +const a = true; +if (!a) { } `, - errors: [ruleError(3, 10, 'alwaysFalsy')], + errors: [ruleError(3, 5, 'alwaysFalsy')], }, { code: ` From a4563b2be32de5b8e93bc715302f46ea1c5d836b Mon Sep 17 00:00:00 2001 From: Renuga Saraswathy Date: Tue, 18 Aug 2020 07:52:11 +0530 Subject: [PATCH 3/4] fix(eslint-plugin): [no-unnecessary-condition] added tests for coverage --- .../tests/rules/no-unnecessary-condition.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 811cad447e3f..f3e1b47c2924 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -486,6 +486,18 @@ if (!a) { }, { code: ` +function sayHi(): void { + console.log('Hi!'); +} + +let speech: never = sayHi(); +if (!speech) { +} + `, + errors: [ruleError(7, 4, 'never')], + }, + { + code: ` const b1 = true; declare const b2: boolean; const t1 = b1 && b2; From 9d2fd479794086aa4a2b96edf525006e76268fa2 Mon Sep 17 00:00:00 2001 From: Renuga Saraswathy Date: Tue, 18 Aug 2020 10:02:46 +0530 Subject: [PATCH 4/4] fix(eslint-plugin): [no-unnecessary-condition] fixed unit test after merging master --- .../eslint-plugin/tests/rules/no-unnecessary-condition.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 59e8ce7873ef..3a9d0fb450bc 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -501,7 +501,7 @@ let speech: never = sayHi(); if (!speech) { } `, - errors: [ruleError(7, 4, 'never')], + errors: [ruleError(7, 5, 'never')], }, { code: `