From 18e90ef8d2e1c7e99a85b09a9289bf7e01b371f4 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 09:53:53 +0800 Subject: [PATCH 01/14] Refactor `explicit-length-check` --- rules/explicit-length-check.js | 70 +++++++++++++--------------------- 1 file changed, 26 insertions(+), 44 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 0da58cfcde..485376d524 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -59,16 +59,7 @@ const zeroStyle = { test: node => isCompareRight(node, '===', 0) }; -const cache = new WeakMap(); function getCheckTypeAndLengthNode(node) { - if (!cache.has(node)) { - cache.set(node, getCheckTypeAndLengthNodeWithoutCache(node)); - } - - return cache.get(node); -} - -function getCheckTypeAndLengthNodeWithoutCache(node) { // Non-Zero length check if ( // `foo.length !== 0` @@ -80,7 +71,7 @@ function getCheckTypeAndLengthNodeWithoutCache(node) { // `foo.length >= 1` isCompareRight(node, '>=', 1) ) { - return {type: TYPE_NON_ZERO, node, lengthNode: node.left}; + return {zeroLength: false, lengthNode: node.left}; } if ( @@ -93,7 +84,7 @@ function getCheckTypeAndLengthNodeWithoutCache(node) { // `1 <= foo.length` isCompareLeft(node, '<=', 1) ) { - return {type: TYPE_NON_ZERO, node, lengthNode: node.right}; + return {zeroLength: false, lengthNode: node.right}; } // Zero length check @@ -105,7 +96,7 @@ function getCheckTypeAndLengthNodeWithoutCache(node) { // `foo.length < 1` isCompareRight(node, '<', 1) ) { - return {type: TYPE_ZERO, node, lengthNode: node.left}; + return {zeroLength: true, lengthNode: node.left}; } if ( @@ -116,7 +107,7 @@ function getCheckTypeAndLengthNodeWithoutCache(node) { // `1 > foo.length` isCompareLeft(node, '>', 1) ) { - return {type: TYPE_ZERO, node, lengthNode: node.right}; + return {zeroLength: true, lengthNode: node.right}; } } @@ -140,12 +131,16 @@ function create(context) { const sourceCode = context.getSourceCode(); const reportedBinaryExpressions = new Set(); - function reportProblem({node, type, lengthNode}, isNegative) { + function reportProblem(node, {zeroLength, lengthNode}, isNegative) { if (isNegative) { - type = type === TYPE_NON_ZERO ? TYPE_ZERO : TYPE_NON_ZERO; + zeroLength = !zeroLength; + } + + const {code, test} = zeroLength ? zeroStyle : nonZeroStyle; + if (test(node)) { + return; } - const {code} = type === TYPE_NON_ZERO ? nonZeroStyle : zeroStyle; let fixed = `${sourceCode.getText(lengthNode)} ${code}`; if ( !isParenthesized(node, sourceCode) && @@ -157,13 +152,13 @@ function create(context) { context.report({ node, - messageId: type, + messageId: zeroLength ? TYPE_ZERO : TYPE_NON_ZERO, data: {code}, fix: fixer => fixer.replaceText(node, fixed) }); } - function checkBooleanNode(node) { + function checkBooleanNode(node, isNegative) { if (node.type === 'LogicalExpression') { checkBooleanNode(node.left); checkBooleanNode(node.right); @@ -171,7 +166,7 @@ function create(context) { } if (isLengthProperty(node)) { - reportProblem({node, type: TYPE_NON_ZERO, lengthNode: node}); + reportProblem(node, {zeroLength: false, lengthNode: node}); } } @@ -192,39 +187,26 @@ function create(context) { } if (isLengthProperty(expression)) { - reportProblem({type: TYPE_NON_ZERO, node, lengthNode: expression}, isNegative); + reportProblem(node, {zeroLength: false, lengthNode: expression}, isNegative); return; } - - const result = getCheckTypeAndLengthNode(expression); - if (result) { - reportProblem({...result, node}, isNegative); - reportedBinaryExpressions.add(result.lengthNode); - } }, [booleanNodeSelector](node) { checkBooleanNode(node); }, BinaryExpression(node) { - // Delay check on this, so we don't need take two steps for this case - // `const isEmpty = !(foo.length >= 1);` - binaryExpressions.push(node); - }, - 'Program:exit'() { - for (const node of binaryExpressions) { - if ( - reportedBinaryExpressions.has(node) || - zeroStyle.test(node) || - nonZeroStyle.test(node) - ) { - continue; - } - - const result = getCheckTypeAndLengthNode(node); - if (result) { - reportProblem(result); - } + const result = getCheckTypeAndLengthNode(node); + if (!result) { + return; + } + + let isNegative = false; + while (isLogicNot(node.parent) && node.parent.argument === node) { + isNegative = !isNegative; + node = node.parent; } + + reportProblem(node, result, isNegative); } }; } From 416769ca9e7c9fc9deaaa46a9069dc399a93c340 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 13:34:12 +0800 Subject: [PATCH 02/14] Style --- rules/explicit-length-check.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 485376d524..9393b2ae6a 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -129,7 +129,6 @@ function create(context) { }; const nonZeroStyle = nonZeroStyles.get(options['non-zero']); const sourceCode = context.getSourceCode(); - const reportedBinaryExpressions = new Set(); function reportProblem(node, {zeroLength, lengthNode}, isNegative) { if (isNegative) { @@ -158,7 +157,7 @@ function create(context) { }); } - function checkBooleanNode(node, isNegative) { + function checkBooleanNode(node) { if (node.type === 'LogicalExpression') { checkBooleanNode(node.left); checkBooleanNode(node.right); @@ -170,7 +169,6 @@ function create(context) { } } - const binaryExpressions = []; return { // The outer `!` expression 'UnaryExpression[operator="!"]:not(UnaryExpression[operator="!"] > .argument)'(node) { @@ -188,7 +186,6 @@ function create(context) { if (isLengthProperty(expression)) { reportProblem(node, {zeroLength: false, lengthNode: expression}, isNegative); - return; } }, [booleanNodeSelector](node) { From df5cd4e82a9a9f79eb5150f37eaf08f0da15701b Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 13:44:30 +0800 Subject: [PATCH 03/14] Add `getRemovableBooleanParent` --- rules/explicit-length-check.js | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 9393b2ae6a..6df6c9d6ea 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -121,6 +121,22 @@ const booleanNodeSelector = `:matches(${ 'ForStatement' ].join(', ') }) > *.test`; +const lengthPropertySelector = [ + 'MemberExpression', + '[computed=false]', + '[property.type="Identifier"]', + '[property.name="length]' +].join(''); + +function getRemovableBooleanParent(node) { + let isNegative = false; + while (node.parent && isLogicNot(node.parent) && node.parent.argument === node) { + isNegative = !isNegative; + node = node.parent; + } + + return {node, isNegative}; +} function create(context) { const options = { @@ -170,6 +186,10 @@ function create(context) { } return { + [lengthPropertySelector](node) { + + }, + // The outer `!` expression 'UnaryExpression[operator="!"]:not(UnaryExpression[operator="!"] > .argument)'(node) { let isNegative = false; @@ -197,13 +217,8 @@ function create(context) { return; } - let isNegative = false; - while (isLogicNot(node.parent) && node.parent.argument === node) { - isNegative = !isNegative; - node = node.parent; - } - - reportProblem(node, result, isNegative); + const {isNegative, node: replaceNode} = getRemovableBooleanParent(node); + reportProblem(replaceNode, result, isNegative); } }; } From fafc2a476da2456154fd22a1a755ecaf9d4677a1 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 13:47:01 +0800 Subject: [PATCH 04/14] Add `isLogicNotArgument` --- rules/explicit-length-check.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 6df6c9d6ea..f500fb2792 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -17,6 +17,10 @@ const isLengthProperty = node => const isLogicNot = node => node.type === 'UnaryExpression' && node.operator === '!'; +const isLogicNotArgument = node => + node.parent && + isLogicNot(node.parent) && + node.parent.argument = node; const isLiteralNumber = (node, value) => node.type === 'Literal' && typeof node.value === 'number' && @@ -130,7 +134,7 @@ const lengthPropertySelector = [ function getRemovableBooleanParent(node) { let isNegative = false; - while (node.parent && isLogicNot(node.parent) && node.parent.argument === node) { + while (isLogicNotArgument(node)) { isNegative = !isNegative; node = node.parent; } @@ -186,7 +190,8 @@ function create(context) { } return { - [lengthPropertySelector](node) { + [lengthPropertySelector](lengthNode) { + const {isNegative, node} = getRemovableBooleanParent(lengthNode); }, From 00b4b1bb2032043e74cd86e91a1a15b8c5c985bd Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 14:08:51 +0800 Subject: [PATCH 05/14] Refactor to check from "inner" --- rules/explicit-length-check.js | 62 +++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index f500fb2792..782aec5196 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -20,7 +20,7 @@ const isLogicNot = node => const isLogicNotArgument = node => node.parent && isLogicNot(node.parent) && - node.parent.argument = node; + node.parent.argument === node; const isLiteralNumber = (node, value) => node.type === 'Literal' && typeof node.value === 'number' && @@ -129,7 +129,7 @@ const lengthPropertySelector = [ 'MemberExpression', '[computed=false]', '[property.type="Identifier"]', - '[property.name="length]' + '[property.name="length"]' ].join(''); function getRemovableBooleanParent(node) { @@ -138,10 +138,43 @@ function getRemovableBooleanParent(node) { isNegative = !isNegative; node = node.parent; } - return {node, isNegative}; } +function isBooleanNode(node) { + if (isLogicNot(node)) { + return true; + } + + let {parent} = node; + if (!parent) { + return false; + } + + if (isLogicNotArgument(node)) { + return true; + } + + if (parent.type === 'LogicalExpression') { + return isBooleanNode(parent); + } + + if ( + ( + parent.type === 'IfStatement' || + parent.type === 'ConditionalExpression' || + parent.type === 'WhileStatement' || + parent.type === 'DoWhileStatement' || + parent.type === 'ForStatement' + ) && + parent.test === node + ) { + return true; + } + + return false; +} + function create(context) { const options = { 'non-zero': 'greater-than', @@ -192,32 +225,13 @@ function create(context) { return { [lengthPropertySelector](lengthNode) { const {isNegative, node} = getRemovableBooleanParent(lengthNode); - - }, - - // The outer `!` expression - 'UnaryExpression[operator="!"]:not(UnaryExpression[operator="!"] > .argument)'(node) { - let isNegative = false; - let expression = node; - while (isLogicNot(expression)) { - isNegative = !isNegative; - expression = expression.argument; - } - - if (expression.type === 'LogicalExpression') { - checkBooleanNode(expression); + if (!isBooleanNode(node)) { return; } - if (isLengthProperty(expression)) { - reportProblem(node, {zeroLength: false, lengthNode: expression}, isNegative); - } - }, - [booleanNodeSelector](node) { - checkBooleanNode(node); + reportProblem(node, {zeroLength: false, lengthNode}, isNegative); }, BinaryExpression(node) { - const result = getCheckTypeAndLengthNode(node); if (!result) { return; } From 8e7fc1ef6aeb249ccbf46a846edcc2f5b631f85f Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 14:20:32 +0800 Subject: [PATCH 06/14] Combine selectors --- rules/explicit-length-check.js | 35 ++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 782aec5196..e3994b5691 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -63,7 +63,7 @@ const zeroStyle = { test: node => isCompareRight(node, '===', 0) }; -function getCheckTypeAndLengthNode(node) { +function isNonZeroLengthCheck(node) { // Non-Zero length check if ( // `foo.length !== 0` @@ -75,7 +75,7 @@ function getCheckTypeAndLengthNode(node) { // `foo.length >= 1` isCompareRight(node, '>=', 1) ) { - return {zeroLength: false, lengthNode: node.left}; + return true; } if ( @@ -88,9 +88,11 @@ function getCheckTypeAndLengthNode(node) { // `1 <= foo.length` isCompareLeft(node, '<=', 1) ) { - return {zeroLength: false, lengthNode: node.right}; + return true; } +} +function isZeroLengthCheck(node) { // Zero length check if ( // `foo.length === 0` @@ -100,7 +102,7 @@ function getCheckTypeAndLengthNode(node) { // `foo.length < 1` isCompareRight(node, '<', 1) ) { - return {zeroLength: true, lengthNode: node.left}; + return true; } if ( @@ -111,7 +113,7 @@ function getCheckTypeAndLengthNode(node) { // `1 > foo.length` isCompareLeft(node, '>', 1) ) { - return {zeroLength: true, lengthNode: node.right}; + return true; } } @@ -224,20 +226,25 @@ function create(context) { return { [lengthPropertySelector](lengthNode) { + const {parent} = lengthNode; + let zeroLength; + if (isZeroLengthCheck(parent)) { + zeroLength = true; + } else if (isNonZeroLengthCheck(parent)) { + zeroLength = false; + } + if (typeof zeroLength === 'boolean') { + const {isNegative, node} = getRemovableBooleanParent(parent); + reportProblem(node, {zeroLength, lengthNode}, isNegative); + return + } + const {isNegative, node} = getRemovableBooleanParent(lengthNode); + if (!isBooleanNode(node)) { return; } - reportProblem(node, {zeroLength: false, lengthNode}, isNegative); - }, - BinaryExpression(node) { - if (!result) { - return; - } - - const {isNegative, node: replaceNode} = getRemovableBooleanParent(node); - reportProblem(replaceNode, result, isNegative); } }; } From 6db350ebfe5acbcfff8f5d27f7f3996b575d55bb Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 14:25:41 +0800 Subject: [PATCH 07/14] Move `isNonZeroLengthCheck` `isZeroLengthCheck` logic to listener --- rules/explicit-length-check.js | 114 +++++++++++---------------------- 1 file changed, 36 insertions(+), 78 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index e3994b5691..b8dee237e4 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -63,70 +63,6 @@ const zeroStyle = { test: node => isCompareRight(node, '===', 0) }; -function isNonZeroLengthCheck(node) { - // Non-Zero length check - if ( - // `foo.length !== 0` - isCompareRight(node, '!==', 0) || - // `foo.length != 0` - isCompareRight(node, '!=', 0) || - // `foo.length > 0` - isCompareRight(node, '>', 0) || - // `foo.length >= 1` - isCompareRight(node, '>=', 1) - ) { - return true; - } - - if ( - // `0 !== foo.length` - isCompareLeft(node, '!==', 0) || - // `0 !== foo.length` - isCompareLeft(node, '!=', 0) || - // `0 < foo.length` - isCompareLeft(node, '<', 0) || - // `1 <= foo.length` - isCompareLeft(node, '<=', 1) - ) { - return true; - } -} - -function isZeroLengthCheck(node) { - // Zero length check - if ( - // `foo.length === 0` - isCompareRight(node, '===', 0) || - // `foo.length == 0` - isCompareRight(node, '==', 0) || - // `foo.length < 1` - isCompareRight(node, '<', 1) - ) { - return true; - } - - if ( - // `0 === foo.length` - isCompareLeft(node, '===', 0) || - // `0 == foo.length` - isCompareLeft(node, '==', 0) || - // `1 > foo.length` - isCompareLeft(node, '>', 1) - ) { - return true; - } -} - -// TODO: check other `LogicalExpression`s -const booleanNodeSelector = `:matches(${ - [ - 'IfStatement', - 'ConditionalExpression', - 'WhileStatement', - 'DoWhileStatement', - 'ForStatement' - ].join(', ') -}) > *.test`; const lengthPropertySelector = [ 'MemberExpression', '[computed=false]', @@ -212,27 +148,49 @@ function create(context) { }); } - function checkBooleanNode(node) { - if (node.type === 'LogicalExpression') { - checkBooleanNode(node.left); - checkBooleanNode(node.right); - return; - } - - if (isLengthProperty(node)) { - reportProblem(node, {zeroLength: false, lengthNode: node}); - } - } - return { [lengthPropertySelector](lengthNode) { const {parent} = lengthNode; let zeroLength; - if (isZeroLengthCheck(parent)) { + + if ( + // Zero length check + // `foo.length === 0` + isCompareRight(parent, '===', 0) || + // `foo.length == 0` + isCompareRight(parent, '==', 0) || + // `foo.length < 1` + isCompareRight(parent, '<', 1) || + // `0 === foo.length` + isCompareLeft(parent, '===', 0) || + // `0 == foo.length` + isCompareLeft(parent, '==', 0) || + // `1 > foo.length` + isCompareLeft(parent, '>', 1) + ) { zeroLength = true; - } else if (isNonZeroLengthCheck(parent)) { + } else if ( + // Non-Zero length check + // `foo.length !== 0` + isCompareRight(parent, '!==', 0) || + // `foo.length != 0` + isCompareRight(parent, '!=', 0) || + // `foo.length > 0` + isCompareRight(parent, '>', 0) || + // `foo.length >= 1` + isCompareRight(parent, '>=', 1) || + // `0 !== foo.length` + isCompareLeft(parent, '!==', 0) || + // `0 !== foo.length` + isCompareLeft(parent, '!=', 0) || + // `0 < foo.length` + isCompareLeft(parent, '<', 0) || + // `1 <= foo.length` + isCompareLeft(parent, '<=', 1) + ) { zeroLength = false; } + if (typeof zeroLength === 'boolean') { const {isNegative, node} = getRemovableBooleanParent(parent); reportProblem(node, {zeroLength, lengthNode}, isNegative); From 2d44488400c53c1778a8b91b1593da1fa35fcde1 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 14:45:25 +0800 Subject: [PATCH 08/14] Simplify --- rules/explicit-length-check.js | 113 ++++++++++++++++++--------------- 1 file changed, 61 insertions(+), 52 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index b8dee237e4..e7091bb502 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -79,6 +79,52 @@ function getRemovableBooleanParent(node) { return {node, isNegative}; } +function getLengthCheckType(node) { + if (!node || node.type !== 'BinaryExpression') { + return; + } + + if ( + // Zero length check + // `foo.length === 0` + isCompareRight(node, '===', 0) || + // `foo.length == 0` + isCompareRight(node, '==', 0) || + // `foo.length < 1` + isCompareRight(node, '<', 1) || + // `0 === foo.length` + isCompareLeft(node, '===', 0) || + // `0 == foo.length` + isCompareLeft(node, '==', 0) || + // `1 > foo.length` + isCompareLeft(node, '>', 1) + ) { + return {zeroLength: true}; + } + + if ( + // Non-Zero length check + // `foo.length !== 0` + isCompareRight(node, '!==', 0) || + // `foo.length != 0` + isCompareRight(node, '!=', 0) || + // `foo.length > 0` + isCompareRight(node, '>', 0) || + // `foo.length >= 1` + isCompareRight(node, '>=', 1) || + // `0 !== foo.length` + isCompareLeft(node, '!==', 0) || + // `0 !== foo.length` + isCompareLeft(node, '!=', 0) || + // `0 < foo.length` + isCompareLeft(node, '<', 0) || + // `1 <= foo.length` + isCompareLeft(node, '<=', 1) + ) { + return {zeroLength: false}; + } +} + function isBooleanNode(node) { if (isLogicNot(node)) { return true; @@ -121,11 +167,7 @@ function create(context) { const nonZeroStyle = nonZeroStyles.get(options['non-zero']); const sourceCode = context.getSourceCode(); - function reportProblem(node, {zeroLength, lengthNode}, isNegative) { - if (isNegative) { - zeroLength = !zeroLength; - } - + function reportProblem({node, zeroLength, lengthNode}) { const {code, test} = zeroLength ? zeroStyle : nonZeroStyle; if (test(node)) { return; @@ -151,58 +193,25 @@ function create(context) { return { [lengthPropertySelector](lengthNode) { const {parent} = lengthNode; - let zeroLength; - - if ( - // Zero length check - // `foo.length === 0` - isCompareRight(parent, '===', 0) || - // `foo.length == 0` - isCompareRight(parent, '==', 0) || - // `foo.length < 1` - isCompareRight(parent, '<', 1) || - // `0 === foo.length` - isCompareLeft(parent, '===', 0) || - // `0 == foo.length` - isCompareLeft(parent, '==', 0) || - // `1 > foo.length` - isCompareLeft(parent, '>', 1) - ) { - zeroLength = true; - } else if ( - // Non-Zero length check - // `foo.length !== 0` - isCompareRight(parent, '!==', 0) || - // `foo.length != 0` - isCompareRight(parent, '!=', 0) || - // `foo.length > 0` - isCompareRight(parent, '>', 0) || - // `foo.length >= 1` - isCompareRight(parent, '>=', 1) || - // `0 !== foo.length` - isCompareLeft(parent, '!==', 0) || - // `0 !== foo.length` - isCompareLeft(parent, '!=', 0) || - // `0 < foo.length` - isCompareLeft(parent, '<', 0) || - // `1 <= foo.length` - isCompareLeft(parent, '<=', 1) - ) { - zeroLength = false; - } - + let {zeroLength} = getLengthCheckType(parent) || {}; + let replaceNode; if (typeof zeroLength === 'boolean') { const {isNegative, node} = getRemovableBooleanParent(parent); - reportProblem(node, {zeroLength, lengthNode}, isNegative); - return + replaceNode = node; + if (isNegative) { + zeroLength = !zeroLength; + } + } else { + const {isNegative, node} = getRemovableBooleanParent(lengthNode); + if (isBooleanNode(node)) { + zeroLength = isNegative; + replaceNode = node + } } - const {isNegative, node} = getRemovableBooleanParent(lengthNode); - - if (!isBooleanNode(node)) { - return; + if (replaceNode) { + reportProblem({node: replaceNode, zeroLength, lengthNode}); } - reportProblem(node, {zeroLength: false, lengthNode}, isNegative); } }; } From 9ef209f6a02e991978a9ae729b054a855441a072 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 14:56:54 +0800 Subject: [PATCH 09/14] Simplify --- rules/explicit-length-check.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index e7091bb502..0e1e919061 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -1,6 +1,7 @@ 'use strict'; const {isParenthesized} = require('eslint-utils'); const getDocumentationUrl = require('./utils/get-documentation-url'); +const isLiteralValue = require('./utils/is-literal-value'); const TYPE_NON_ZERO = 'non-zero'; const TYPE_ZERO = 'zero'; @@ -21,20 +22,12 @@ const isLogicNotArgument = node => node.parent && isLogicNot(node.parent) && node.parent.argument === node; -const isLiteralNumber = (node, value) => - node.type === 'Literal' && - typeof node.value === 'number' && - node.value === value; const isCompareRight = (node, operator, value) => - node.type === 'BinaryExpression' && node.operator === operator && - isLengthProperty(node.left) && - isLiteralNumber(node.right, value); + isLiteralValue(node.right, value); const isCompareLeft = (node, operator, value) => - node.type === 'BinaryExpression' && node.operator === operator && - isLengthProperty(node.right) && - isLiteralNumber(node.left, value); + isLiteralValue(node.left, value); const nonZeroStyles = new Map([ [ 'greater-than', From ddb3e32c6ca3327953f3bb216c31dd071530e538 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 15:11:58 +0800 Subject: [PATCH 10/14] Clean --- rules/explicit-length-check.js | 60 +++++++++++++++------------------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 0e1e919061..798a109ac1 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -10,11 +10,6 @@ const messages = { [TYPE_ZERO]: 'Use `.length {{code}}` when checking length is zero.' }; -const isLengthProperty = node => - node.type === 'MemberExpression' && - node.computed === false && - node.property.type === 'Identifier' && - node.property.name === 'length'; const isLogicNot = node => node.type === 'UnaryExpression' && node.operator === '!'; @@ -63,18 +58,20 @@ const lengthPropertySelector = [ '[property.name="length"]' ].join(''); -function getRemovableBooleanParent(node) { +function getBooleanAncestor(node) { let isNegative = false; while (isLogicNotArgument(node)) { isNegative = !isNegative; node = node.parent; } + return {node, isNegative}; } -function getLengthCheckType(node) { - if (!node || node.type !== 'BinaryExpression') { - return; +function getLengthCheckNode(node) { + node = node.parent; + if (node.type !== 'BinaryExpression') { + return {}; } if ( @@ -92,7 +89,7 @@ function getLengthCheckType(node) { // `1 > foo.length` isCompareLeft(node, '>', 1) ) { - return {zeroLength: true}; + return {isZeroLengthCheck: true, node}; } if ( @@ -114,24 +111,22 @@ function getLengthCheckType(node) { // `1 <= foo.length` isCompareLeft(node, '<=', 1) ) { - return {zeroLength: false}; + return {isZeroLengthCheck: false, node}; } + + return {}; } function isBooleanNode(node) { - if (isLogicNot(node)) { + if (isLogicNot(node) || isLogicNotArgument(node)) { return true; } - let {parent} = node; + const {parent} = node; if (!parent) { return false; } - if (isLogicNotArgument(node)) { - return true; - } - if (parent.type === 'LogicalExpression') { return isBooleanNode(parent); } @@ -160,8 +155,8 @@ function create(context) { const nonZeroStyle = nonZeroStyles.get(options['non-zero']); const sourceCode = context.getSourceCode(); - function reportProblem({node, zeroLength, lengthNode}) { - const {code, test} = zeroLength ? zeroStyle : nonZeroStyle; + function reportProblem({node, isZeroLengthCheck, lengthNode}) { + const {code, test} = isZeroLengthCheck ? zeroStyle : nonZeroStyle; if (test(node)) { return; } @@ -177,7 +172,7 @@ function create(context) { context.report({ node, - messageId: zeroLength ? TYPE_ZERO : TYPE_NON_ZERO, + messageId: isZeroLengthCheck ? TYPE_ZERO : TYPE_NON_ZERO, data: {code}, fix: fixer => fixer.replaceText(node, fixed) }); @@ -185,25 +180,24 @@ function create(context) { return { [lengthPropertySelector](lengthNode) { - const {parent} = lengthNode; - let {zeroLength} = getLengthCheckType(parent) || {}; - let replaceNode; - if (typeof zeroLength === 'boolean') { - const {isNegative, node} = getRemovableBooleanParent(parent); - replaceNode = node; + let {isZeroLengthCheck, node: lengthCheckNode} = getLengthCheckNode(lengthNode); + let node; + if (lengthCheckNode) { + const {isNegative, node: ancestor} = getBooleanAncestor(lengthCheckNode); + node = ancestor; if (isNegative) { - zeroLength = !zeroLength; + isZeroLengthCheck = !isZeroLengthCheck; } } else { - const {isNegative, node} = getRemovableBooleanParent(lengthNode); - if (isBooleanNode(node)) { - zeroLength = isNegative; - replaceNode = node + const {isNegative, node: ancestor} = getBooleanAncestor(lengthNode); + if (isBooleanNode(ancestor)) { + isZeroLengthCheck = isNegative; + node = ancestor; } } - if (replaceNode) { - reportProblem({node: replaceNode, zeroLength, lengthNode}); + if (node) { + reportProblem({node, isZeroLengthCheck, lengthNode}); } } }; From d579985becf251b699066445e3702a12a2721d57 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 15:16:30 +0800 Subject: [PATCH 11/14] Style --- rules/explicit-length-check.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 798a109ac1..e829ba7723 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -180,8 +180,9 @@ function create(context) { return { [lengthPropertySelector](lengthNode) { - let {isZeroLengthCheck, node: lengthCheckNode} = getLengthCheckNode(lengthNode); let node; + + let {isZeroLengthCheck, node: lengthCheckNode} = getLengthCheckNode(lengthNode); if (lengthCheckNode) { const {isNegative, node: ancestor} = getBooleanAncestor(lengthCheckNode); node = ancestor; From 981c5acd3d2811ec85714a4e545275879f1f9ad7 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 15:22:21 +0800 Subject: [PATCH 12/14] Fix logic --- rules/explicit-length-check.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index e829ba7723..5614785a5a 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -18,9 +18,11 @@ const isLogicNotArgument = node => isLogicNot(node.parent) && node.parent.argument === node; const isCompareRight = (node, operator, value) => + node.type === 'BinaryExpression' && node.operator === operator && isLiteralValue(node.right, value); const isCompareLeft = (node, operator, value) => + node.type === 'BinaryExpression' && node.operator === operator && isLiteralValue(node.left, value); const nonZeroStyles = new Map([ @@ -70,12 +72,9 @@ function getBooleanAncestor(node) { function getLengthCheckNode(node) { node = node.parent; - if (node.type !== 'BinaryExpression') { - return {}; - } + // Zero length check if ( - // Zero length check // `foo.length === 0` isCompareRight(node, '===', 0) || // `foo.length == 0` @@ -92,8 +91,8 @@ function getLengthCheckNode(node) { return {isZeroLengthCheck: true, node}; } + // Non-Zero length check if ( - // Non-Zero length check // `foo.length !== 0` isCompareRight(node, '!==', 0) || // `foo.length != 0` From d1639fc396f6241c5e91726ceac44e5674e67ad3 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 15:30:32 +0800 Subject: [PATCH 13/14] Coverage --- rules/explicit-length-check.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 5614785a5a..4440919512 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -122,14 +122,6 @@ function isBooleanNode(node) { } const {parent} = node; - if (!parent) { - return false; - } - - if (parent.type === 'LogicalExpression') { - return isBooleanNode(parent); - } - if ( ( parent.type === 'IfStatement' || @@ -143,6 +135,10 @@ function isBooleanNode(node) { return true; } + if (parent.type === 'LogicalExpression') { + return isBooleanNode(parent); + } + return false; } From aafa45af7fcf5bece0774fc4b98cf1fad3a601b1 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 22 Dec 2020 15:51:47 +0800 Subject: [PATCH 14/14] Rename --- rules/explicit-length-check.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/explicit-length-check.js b/rules/explicit-length-check.js index 4440919512..621285221d 100644 --- a/rules/explicit-length-check.js +++ b/rules/explicit-length-check.js @@ -53,7 +53,7 @@ const zeroStyle = { test: node => isCompareRight(node, '===', 0) }; -const lengthPropertySelector = [ +const lengthSelector = [ 'MemberExpression', '[computed=false]', '[property.type="Identifier"]', @@ -174,7 +174,7 @@ function create(context) { } return { - [lengthPropertySelector](lengthNode) { + [lengthSelector](lengthNode) { let node; let {isZeroLengthCheck, node: lengthCheckNode} = getLengthCheckNode(lengthNode);