Make function name checking more resilient for #116.

solidjs-community · Dec 30, 2023 · a69c97f · a69c97f
1 parent 87e2f9b
commit a69c97f
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 9 deletions.
diff --git a/docs/components-return-once.md b/docs/components-return-once.md
@@ -145,5 +145,33 @@ callback(() => {
   return <div />;
 });
 
+function Component() {
+  const renderContent = () => {
+    if (false) return <></>;
+    return <></>;
+  };
+  return <>{renderContent()}</>;
+}
+
+function Component() {
+  function renderContent() {
+    if (false) return <></>;
+    return <></>;
+  }
+  return <>{renderContent()}</>;
+}
+
+function Component() {
+  const renderContent = () => {
+    const renderContentInner = () => {
+      // ifs in render functions are fine no matter what nesting level this is
+      if (false) return;
+      return <></>;
+    };
+    return <>{renderContentInner()}</>;
+  };
+  return <></>;
+}
+
 ```
 <!-- AUTO-GENERATED-CONTENT:END -->
diff --git a/src/rules/components-return-once.ts b/src/rules/components-return-once.ts
@@ -1,5 +1,5 @@
 import { TSESTree as T, ESLintUtils } from "@typescript-eslint/utils";
-import type { FunctionNode } from "../utils";
+import { getFunctionName, type FunctionNode } from "../utils";
 
 const createRule = ESLintUtils.RuleCreator.withoutDocs;
 
@@ -63,8 +63,8 @@ export default createRule({
 
     const onFunctionExit = (node: FunctionNode) => {
       if (
-        (node.type === "FunctionDeclaration" && node.id?.name?.match(/^[a-z]/)) ||
         // "render props" aren't components
+        getFunctionName(node)?.match(/^[a-z]/) ||
         node.parent?.type === "JSXExpressionContainer" ||
         // ignore createMemo(() => conditional JSX), report HOC(() => conditional JSX)
         (node.parent?.type === "CallExpression" &&

diff --git a/src/rules/reactivity.ts b/src/rules/reactivity.ts
@@ -15,6 +15,7 @@ import {
   trackImports,
   isDOMElementName,
   ignoreTransparentWrappers,
+  getFunctionName,
 } from "../utils";
 
 const { findVariable, getFunctionHeadLocation } = ASTUtils;
@@ -425,15 +426,17 @@ export default createRule<Options, MessageIds>({
     const onFunctionExit = (currentScopeNode: ProgramOrFunctionNode) => {
       // If this function is a component, add its props as a reactive variable
       if (isFunctionNode(currentScopeNode)) {
-        markPropsOnCondition(
-          currentScopeNode,
-          (props) =>
+        markPropsOnCondition(currentScopeNode, (props) => {
+          if (
             !isPropsByName(props.name) && // already added in markPropsOnEnter
-            currentScope().hasJSX &&
+            currentScope().hasJSX
+          ) {
+            const functionName = getFunctionName(currentScopeNode);
             // begins with lowercase === not component
-            (currentScopeNode.type !== "FunctionDeclaration" ||
-              !currentScopeNode.id?.name?.match(/^[a-z]/))
-        );
+            if (functionName && !/^[a-z]/.test(functionName)) return true;
+          }
+          return false;
+        });
       }
 
       // Ignore sync callbacks like Array#forEach and certain Solid primitives.

diff --git a/src/utils.ts b/src/utils.ts
@@ -97,6 +97,19 @@ export const isProgramOrFunctionNode = (
   node: T.Node | null | undefined
 ): node is ProgramOrFunctionNode => !!node && PROGRAM_OR_FUNCTION_TYPES.includes(node.type);
 
+export const getFunctionName = (node: FunctionNode): string | null => {
+  if (
+    (node.type === "FunctionDeclaration" || node.type === "FunctionExpression") &&
+    node.id != null
+  ) {
+    return node.id.name;
+  }
+  if (node.parent?.type === "VariableDeclarator" && node.parent.id.type === "Identifier") {
+    return node.parent.id.name;
+  }
+  return null;
+};
+
 export function findInScope(
   node: T.Node,
   scope: ProgramOrFunctionNode,

diff --git a/test/rules/components-return-once.test.ts b/test/rules/components-return-once.test.ts
@@ -24,6 +24,31 @@ export const cases = run("components-return-once", rule, {
       }
       return <div />;
     });`,
+    `function Component() {
+      const renderContent = () => {
+        if (false) return <></>;
+        return <></>;
+      }
+      return <>{renderContent()}</>;
+    }`,
+    `function Component() {
+      function renderContent() {
+        if (false) return <></>;
+        return <></>;
+      }
+      return <>{renderContent()}</>;
+    }`,
+    `function Component() {
+      const renderContent = () => {
+        const renderContentInner = () => {
+          // ifs in render functions are fine no matter what nesting level this is
+          if (false) return;
+          return <></>;
+        };
+        return <>{renderContentInner()}</>;
+      };
+      return <></>;
+    }`,
   ],
   invalid: [
     // Early returns