fix: isAlternateGuarded in jsx/no-leaked-conditional-rendering

Rel1cx · Nov 10, 2023 · 32be8d6 · 32be8d6
1 parent 33a10db
commit 32be8d6
Show file tree

Hide file tree

Showing 16 changed files with 124 additions and 47 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/monorepo",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "ESLint plugin for React function components with TypeScript, built (mostly) from scratch.",
   "keywords": [
     "eslint",

diff --git a/packages/ast/package.json b/packages/ast/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/ast",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "AST Utility Module for Static Analysis of TypeScript",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/ast/src/unary.ts b/packages/ast/src/unary.ts
@@ -2,16 +2,18 @@ import type { TSESTree } from "@typescript-eslint/types";
 
 import { NodeType } from "./node-type";
 
-export function getNestedUnaryOperators(
-  node: TSESTree.UnaryExpression,
-  seen = [],
-): TSESTree.UnaryExpression["operator"][] {
+/**
+ * Get all unary operators in a nested unary expression.
+ * @param node The node to get the operators from.
+ * @returns All unary operators in a nested unary expression.
+ */
+export function getNestedUnaryOperators(node: TSESTree.UnaryExpression): TSESTree.UnaryExpression["operator"][] {
   const { operator } = node;
   const { argument } = node;
 
   if (argument.type === NodeType.UnaryExpression) {
-    return [...seen, operator, ...getNestedUnaryOperators(argument, seen)];
+    return [operator, ...getNestedUnaryOperators(argument)];
   }
 
-  return [...seen, operator];
+  return [operator];
 }
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/core",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "AST Utility Module for Static Analysis of React core API and Patterns.",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/eslint-plugin-debug/package.json b/packages/eslint-plugin-debug/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/eslint-plugin-debug",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "Debug specific rules for @eslint-react/eslint-plugin",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/eslint-plugin-hooks/package.json b/packages/eslint-plugin-hooks/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/eslint-plugin-hooks",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "Hooks specific rules for @eslint-react/eslint-plugin",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/eslint-plugin-jsx/package.json b/packages/eslint-plugin-jsx/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/eslint-plugin-jsx",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "JSX specific rules for @eslint-react/eslint-plugin",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/eslint-plugin-jsx/src/rules/no-leaked-conditional-rendering.spec.ts b/packages/eslint-plugin-jsx/src/rules/no-leaked-conditional-rendering.spec.ts
@@ -281,7 +281,7 @@ ruleTester.run(RULE_NAME, rule, {
       }
     `,
     dedent`
-      const someCondition1 = 0;
+      const someCondition = 0;
       const SomeComponent = () => <div />;
 
       const App = () => {
@@ -293,7 +293,7 @@ ruleTester.run(RULE_NAME, rule, {
                 prop1={val1}
                 prop2={val2}
               />)
-              : someCondition1 ? null : <div />
+              : someCondition ? null : <div />
             }
           </>
         )
@@ -316,6 +316,28 @@ ruleTester.run(RULE_NAME, rule, {
         )
       }
     `,
+    dedent`
+      const someCondition = 0;
+      const SomeComponent = () => <div />;
+
+      const App = () => {
+        return (
+          <>
+            {!!someCondition
+              ? (
+              <SomeComponent
+                prop1={val1}
+                prop2={val2}
+              />)
+              : someCondition ? Date()
+              : someCondition && someCondition
+              ? <div />
+              : null
+            }
+          </>
+        )
+      }
+    `,
   ],
   invalid: [
     {
@@ -489,7 +511,7 @@ ruleTester.run(RULE_NAME, rule, {
     },
     {
       code: dedent`
-        const someCondition1 = 0;
+        const someCondition = 0;
         const SomeComponent = () => <div />;
 
         const App = () => {
@@ -515,7 +537,7 @@ ruleTester.run(RULE_NAME, rule, {
     },
     {
       code: dedent`
-        const someCondition1 = 0;
+        const someCondition = 0;
         const SomeComponent = () => <div />;
 
         const App = () => {
@@ -527,7 +549,7 @@ ruleTester.run(RULE_NAME, rule, {
                   prop1={val1}
                   prop2={val2}
                 />)
-                : someCondition1 && <div />
+                : someCondition && <div />
               }
             </>
           )
@@ -541,7 +563,7 @@ ruleTester.run(RULE_NAME, rule, {
     },
     {
       code: dedent`
-        const someCondition1 = 0;
+        const someCondition = 1;
         const SomeComponent = () => <div />;
 
         const App = () => {
@@ -553,7 +575,7 @@ ruleTester.run(RULE_NAME, rule, {
                   prop1={val1}
                   prop2={val2}
                 />)
-                : someCondition1 ? Date() : <div />
+                : someCondition ? Date() : <div />
               }
             </>
           )
@@ -585,5 +607,32 @@ ruleTester.run(RULE_NAME, rule, {
         },
       ],
     },
+    {
+      code: dedent`
+        const someCondition = 1;
+        const SomeComponent = () => <div />;
+
+        const App = () => {
+          return (
+            <>
+              {!!someCondition
+                ? (
+                <SomeComponent
+                  prop1={val1}
+                  prop2={val2}
+                />)
+                : someCondition ? window
+                : someCondition && someCondition
+                ? <div />
+                : null
+              }
+            </>
+          )
+        }
+      `,
+      errors: [
+        { messageId: "NO_LEAKED_CONDITIONAL_RENDERING" },
+      ],
+    },
   ],
 });
diff --git a/packages/eslint-plugin-jsx/src/rules/no-leaked-conditional-rendering.ts b/packages/eslint-plugin-jsx/src/rules/no-leaked-conditional-rendering.ts
@@ -1,5 +1,5 @@
 /* eslint-disable sonarjs/no-duplicate-string */
-import { getNestedUnaryOperators, isJSX, isNodeEqual, NodeType } from "@eslint-react/ast";
+import { getNestedUnaryOperators, isJSX, isNodeEqual, isOneOf, NodeType } from "@eslint-react/ast";
 import { isJSXValue, JSXValueCheckHint } from "@eslint-react/jsx";
 import { F } from "@eslint-react/tools";
 import { getConstrainedTypeAtLocation } from "@typescript-eslint/type-utils";
@@ -34,6 +34,13 @@ type VariantType =
   | "truthy number"
   | "truthy string";
 
+const falsyTypes = [
+  "nullish",
+  "falsy boolean",
+  "falsy number",
+  "falsy string",
+] as const satisfies VariantType[];
+
 // Allowed un-guarded type variants
 const allowTypes = [
   "boolean",
@@ -54,7 +61,7 @@ const allowGuardedConsequentTypes = [
   "truthy boolean",
   "truthy number",
   "truthy string",
-] as const;
+] as const satisfies VariantType[];
 
 // Allowed guarded alternate type variants
 const allowGuardedAlternateTypes = [
@@ -66,7 +73,7 @@ const allowGuardedAlternateTypes = [
   "falsy boolean",
   "falsy number",
   "falsy string",
-] as const;
+] as const satisfies VariantType[];
 
 // Allowed guarded logical right type variants
 const allowGuardedUnaryNotTypes = [
@@ -82,7 +89,7 @@ const allowGuardedUnaryNotTypes = [
   "falsy boolean",
   "falsy number",
   "falsy string",
-] as const;
+] as const satisfies VariantType[];
 
 /**
  * Ported from https://github.com/typescript-eslint/typescript-eslint/blob/eb736bbfc22554694400e6a4f97051d845d32e0b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts#L826
@@ -220,6 +227,7 @@ export default createRule<[], MessageID>({
     },
   },
   defaultOptions: [],
+  // eslint-disable-next-line sonarjs/cognitive-complexity
   create(context) {
     const hint = JSXValueCheckHint.StrictArray
       | JSXValueCheckHint.StrictLogical
@@ -252,24 +260,24 @@ export default createRule<[], MessageID>({
         return true;
       }
 
-      const isLeftHasUnaryNot = left.type === NodeType.UnaryExpression
-        && getNestedUnaryOperators(left).some(op => op === "!");
+      const isLeftUnaryNot = left.type === NodeType.UnaryExpression
+        && left.operator === "!";
 
-      if (isLeftHasUnaryNot) {
+      if (isLeftUnaryNot) {
         if (isJSX(right)) {
           return true;
         }
 
         const rightType = getConstrainedTypeAtLocation(services, right);
-        const types = inspectVariantTypes(tsutils.unionTypeParts(rightType));
+        const rightTypeVariants = inspectVariantTypes(tsutils.unionTypeParts(rightType));
 
-        return types.every(type => allowGuardedUnaryNotTypes.includes(type as never));
+        return rightTypeVariants.every(type => allowGuardedUnaryNotTypes.includes(type as never));
       }
 
       const leftType = getConstrainedTypeAtLocation(services, left);
-      const types = inspectVariantTypes(tsutils.unionTypeParts(leftType));
+      const leftTypeVariants = inspectVariantTypes(tsutils.unionTypeParts(leftType));
 
-      return types.every(type => allowTypes.includes(type as never));
+      return leftTypeVariants.every(type => allowTypes.includes(type as never));
     }
 
     function isValidConditionalExpression(
@@ -279,27 +287,45 @@ export default createRule<[], MessageID>({
 
       const isConsequentGuarded = isNodeEqual(consequent, test);
       const testType = getConstrainedTypeAtLocation(services, test);
-      const types = inspectVariantTypes(tsutils.unionTypeParts(testType));
+      const testTypeVariants = inspectVariantTypes(tsutils.unionTypeParts(testType));
 
       if (
         isConsequentGuarded
-        && types.every(type => allowGuardedConsequentTypes.includes(type as never))
+        && testTypeVariants.every(type => allowGuardedConsequentTypes.includes(type as never))
       ) {
         return true;
       }
 
-      const unaryOperatorsInTest = test.type === NodeType.UnaryExpression
-        ? getNestedUnaryOperators(test)
-        : [];
+      if (test.type === NodeType.UnaryExpression) {
+        const unaryNotOperatorsInTest = getNestedUnaryOperators(test);
+        const testIsFalsy = testTypeVariants.every(type => falsyTypes.includes(type as never));
+        const isAlternateGuarded = testIsFalsy
+          // Check for `!!` or `!!!!` etc in the test
+          && unaryNotOperatorsInTest.every(op => op === "!")
+          && unaryNotOperatorsInTest.length % 2 === 0;
 
-      const isAlternateGuarded = unaryOperatorsInTest.every(op => op === "!")
-        && unaryOperatorsInTest.length % 2 === 1;
+        if (isAlternateGuarded && testTypeVariants.every(type => allowGuardedAlternateTypes.includes(type as never))) {
+          return isValidInnerExpression(alternate);
+        }
+      }
 
-      if (isAlternateGuarded && types.every(type => allowGuardedAlternateTypes.includes(type as never))) {
-        return true;
+      if (test.type === NodeType.Identifier) {
+        const isAlternateGuarded = testTypeVariants.every(type => falsyTypes.includes(type as never));
+
+        if (isAlternateGuarded) {
+          if (isJSX(alternate)) {
+            return true;
+          }
+
+          if (isOneOf([NodeType.LogicalExpression, NodeType.ConditionalExpression])(alternate)) {
+            return isValidInnerExpression(alternate);
+          }
+
+          return true;
+        }
       }
 
-      return isValidInnerExpression(alternate) && isValidInnerExpression(consequent);
+      return isValidInnerExpression(consequent) && isValidInnerExpression(alternate);
     }
 
     return {

diff --git a/packages/eslint-plugin-naming-convention/package.json b/packages/eslint-plugin-naming-convention/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/eslint-plugin-naming-convention",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "Naming convention specific rules for ESLint-plugin-React",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/eslint-plugin-react/package.json b/packages/eslint-plugin-react/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/eslint-plugin-react",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "React specific rules for ESLint",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/eslint-plugin",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "ESLint plugin for React function components with TypeScript, built (mostly) from scratch.",
   "keywords": [
     "eslint",

diff --git a/packages/jsx/package.json b/packages/jsx/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/jsx",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "AST Utility Module for Static Analysis of JSX",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/shared/package.json b/packages/shared/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/shared",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "Shared constants and utilities for @eslint-react's packages",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/tools/package.json b/packages/tools/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/tools",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "Primitive tools for @eslint-react's packages",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {

diff --git a/packages/types/package.json b/packages/types/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eslint-react/types",
-  "version": "0.7.9",
+  "version": "0.8.0",
   "description": "Type definitions for @eslint-react",
   "homepage": "https://github.com/eslint-react/eslint-react",
   "bugs": {