[compiler][rfc] Switch eslint-plugin-react-compiler to single rule for Meta compat

Now that the official way to use React Compiler's linting is via `eslint-plugin-react-hooks` v7, eslint-plugin-react-compiler isn't strictly necessary anymore. However, at Meta our linting setup makes it a bit tedious to use the current eprh setup with lots of rules. Here I'm experimenting with making eslint-plugin-react-compiler just report all issues under one rule, to make it a bit easier to sync internally.

Unclear if we even need to land this or just use it to help figure out the migration.

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts

@@ -184,25 +184,28 @@ function validateNoContextVariableAssignment(
   fn: HIRFunction,
   errors: CompilerError,
 ): void {
+  const context = new Set(fn.context.map(place => place.identifier.id));
   for (const block of fn.body.blocks.values()) {
     for (const instr of block.instructions) {
       const value = instr.value;
       switch (value.kind) {
         case 'StoreContext': {
-          errors.pushDiagnostic(
-            CompilerDiagnostic.create({
-              category: ErrorCategory.UseMemo,
-              reason:
-                'useMemo() callbacks may not reassign variables declared outside of the callback',
-              description:
-                'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function',
-              suggestions: null,
-            }).withDetails({
-              kind: 'error',
-              loc: value.lvalue.place.loc,
-              message: 'Cannot reassign variable',
-            }),
-          );
+          if (context.has(value.lvalue.place.identifier.id)) {
+            errors.pushDiagnostic(
+              CompilerDiagnostic.create({
+                category: ErrorCategory.UseMemo,
+                reason:
+                  'useMemo() callbacks may not reassign variables declared outside of the callback',
+                description:
+                  'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function',
+                suggestions: null,
+              }).withDetails({
+                kind: 'error',
+                loc: value.lvalue.place.loc,
+                message: 'Cannot reassign variable',
+              }),
+            );
+          }
           break;
         }
       }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md

@@ -0,0 +1,45 @@
+
+## Input
+
+```javascript
+// @flow
+export hook useItemLanguage(items) {
+  return useMemo(() => {
+    let language: ?string = null;
+    items.forEach(item => {
+      if (item.language != null) {
+        language = item.language;
+      }
+    });
+    return language;
+  }, [items]);
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+export function useItemLanguage(items) {
+  const $ = _c(2);
+  let language;
+  if ($[0] !== items) {
+    language = null;
+    items.forEach((item) => {
+      if (item.language != null) {
+        language = item.language;
+      }
+    });
+    $[0] = items;
+    $[1] = language;
+  } else {
+    language = $[1];
+  }
+  return language;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js

@@ -0,0 +1,12 @@
+// @flow
+export hook useItemLanguage(items) {
+  return useMemo(() => {
+    let language: ?string = null;
+    items.forEach(item => {
+      if (item.language != null) {
+        language = item.language;
+      }
+    });
+    return language;
+  }, [items]);
+}

compiler/packages/eslint-plugin-react-compiler/__tests__/ImpureFunctionCallsRule-test.ts

@@ -5,22 +5,15 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {
-  ErrorCategory,
-  getRuleForCategory,
-} from 'babel-plugin-react-compiler/src/CompilerError';
 import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
-import {allRules} from '../src/rules/ReactCompilerRule';
+import ReactCompilerRule from '../src/rules/ReactCompilerRule';
 
-testRule(
-  'no impure function calls rule',
-  allRules[getRuleForCategory(ErrorCategory.Purity).name].rule,
-  {
-    valid: [],
-    invalid: [
-      {
-        name: 'Known impure function calls are caught',
-        code: normalizeIndent`
+testRule('no impure function calls rule', ReactCompilerRule, {
+  valid: [],
+  invalid: [
+    {
+      name: 'Known impure function calls are caught',
+      code: normalizeIndent`
       function Component() {
         const date = Date.now();
         const now = performance.now();
@@ -28,12 +21,11 @@ testRule(
         return <Foo date={date} now={now} rand={rand} />;
       }
     `,
-        errors: [
-          makeTestCaseError('Cannot call impure function during render'),
-          makeTestCaseError('Cannot call impure function during render'),
-          makeTestCaseError('Cannot call impure function during render'),
-        ],
-      },
-    ],
-  },
-);
+      errors: [
+        makeTestCaseError('Cannot call impure function during render'),
+        makeTestCaseError('Cannot call impure function during render'),
+        makeTestCaseError('Cannot call impure function during render'),
+      ],
+    },
+  ],
+});

compiler/packages/eslint-plugin-react-compiler/__tests__/InvalidHooksRule-test.ts

@@ -5,30 +5,23 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {
-  ErrorCategory,
-  getRuleForCategory,
-} from 'babel-plugin-react-compiler/src/CompilerError';
 import {normalizeIndent, makeTestCaseError, testRule} from './shared-utils';
-import {allRules} from '../src/rules/ReactCompilerRule';
+import {AllRules} from '../src/rules/ReactCompilerRule';
 
-testRule(
-  'rules-of-hooks',
-  allRules[getRuleForCategory(ErrorCategory.Hooks).name].rule,
-  {
-    valid: [
-      {
-        name: 'Basic example',
-        code: normalizeIndent`
+testRule('rules-of-hooks', AllRules, {
+  valid: [
+    {
+      name: 'Basic example',
+      code: normalizeIndent`
         function Component() {
           useHook();
           return <div>Hello world</div>;
         }
       `,
-      },
-      {
-        name: 'Violation with Flow suppression',
-        code: `
+    },
+    {
+      name: 'Violation with Flow suppression',
+      code: `
       // Valid since error already suppressed with flow.
       function useHook() {
         if (cond) {
@@ -37,11 +30,11 @@ testRule(
         }
       }
     `,
-      },
-      {
-        // OK because invariants are only meant for the compiler team's consumption
-        name: '[Invariant] Defined after use',
-        code: normalizeIndent`
+    },
+    {
+      // OK because invariants are only meant for the compiler team's consumption
+      name: '[Invariant] Defined after use',
+      code: normalizeIndent`
         function Component(props) {
           let y = function () {
             m(x);
@@ -52,49 +45,42 @@ testRule(
           return y;
         }
       `,
-      },
-      {
-        name: "Classes don't throw",
-        code: normalizeIndent`
+    },
+    {
+      name: "Classes don't throw",
+      code: normalizeIndent`
         class Foo {
           #bar() {}
         }
       `,
-      },
-    ],
-    invalid: [
-      {
-        name: 'Simple violation',
-        code: normalizeIndent`
+    },
+  ],
+  invalid: [
+    {
+      name: 'Simple violation',
+      code: normalizeIndent`
       function useConditional() {
         if (cond) {
           useConditionalHook();
         }
       }
     `,
-        errors: [
-          makeTestCaseError(
-            'Hooks must always be called in a consistent order',
-          ),
-        ],
-      },
-      {
-        name: 'Multiple diagnostics within the same function are surfaced',
-        code: normalizeIndent`
+      errors: [
+        makeTestCaseError('Hooks must always be called in a consistent order'),
+      ],
+    },
+    {
+      name: 'Multiple diagnostics within the same function are surfaced',
+      code: normalizeIndent`
         function useConditional() {
           cond ?? useConditionalHook();
           props.cond && useConditionalHook();
           return <div>Hello world</div>;
         }`,
-        errors: [
-          makeTestCaseError(
-            'Hooks must always be called in a consistent order',
-          ),
-          makeTestCaseError(
-            'Hooks must always be called in a consistent order',
-          ),
-        ],
-      },
-    ],
-  },
-);
+      errors: [
+        makeTestCaseError('Hooks must always be called in a consistent order'),
+        makeTestCaseError('Hooks must always be called in a consistent order'),
+      ],
+    },
+  ],
+});

compiler/packages/eslint-plugin-react-compiler/__tests__/NoAmbiguousJsxRule-test.ts

@@ -5,22 +5,15 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {
-  ErrorCategory,
-  getRuleForCategory,
-} from 'babel-plugin-react-compiler/src/CompilerError';
 import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
-import {allRules} from '../src/rules/ReactCompilerRule';
+import ReactCompilerRule from '../src/rules/ReactCompilerRule';
 
-testRule(
-  'no ambiguous JSX rule',
-  allRules[getRuleForCategory(ErrorCategory.ErrorBoundaries).name].rule,
-  {
-    valid: [],
-    invalid: [
-      {
-        name: 'JSX in try blocks are warned against',
-        code: normalizeIndent`
+testRule('no ambiguous JSX rule', ReactCompilerRule, {
+  valid: [],
+  invalid: [
+    {
+      name: 'JSX in try blocks are warned against',
+      code: normalizeIndent`
       function Component(props) {
         let el;
         try {
@@ -31,8 +24,7 @@ testRule(
         return el;
       }
     `,
-        errors: [makeTestCaseError('Avoid constructing JSX within try/catch')],
-      },
-    ],
-  },
-);
+      errors: [makeTestCaseError('Avoid constructing JSX within try/catch')],
+    },
+  ],
+});

compiler/packages/eslint-plugin-react-compiler/__tests__/NoCapitalizedCallsRule-test.ts

@@ -4,22 +4,15 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */
-import {
-  ErrorCategory,
-  getRuleForCategory,
-} from 'babel-plugin-react-compiler/src/CompilerError';
 import {normalizeIndent, makeTestCaseError, testRule} from './shared-utils';
-import {allRules} from '../src/rules/ReactCompilerRule';
+import ReactCompilerRule from '../src/rules/ReactCompilerRule';
 
-testRule(
-  'no-capitalized-calls',
-  allRules[getRuleForCategory(ErrorCategory.CapitalizedCalls).name].rule,
-  {
-    valid: [],
-    invalid: [
-      {
-        name: 'Simple violation',
-        code: normalizeIndent`
+testRule('no-capitalized-calls', ReactCompilerRule, {
+  valid: [],
+  invalid: [
+    {
+      name: 'Simple violation',
+      code: normalizeIndent`
         import Child from './Child';
         function Component() {
           return <>
@@ -27,15 +20,13 @@ testRule(
           </>;
         }
       `,
-        errors: [
-          makeTestCaseError(
-            'Capitalized functions are reserved for components',
-          ),
-        ],
-      },
-      {
-        name: 'Method call violation',
-        code: normalizeIndent`
+      errors: [
+        makeTestCaseError('Capitalized functions are reserved for components'),
+      ],
+    },
+    {
+      name: 'Method call violation',
+      code: normalizeIndent`
         import myModule from './MyModule';
         function Component() {
           return <>
@@ -43,15 +34,13 @@ testRule(
           </>;
         }
       `,
-        errors: [
-          makeTestCaseError(
-            'Capitalized functions are reserved for components',
-          ),
-        ],
-      },
-      {
-        name: 'Multiple diagnostics within the same function are surfaced',
-        code: normalizeIndent`
+      errors: [
+        makeTestCaseError('Capitalized functions are reserved for components'),
+      ],
+    },
+    {
+      name: 'Multiple diagnostics within the same function are surfaced',
+      code: normalizeIndent`
         import Child1 from './Child1';
         import MyModule from './MyModule';
         function Component() {
@@ -60,12 +49,9 @@ testRule(
             {MyModule.Child2()}
           </>;
         }`,
-        errors: [
-          makeTestCaseError(
-            'Capitalized functions are reserved for components',
-          ),
-        ],
-      },
-    ],
-  },
-);
+      errors: [
+        makeTestCaseError('Capitalized functions are reserved for components'),
+      ],
+    },
+  ],
+});

compiler/packages/eslint-plugin-react-compiler/__tests__/NoRefAccessInRender-tests.ts

@@ -5,30 +5,22 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {
-  ErrorCategory,
-  getRuleForCategory,
-} from 'babel-plugin-react-compiler/src/CompilerError';
 import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
-import {allRules} from '../src/rules/ReactCompilerRule';
+import ReactCompilerRule from '../src/rules/ReactCompilerRule';
 
-testRule(
-  'no ref access in render rule',
-  allRules[getRuleForCategory(ErrorCategory.Refs).name].rule,
-  {
-    valid: [],
-    invalid: [
-      {
-        name: 'validate against simple ref access in render',
-        code: normalizeIndent`
+testRule('no ref access in render rule', ReactCompilerRule, {
+  valid: [],
+  invalid: [
+    {
+      name: 'validate against simple ref access in render',
+      code: normalizeIndent`
       function Component(props) {
         const ref = useRef(null);
         const value = ref.current;
         return value;
       }
     `,
-        errors: [makeTestCaseError('Cannot access refs during render')],
-      },
-    ],
-  },
-);
+      errors: [makeTestCaseError('Cannot access refs during render')],
+    },
+  ],
+});

compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts

@@ -5,19 +5,10 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {
-  ErrorCategory,
-  getRuleForCategory,
-} from 'babel-plugin-react-compiler/src/CompilerError';
-import {
-  normalizeIndent,
-  testRule,
-  makeTestCaseError,
-  TestRecommendedRules,
-} from './shared-utils';
-import {allRules} from '../src/rules/ReactCompilerRule';
+import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
+import {AllRules} from '../src/rules/ReactCompilerRule';
 
-testRule('plugin-recommended', TestRecommendedRules, {
+testRule('plugin-recommended', AllRules, {
   valid: [
     {
       name: 'Basic example with component syntax',

compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRuleTypescript-test.ts

@@ -6,11 +6,8 @@
  */
 
 import {RuleTester} from 'eslint';
-import {
-  CompilerTestCases,
-  normalizeIndent,
-  TestRecommendedRules,
-} from './shared-utils';
+import {CompilerTestCases, normalizeIndent} from './shared-utils';
+import ReactCompilerRule from '../src/rules/ReactCompilerRule';
 
 const tests: CompilerTestCases = {
   valid: [
@@ -62,4 +59,4 @@ const eslintTester = new RuleTester({
   // @ts-ignore[2353] - outdated types
   parser: require.resolve('@typescript-eslint/parser'),
 });
-eslintTester.run('react-compiler', TestRecommendedRules, tests);
+eslintTester.run('react-compiler', ReactCompilerRule, tests);

compiler/packages/eslint-plugin-react-compiler/__tests__/shared-utils.ts

@@ -1,8 +1,5 @@
 import {RuleTester as ESLintTester, Rule} from 'eslint';
-import {type ErrorCategory} from 'babel-plugin-react-compiler/src/CompilerError';
 import escape from 'regexp.escape';
-import {configs} from '../src/index';
-import {allRules} from '../src/rules/ReactCompilerRule';
 
 /**
  * A string template tag that removes padding from the left side of multi-line strings
@@ -46,31 +43,4 @@ export function testRule(
   eslintTester.run(name, rule, tests);
 }
 
-/**
- * Aggregates all recommended rules from the plugin.
- */
-export const TestRecommendedRules: Rule.RuleModule = {
-  meta: {
-    type: 'problem',
-    docs: {
-      description: 'Disallow capitalized function calls',
-      category: 'Possible Errors',
-      recommended: true,
-    },
-    // validation is done at runtime with zod
-    schema: [{type: 'object', additionalProperties: true}],
-  },
-  create(context) {
-    for (const ruleConfig of Object.values(
-      configs.recommended.plugins['react-compiler'].rules,
-    )) {
-      const listener = ruleConfig.rule.create(context);
-      if (Object.entries(listener).length !== 0) {
-        throw new Error('TODO: handle rules that return listeners to eslint');
-      }
-    }
-    return {};
-  },
-};
-
 test('no test', () => {});

compiler/packages/eslint-plugin-react-compiler/src/index.ts

@@ -5,37 +5,24 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {type Linter} from 'eslint';
-import {
-  allRules,
-  mapErrorSeverityToESlint,
-  recommendedRules,
-} from './rules/ReactCompilerRule';
+import ReactCompilerRule from './rules/ReactCompilerRule';
 
-const meta = {
-  name: 'eslint-plugin-react-compiler',
-};
-
-const configs = {
-  recommended: {
-    plugins: {
-      'react-compiler': {
-        rules: allRules,
+module.exports = {
+  rules: {
+    'react-compiler': ReactCompilerRule,
+  },
+  configs: {
+    recommended: {
+      plugins: {
+        'react-compiler': {
+          rules: {
+            'react-compiler': ReactCompilerRule,
+          },
+        },
+      },
+      rules: {
+        'react-compiler/react-compiler': 'error',
       },
     },
-    rules: Object.fromEntries(
-      Object.entries(recommendedRules).map(([name, ruleConfig]) => {
-        return [
-          'react-compiler/' + name,
-          mapErrorSeverityToESlint(ruleConfig.severity),
-        ];
-      }),
-    ) as Record<string, Linter.StringSeverity>,
   },
 };
-
-const rules = Object.fromEntries(
-  Object.entries(allRules).map(([name, {rule}]) => [name, rule]),
-);
-
-export {configs, rules, meta};

compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts

@@ -14,7 +14,7 @@ import {
 import type {Linter, Rule} from 'eslint';
 import runReactCompiler, {RunCacheEntry} from '../shared/RunReactCompiler';
 import {
-  ErrorSeverity,
+  ErrorCategory,
   LintRulePreset,
   LintRules,
   type LintRule,
@@ -108,14 +108,15 @@ function hasFlowSuppression(
   return false;
 }
 
-function makeRule(rule: LintRule): Rule.RuleModule {
+function makeRule(rules: Array<LintRule>): Rule.RuleModule {
+  const categories = new Set(rules.map(rule => rule.category));
   const create = (context: Rule.RuleContext): Rule.RuleListener => {
     const result = getReactCompilerResult(context);
 
     for (const event of result.events) {
       if (event.kind === 'CompileError') {
         const detail = event.detail;
-        if (detail.category === rule.category) {
+        if (categories.has(detail.category)) {
           const loc = detail.primaryLocation();
           if (loc == null || typeof loc === 'symbol') {
             continue;
@@ -150,8 +151,8 @@ function makeRule(rule: LintRule): Rule.RuleModule {
     meta: {
       type: 'problem',
       docs: {
-        description: rule.description,
-        recommended: rule.preset === LintRulePreset.Recommended,
+        description: 'React Compiler diagnostics',
+        recommended: true,
       },
       fixable: 'code',
       hasSuggestions: true,
@@ -162,47 +163,13 @@ function makeRule(rule: LintRule): Rule.RuleModule {
   };
 }
 
-type RulesConfig = {
-  [name: string]: {rule: Rule.RuleModule; severity: ErrorSeverity};
-};
+export default makeRule(
+  LintRules.filter(
+    rule =>
+      rule.preset === LintRulePreset.Recommended ||
+      rule.preset === LintRulePreset.RecommendedLatest ||
+      rule.category === ErrorCategory.CapitalizedCalls,
+  ),
+);
 
-export const allRules: RulesConfig = LintRules.reduce((acc, rule) => {
-  acc[rule.name] = {rule: makeRule(rule), severity: rule.severity};
-  return acc;
-}, {} as RulesConfig);
-
-export const recommendedRules: RulesConfig = LintRules.filter(
-  rule => rule.preset === LintRulePreset.Recommended,
-).reduce((acc, rule) => {
-  acc[rule.name] = {rule: makeRule(rule), severity: rule.severity};
-  return acc;
-}, {} as RulesConfig);
-
-export const recommendedLatestRules: RulesConfig = LintRules.filter(
-  rule =>
-    rule.preset === LintRulePreset.Recommended ||
-    rule.preset === LintRulePreset.RecommendedLatest,
-).reduce((acc, rule) => {
-  acc[rule.name] = {rule: makeRule(rule), severity: rule.severity};
-  return acc;
-}, {} as RulesConfig);
-
-export function mapErrorSeverityToESlint(
-  severity: ErrorSeverity,
-): Linter.StringSeverity {
-  switch (severity) {
-    case ErrorSeverity.Error: {
-      return 'error';
-    }
-    case ErrorSeverity.Warning: {
-      return 'warn';
-    }
-    case ErrorSeverity.Hint:
-    case ErrorSeverity.Off: {
-      return 'off';
-    }
-    default: {
-      assertExhaustive(severity, `Unhandled severity: ${severity}`);
-    }
-  }
-}
+export const AllRules = makeRule(LintRules);