Fix no-unnecessary-key false positives on render function

Author: Rel1cx

packages/core/docs/functions/isRenderFunctionLoose.md

@@ -3,7 +3,7 @@
 # Function: isRenderFunctionLoose()
 
 ```ts
-function isRenderFunctionLoose(context: RuleContext, node: TSESTreeFunction): boolean;
+function isRenderFunctionLoose(context: RuleContext, node: Node): node is TSESTreeFunction;
 ```
 
 Unsafe check whether given node is a render function
@@ -19,10 +19,10 @@ _ = <Component renderRow={() => <div />} />
 | Parameter | Type | Description |
 | ------ | ------ | ------ |
 | `context` | `RuleContext` | The rule context |
-| `node` | `TSESTreeFunction` | The AST node to check |
+| `node` | `Node` | The AST node to check |
 
 ## Returns
 
-`boolean`
+`node is TSESTreeFunction`
 
 `true` if node is a render function, `false` if not

packages/core/src/component/component-render-prop.ts

@@ -2,7 +2,6 @@ import * as AST from "@eslint-react/ast";
 import type { RuleContext } from "@eslint-react/shared";
 import type { TSESTree } from "@typescript-eslint/types";
 import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
-import { JsxDetectionHint, isJsxLike } from "../jsx";
 
 /**
  * Unsafe check whether given node is a render function
@@ -16,23 +15,13 @@ import { JsxDetectionHint, isJsxLike } from "../jsx";
  * @param node The AST node to check
  * @returns `true` if node is a render function, `false` if not
  */
-export function isRenderFunctionLoose(context: RuleContext, node: AST.TSESTreeFunction) {
-  const { body, parent } = node;
-  // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-  if (AST.getFunctionId(node)?.name.startsWith("render")) {
-    return parent.type === T.JSXExpressionContainer
-      && parent.parent.type === T.JSXAttribute
-      && parent.parent.name.type === T.JSXIdentifier
-      && parent.parent.name.name.startsWith("render");
-  }
-  return isJsxLike(
-    context.sourceCode,
-    body,
-    JsxDetectionHint.SkipNullLiteral
-      | JsxDetectionHint.SkipUndefined
-      | JsxDetectionHint.StrictLogical
-      | JsxDetectionHint.StrictConditional,
-  );
+export function isRenderFunctionLoose(context: RuleContext, node: TSESTree.Node): node is AST.TSESTreeFunction {
+  if (!AST.isFunction(node)) return false;
+  if ((AST.getFunctionId(node)?.name.startsWith("render")) ?? false) return true;
+  return node.parent.type === T.JSXExpressionContainer
+    && node.parent.parent.type === T.JSXAttribute
+    && node.parent.parent.name.type === T.JSXIdentifier
+    && node.parent.parent.name.name.startsWith("render");
 }
 
 /**
@@ -51,7 +40,6 @@ export function isRenderPropLoose(context: RuleContext, node: TSESTree.JSXAttrib
   }
   return node.name.name.startsWith("render")
     && node.value?.type === T.JSXExpressionContainer
-    && AST.isFunction(node.value.expression)
     && isRenderFunctionLoose(context, node.value.expression);
 }
 

packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.spec.ts

@@ -89,13 +89,6 @@ ruleTester.run(RULE_NAME, rule, {
       `,
       errors: [{ messageId: "noUnnecessaryKey" }],
     },
-    // Invalid: static key in arrow function component
-    {
-      code: tsx`
-        const ArrowComponent = () => <span key="static" />;
-      `,
-      errors: [{ messageId: "noUnnecessaryKey" }],
-    },
     // Invalid: deeply nested unnecessary keys
     {
       code: tsx`
@@ -132,17 +125,6 @@ ruleTester.run(RULE_NAME, rule, {
       `,
       errors: [{ messageId: "noUnnecessaryKey" }],
     },
-    // Invalid: unnecessary key in class component render
-    {
-      code: tsx`
-        class MyComponent extends React.Component {
-          render() {
-            return <div key="class-static" />;
-          }
-        }
-      `,
-      errors: [{ messageId: "noUnnecessaryKey" }],
-    },
     // Invalid:  key on child with sibling elements (not in list context)
     {
       code: tsx`
@@ -180,13 +162,6 @@ ruleTester.run(RULE_NAME, rule, {
       `,
       errors: [{ messageId: "noUnnecessaryKey" }, { messageId: "noUnnecessaryKey" }],
     },
-    // Invalid: key in immediately invoked function expression
-    {
-      code: tsx`
-        const element = (() => <div key="iife" />)();
-      `,
-      errors: [{ messageId: "noUnnecessaryKey" }],
-    },
     // Invalid: multiple levels of unnecessary keys
     {
       code: tsx`
@@ -383,6 +358,7 @@ ruleTester.run(RULE_NAME, rule, {
       items?.map(item => <div key={item.id} />)
     `,
     // Valid: key on element in callback passed to custom component
+    // FIXME: Currently reports a false positive
     tsx`
       <List renderItem={(item) => <ListItem key={item.id} />} />
     `,

packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.ts

@@ -5,6 +5,7 @@ import {
   getJsxConfigFromAnnotation,
   getJsxConfigFromContext,
   isJsxFragmentElement,
+  isRenderFunctionLoose,
 } from "@eslint-react/core";
 import { type RuleContext, type RuleFeature } from "@eslint-react/shared";
 import type { TSESTree } from "@typescript-eslint/types";
@@ -54,6 +55,9 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
       if (isJsxFragmentElement(context, jsxElement, jsxConfig)) return;
       // If there is a spread attribute, it's not safe to report an unnecessary key
       if (jsxElement.openingElement.attributes.some((attr) => attr.type === T.JSXSpreadAttribute)) return;
+      const isInsideRenderFunction = AST.findParentNode(jsxElement, (n) => isRenderFunctionLoose(context, n)) != null;
+      // If inside a render function, skip checking to avoid false positives
+      if (isInsideRenderFunction) return;
       // Find the parent `.map()` callback function, if it exists
       const mapCallback = AST.findParentNode(jsxElement, isArrayMethodCallback);
       // Check static keys on elements that are not in a map context
@@ -99,13 +103,8 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
   };
 }
 
-/**
- * Checks if a node is a callback function passed to an array's `.map()` method
- * @param node The node to check
- * @returns `true` if the node is a map callback, `false` otherwise
- */
 function isArrayMethodCallback(node: TSESTree.Node) {
   if (node.parent?.type !== T.CallExpression) return false;
-  if (!AST.isArrayMapCall(node.parent) || !AST.isArrayFromCall(node.parent)) return false;
+  if (!AST.isArrayMapCall(node.parent) && !AST.isArrayFromCall(node.parent)) return false;
   return AST.isOneOf([T.ArrowFunctionExpression, T.FunctionExpression])(AST.getUnderlyingExpression(node));
 }