Refactor no-implicit-key rule to use type checking (#1466)

Signed-off-by: REL1CX <solarflamex@hotmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>

Author: 3 people

apps/website/content/docs/rules/overview.mdx

@@ -55,7 +55,7 @@ full: true
 | [`no-direct-mutation-state`](no-direct-mutation-state)                             | 2️⃣ 2️⃣ |           | Disallows direct mutation of `this.state`                                                                                     |          |
 | [`no-duplicate-key`](no-duplicate-key)                                             | 0️⃣ 0️⃣ |    `🧪`   | Prevents duplicate `key` props on sibling elements when rendering lists                                                       |          |
 | [`no-forward-ref`](no-forward-ref)                                                 | 1️⃣ 1️⃣ |    `🔄`   | Replaces usage of `forwardRef` with passing `ref` as a prop                                                                   | >=19.0.0 |
-| [`no-implicit-key`](no-implicit-key)                                               | 1️⃣ 1️⃣ |    `🧪`   | Prevents `key` from not being explicitly specified (e.g., spreading `key` from objects)                                       |          |
+| [`no-implicit-key`](no-implicit-key)                                               | 0️⃣ 0️⃣ | `💭` `🧪` | Prevents implicitly passing the 'key' prop to components                                                                      |          |
 | [`no-leaked-conditional-rendering`](no-leaked-conditional-rendering)               | 0️⃣ 0️⃣ |    `💭`   | Prevents problematic leaked values from being rendered                                                                        |          |
 | [`no-missing-component-display-name`](no-missing-component-display-name)           | 0️⃣ 0️⃣ |           | Enforces that all components have a `displayName` that can be used in DevTools                                                |          |
 | [`no-missing-context-display-name`](no-missing-context-display-name)               | 0️⃣ 0️⃣ |    `🔧`   | Enforces that all contexts have a `displayName` that can be used in DevTools                                                  |          |
@@ -121,9 +121,9 @@ full: true
   RSC rules target [React Server Components](https://react.dev/reference/rsc/server-components), [React Server Functions](https://react.dev/reference/rsc/server-functions) and RSC [Directives](https://react.dev/reference/rsc/directives).
 </Callout>
 
-| Rule                                                                               |    ✅    |     🌟    | Description                                                                                                                   |  `react` |
-| :--------------------------------------------------------------------------------- | :-----: | :-------: | :---------------------------------------------------------------------------------------------------------------------------- | :------: |
-| [`function-definition`](rsc-function-definition)                                   | 0️⃣ 0️⃣ | `🔧` `🧪` | Validate and transform React Client/Server Function definitions                                                                    | >=19.0.0  |
+| Rule                                             |    ✅    |     🌟    | Description                                                     |  `react` |
+| :----------------------------------------------- | :-----: | :-------: | :-------------------------------------------------------------- | :------: |
+| [`function-definition`](rsc-function-definition) | 0️⃣ 0️⃣ | `🔧` `🧪` | Validate and transform React Client/Server Function definitions | >=19.0.0 |
 
 ## Web API Rules
 

packages/plugins/eslint-plugin-react-x/src/configs/recommended-type-checked.ts

@@ -6,6 +6,7 @@ export const name = "react-x/recommended-type-checked";
 
 export const rules = {
   ...recommendedTypescript.rules,
+  "react-x/no-implicit-key": "error",
   "react-x/no-leaked-conditional-rendering": "error",
 } as const satisfies Record<string, RuleConfig>;
 

packages/plugins/eslint-plugin-react-x/src/configs/strict-type-checked.ts

@@ -6,6 +6,7 @@ export const name = "react-x/strict-type-checked";
 
 export const rules = {
   ...strictTypeScript.rules,
+  "react-x/no-implicit-key": "error",
   "react-x/no-leaked-conditional-rendering": "error",
   "react-x/no-unused-props": "warn",
 } as const satisfies Record<string, RuleConfig>;

packages/plugins/eslint-plugin-react-x/src/rules/jsx-key-before-spread.mdx

@@ -69,4 +69,4 @@ If the `key` prop is before any spread props, it is passed as the `key` argument
 ## See Also
 
 - [`no-implicit-key`](./no-implicit-key)\
-  Prevents `key` from not being explicitly specified (e.g., spreading `key` from objects).
+  Prevents implicitly passing the 'key' prop to components.

packages/plugins/eslint-plugin-react-x/src/rules/no-array-index-key.mdx

@@ -90,4 +90,4 @@ function MyComponent({ items }: MyComponentProps) {
 - [`no-duplicate-key`](./no-duplicate-key)\
   Prevents duplicate `key` props on sibling elements when rendering lists.
 - [`no-implicit-key`](./no-implicit-key)\
-  Prevents `key` from not being explicitly specified (e.g., spreading `key` from objects).
+  Prevents implicitly passing the 'key' prop to components.

packages/plugins/eslint-plugin-react-x/src/rules/no-duplicate-key.mdx

@@ -126,7 +126,7 @@ function MyComponent() {
 - [`no-missing-key`](./no-missing-key)\
   Prevents missing `key` on items in list rendering.
 - [`no-implicit-key`](./no-implicit-key)\
-  Prevents `key` from not being explicitly specified (e.g., spreading `key` from objects).
+  Prevents implicitly passing the 'key' prop to components.
 - [`no-array-index-key`](./no-array-index-key)\
   Warns when an array `index` is used as a `key` prop.
 - [`no-unnecessary-key`](./no-unnecessary-key)\

packages/plugins/eslint-plugin-react-x/src/rules/no-implicit-key.mdx

@@ -16,21 +16,16 @@ react-x/no-implicit-key
 
 **Features**
 
-`🧪`
+`💭` `🧪`
 
 **Presets**
 
-`x`
-`recommended`
-`recommended-typescript`
 `recommended-type-checked`
-`strict`
-`strict-typescript`
 `strict-type-checked`
 
 ## Description
 
-Prevents `key` from not being explicitly specified (e.g., spreading `key` from objects).
+Prevents implicitly passing the 'key' prop to components.
 
 This makes it hard to see whether the key was passed correctly to the element or where it came from.
 
@@ -43,21 +38,17 @@ It is also proposed to be deprecated in this RFC: [Deprecate spreading key from
 ```tsx
 import React from "react";
 
-interface MyComponentProps {
-  items: { id: string; name: string }[];
+interface Foo = { key?: string; }
+
+interface MyComponentProps extends Foo {
+  className: string;
+  children: React.ReactNode;
 }
 
-function MyComponent({ items }: MyComponentProps) {
-  return (
-    <ul>
-      {items.map(({ id, name, ...rest }) => {
-        const props = { key: id, ...rest };
-        return <li {...props}>{name}</li>;
-        //         ^^^^^^^^^^
-        //         - Do not use implicit 'key' props.
-      })}
-    </ul>
-  );
+function MyComponent(props: MyComponentProps) {
+  return <div {...props} />;
+  //          ^^^^^^^^^^
+  //          - This spread attribute implicitly passes the 'key' prop to a component, this could lead to unexpected behavior. If you intend to pass the 'key' prop, use 'key={value}'.
 }
 ```
 
@@ -66,18 +57,15 @@ function MyComponent({ items }: MyComponentProps) {
 ```tsx
 import React from "react";
 
-interface MyComponentProps {
-  items: { id: string; name: string }[];
+interface Foo = { key?: string; }
+
+interface MyComponentProps extends Foo {
+  className: string;
+  children: React.ReactNode;
 }
 
-function MyComponent({ items }: MyComponentProps) {
-  return (
-    <ul>
-      {items.map(({ id, name, ...rest }) => {
-        return <li key={id} {...rest}>{name}</li>;
-      })}
-    </ul>
-  );
+function MyComponent({ key, ...rest }) {
+  return <div {...rest} />;
 }
 ```
 

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

@@ -1,9 +1,9 @@
 import tsx from "dedent";
 
-import { allValid, ruleTester } from "../../../../../test";
+import { allValid, ruleTesterWithTypes } from "../../../../../test";
 import rule, { RULE_NAME } from "./no-implicit-key";
 
-ruleTester.run(RULE_NAME, rule, {
+ruleTesterWithTypes.run(RULE_NAME, rule, {
   invalid: [
     {
       code: tsx`

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

@@ -1,14 +1,15 @@
-import * as core from "@eslint-react/core";
 import type { RuleContext, RuleFeature } from "@eslint-react/shared";
-import type { TSESTree } from "@typescript-eslint/types";
-import { AST_NODE_TYPES as AST } from "@typescript-eslint/types";
+import { getConstrainedTypeAtLocation } from "@typescript-eslint/type-utils";
+import { ESLintUtils } from "@typescript-eslint/utils";
 import type { RuleListener } from "@typescript-eslint/utils/ts-eslint";
+import { unionConstituents } from "ts-api-utils";
 
 import { createRule } from "../utils";
 
 export const RULE_NAME = "no-implicit-key";
 
 export const RULE_FEATURES = [
+  "TSC",
   "EXP",
 ] as const satisfies RuleFeature[];
 
@@ -18,10 +19,11 @@ export default createRule<[], MessageID>({
   meta: {
     type: "problem",
     docs: {
-      description: "Prevents 'key' from not being explicitly specified (e.g., spreading 'key' from objects).",
+      description: "Prevents implicitly passing the 'key' prop to components.",
     },
     messages: {
-      default: "Do not use implicit 'key' props.",
+      default:
+        "This spread attribute implicitly passes the 'key' prop to a component, this could lead to unexpected behavior. If you intend to pass the 'key' prop, use 'key={value}'.",
     },
     schema: [],
   },
@@ -30,23 +32,18 @@ export default createRule<[], MessageID>({
   defaultOptions: [],
 });
 
-// TODO: Rewrite the rule to use type checking
 export function create(context: RuleContext<MessageID, []>): RuleListener {
+  const services = ESLintUtils.getParserServices(context, false);
   return {
-    JSXOpeningElement(node: TSESTree.JSXOpeningElement) {
-      // Find the 'key' prop, including those from spread attributes
-      const keyProp = core.getJsxAttribute(context, node.parent)("key");
-      // Check if the 'key' prop is explicitly defined on the element
-      const isKeyPropOnElement = node.attributes
-        .some((n) =>
-          n.type === AST.JSXAttribute
-          && n.name.type === AST.JSXIdentifier
-          && n.name.name === "key"
-        );
-      // If a 'key' prop exists but is not explicitly on the element, it's implicit
-      if (keyProp != null && !isKeyPropOnElement) {
-        // Report an error for the implicit 'key'
-        context.report({ messageId: "default", node: keyProp });
+    JSXSpreadAttribute(node) {
+      for (const type of unionConstituents(getConstrainedTypeAtLocation(services, node.argument))) {
+        if (type.getProperty("key") != null) {
+          context.report({
+            messageId: "default",
+            node,
+          });
+          break;
+        }
       }
     },
   };

packages/plugins/eslint-plugin-react-x/src/rules/no-missing-key.mdx

@@ -84,6 +84,6 @@ function MyComponent({ items }: MyComponentProps) {
 - [`no-duplicate-key`](./no-duplicate-key)\
   Prevents duplicate `key` props on sibling elements when rendering lists.
 - [`no-implicit-key`](./no-implicit-key)\
-  Prevents `key` from not being explicitly specified (e.g., spreading `key` from objects).
+  Prevents implicitly passing the 'key' prop to components.
 - [`no-array-index-key`](./no-array-index-key)\
   Warns when an array `index` is used as a `key` prop.