fix: update logic by review

eslint · Feb 4, 2023 · 7c91ba8 · 7c91ba8
1 parent 8971e90
commit 7c91ba8
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 26 deletions.
diff --git a/lib/rules/no-constant-binary-expression.js b/lib/rules/no-constant-binary-expression.js
@@ -14,6 +14,23 @@ const NUMERIC_OR_STRING_BINARY_OPERATORS = new Set(["+", "-", "*", "/", "%", "|"
 // Helpers
 //------------------------------------------------------------------------------
 
+/**
+ * Checks whether or not a node is `null` or `undefined`. Similar to the one
+ * found in ast-utils.js, but this one correctly handles the edge case that
+ * `undefined` has been redefined.
+ * @param {Scope} scope Scope in which the expression was found.
+ * @param {ASTNode} node A node to check.
+ * @returns {boolean} Whether or not the node is a `null` or `undefined`.
+ * @public
+ */
+function isNullOrUndefined(scope, node) {
+    return (
+        isNullLiteral(node) ||
+        (node.type === "Identifier" && node.name === "undefined" && isReferenceToGlobalVariable(scope, node)) ||
+        (node.type === "UnaryExpression" && node.operator === "void")
+    );
+}
+
 /**
  * Test if an AST node has a statically knowable constant nullishness. Meaning,
  * it will always resolve to a constant value of either: `null`, `undefined`
@@ -46,10 +63,7 @@ function hasConstantNullishness(scope, node) {
                 isReferenceToGlobalVariable(scope, node.callee);
         }
         case "LogicalExpression": {
-            const isLeftConstant = hasConstantNullishness(scope, node.left);
-            const isRightConstant = hasConstantNullishness(scope, node.right);
-
-            return isLeftConstant || isRightConstant;
+            return node.operator === "??" && (hasConstantNullishness(scope, node.right) && !isNullOrUndefined(scope, node.right));
         }
         case "AssignmentExpression":
             if (node.operator === "=") {
@@ -384,24 +398,6 @@ function isAlwaysNew(scope, node) {
     }
 }
 
-/**
- * Checks whether or not a node is `null` or `undefined`. Similar to the one
- * found in ast-utils.js, but this one correctly handles the edge case that
- * `undefined` has been redefined.
- * @param {Scope} scope Scope in which the expression was found.
- * @param {ASTNode} node A node to check.
- * @returns {boolean} Whether or not the node is a `null` or `undefined`.
- * @public
- */
-function isNullOrUndefined(scope, node) {
-    return (
-        isNullLiteral(node) ||
-        (node.type === "Identifier" && node.name === "undefined" && isReferenceToGlobalVariable(scope, node)) ||
-        (node.type === "UnaryExpression" && node.operator === "void")
-    );
-}
-
-
 /**
  * Checks if one operand will cause the result to be constant.
  * @param {Scope} scope Scope in which the expression was found.

diff --git a/tests/lib/rules/no-constant-binary-expression.js b/tests/lib/rules/no-constant-binary-expression.js
@@ -59,7 +59,9 @@ ruleTester.run("no-constant-binary-expression", rule, {
         "function foo(undefined) { undefined === true;}",
         "[...arr, 1] == true",
         "[,,,] == true",
-        { code: "new Foo() === bar;", globals: { Foo: "writable" } }
+        { code: "new Foo() === bar;", globals: { Foo: "writable" } },
+        "(foo && true) ?? bar",
+        "foo ?? null ?? bar"
     ],
     invalid: [
 
@@ -312,8 +314,6 @@ ruleTester.run("no-constant-binary-expression", rule, {
 
         { code: "window.abc && false && anything", errors: [{ messageId: "constantShortCircuit" }] },
         { code: "window.abc || true || anything", errors: [{ messageId: "constantShortCircuit" }] },
-        { code: "window.abc ?? 'non-nullish' ?? 'non-nullish'", errors: [{ messageId: "constantShortCircuit" }] },
-        { code: "window.abc ?? undefined ?? 'non-nullish'", errors: [{ messageId: "constantShortCircuit" }] },
-        { code: "window.abc ?? null ?? 'non-nullish'", errors: [{ messageId: "constantShortCircuit" }] }
+        { code: "window.abc ?? 'non-nullish' ?? 'non-nullish'", errors: [{ messageId: "constantShortCircuit" }] }
     ]
 });