[compiler] Aggregate error reporting, separate eslint rules

All error messages that are not invariants, todos, or config validation errors are now moved to a registry.  This helps us check that error messages and linked documentation is uniform and up to date. This also lets us link error codes to linter reporting categories and break the single `react-compiler` rule up into many smaller rules.


Changes to error reporting in React Compiler:
- instead of writing raw error message, add a new ErrorCode to `CompilerErrorCodes`
- create errors by constructing a `CompilerErrorDetailOptions` with the correct error code, or calling `CompilerErrorDetail.fromCode()` / `CompilerDiagnostic.fromCode()`.

Changes to linter:
- `react-compiler` lint rule is now broken up into many smaller lint rules, each with their own test cases.
- linting no longer fails early when a non-fatal error is find. Instead, we now log and move on to lint for more errors!

Todos:
- Codebases previously using the eslint plugin may already have `react-compiler` eslint suppressions. Since this PR removes this rule, these codebases may get new eslint warnings on already-suppressed ranges. Some options I can think of:
  - Force everyone to add the new suppressions
  - Release a script to rewrite previous suppressions to the new suppressions. Codebases that had incomplete suppressions / waited to update may benefit here, as this still surfaces errors on previously-unseen suppressions
  - Hard code logic in our eslint plugin to avoid reporting if we see a `eslint-disable-next-line react-compiler`. This doesn't feel ideal
- Align on eslint plugin rule names and error code mappings
  - I aggregated a few error codes here. For example, reassigning and modifying local variables after render now have the same error code + message. See changes fixtures for more examples
- More test cases for linter rules. Will add these once lint rules are more finalized

Author: mofeiZ

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

@@ -38,10 +38,10 @@ export function validateRestrictedImports(
     ImportDeclaration(importDeclPath) {
       if (restrictedImports.has(importDeclPath.node.source.value)) {
         error.push({
-          severity: ErrorSeverity.Todo,
           reason: 'Bailing out due to blocklisted import',
           description: `Import from module ${importDeclPath.node.source.value}`,
           loc: importDeclPath.node.loc ?? null,
+          severity: ErrorSeverity.Todo,
         });
       }
     },
@@ -205,10 +205,10 @@ export class ProgramContext {
     }
     const error = new CompilerError();
     error.push({
-      severity: ErrorSeverity.Todo,
       reason: 'Encountered conflicting global in generated program',
       description: `Conflict from local binding ${name}`,
       loc: scope.getBinding(name)?.path.node.loc ?? null,
+      severity: ErrorSeverity.Todo,
       suggestions: null,
     });
     return Err(error);

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts

@@ -11,7 +11,7 @@ import {
   CompilerDiagnostic,
   CompilerError,
   CompilerErrorDetail,
-  CompilerErrorDetailOptions,
+  PlainCompilerErrorDetailOptions,
 } from '../CompilerError';
 import {
   EnvironmentConfig,
@@ -107,6 +107,8 @@ export type PluginOptions = {
    * passes.
    *
    * Defaults to false
+   *
+   * TODO: rename this to lintOnly or something similar
    */
   noEmit: boolean;
 
@@ -234,7 +236,10 @@ export type CompileErrorEvent = {
 export type CompileDiagnosticEvent = {
   kind: 'CompileDiagnostic';
   fnLoc: t.SourceLocation | null;
-  detail: Omit<Omit<CompilerErrorDetailOptions, 'severity'>, 'suggestions'>;
+  detail: Omit<
+    Omit<PlainCompilerErrorDetailOptions, 'severity'>,
+    'suggestions'
+  >;
 };
 export type CompileSuccessEvent = {
   kind: 'CompileSuccess';

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts

@@ -100,7 +100,6 @@ import {outlineJSX} from '../Optimization/OutlineJsx';
 import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
 import {transformFire} from '../Transform';
 import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
-import {CompilerError} from '..';
 import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
 import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions';
 import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects';
@@ -174,7 +173,7 @@ function runWithEnvironment(
     !env.config.disableMemoizationForDebugging &&
     !env.config.enableChangeDetectionForDebugging
   ) {
-    dropManualMemoization(hir).unwrap();
+    env.logOrThrowErrors(dropManualMemoization(hir));
     log({kind: 'hir', name: 'DropManualMemoization', value: hir});
   }
 
@@ -207,10 +206,10 @@ function runWithEnvironment(
 
   if (env.isInferredMemoEnabled) {
     if (env.config.validateHooksUsage) {
-      validateHooksUsage(hir).unwrap();
+      env.logOrThrowErrors(validateHooksUsage(hir));
     }
-    if (env.config.validateNoCapitalizedCalls) {
-      validateNoCapitalizedCalls(hir).unwrap();
+    if (env.config.validateNoCapitalizedCalls != null) {
+      env.logOrThrowErrors(validateNoCapitalizedCalls(hir));
     }
   }
 
@@ -230,20 +229,17 @@ function runWithEnvironment(
   log({kind: 'hir', name: 'AnalyseFunctions', value: hir});
 
   if (!env.config.enableNewMutationAliasingModel) {
-    const fnEffectErrors = inferReferenceEffects(hir);
+    const fnEffectResult = inferReferenceEffects(hir);
+
     if (env.isInferredMemoEnabled) {
-      if (fnEffectErrors.length > 0) {
-        CompilerError.throw(fnEffectErrors[0]);
-      }
+      env.logOrThrowErrors(fnEffectResult);
     }
     log({kind: 'hir', name: 'InferReferenceEffects', value: hir});
   } else {
     const mutabilityAliasingErrors = inferMutationAliasingEffects(hir);
     log({kind: 'hir', name: 'InferMutationAliasingEffects', value: hir});
     if (env.isInferredMemoEnabled) {
-      if (mutabilityAliasingErrors.isErr()) {
-        throw mutabilityAliasingErrors.unwrapErr();
-      }
+      env.logOrThrowErrors(mutabilityAliasingErrors);
     }
   }
 
@@ -272,10 +268,8 @@ function runWithEnvironment(
     });
     log({kind: 'hir', name: 'InferMutationAliasingRanges', value: hir});
     if (env.isInferredMemoEnabled) {
-      if (mutabilityAliasingErrors.isErr()) {
-        throw mutabilityAliasingErrors.unwrapErr();
-      }
-      validateLocalsNotReassignedAfterRender(hir);
+      env.logOrThrowErrors(mutabilityAliasingErrors.map(() => undefined));
+      env.logOrThrowErrors(validateLocalsNotReassignedAfterRender(hir));
     }
   }
 
@@ -285,15 +279,15 @@ function runWithEnvironment(
     }
 
     if (env.config.validateRefAccessDuringRender) {
-      validateNoRefAccessInRender(hir).unwrap();
+      env.logOrThrowErrors(validateNoRefAccessInRender(hir));
     }
 
     if (env.config.validateNoSetStateInRender) {
-      validateNoSetStateInRender(hir).unwrap();
+      env.logOrThrowErrors(validateNoSetStateInRender(hir));
     }
 
     if (env.config.validateNoDerivedComputationsInEffects) {
-      validateNoDerivedComputationsInEffects(hir);
+      env.logOrThrowErrors(validateNoDerivedComputationsInEffects(hir));
     }
 
     if (env.config.validateNoSetStateInEffects) {
@@ -303,16 +297,18 @@ function runWithEnvironment(
     if (env.config.validateNoJSXInTryStatements) {
       env.logErrors(validateNoJSXInTryStatement(hir));
     }
-
-    if (env.config.validateNoImpureFunctionsInRender) {
-      validateNoImpureFunctionsInRender(hir).unwrap();
+    if (
+      env.config.validateNoImpureFunctionsInRender &&
+      !env.config.enableNewMutationAliasingModel
+    ) {
+      env.logOrThrowErrors(validateNoImpureFunctionsInRender(hir));
     }
 
     if (
       env.config.validateNoFreezingKnownMutableFunctions ||
       env.config.enableNewMutationAliasingModel
     ) {
-      validateNoFreezingKnownMutableFunctions(hir).unwrap();
+      env.logOrThrowErrors(validateNoFreezingKnownMutableFunctions(hir));
     }
   }
 

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

@@ -7,11 +7,7 @@
 
 import {NodePath} from '@babel/core';
 import * as t from '@babel/types';
-import {
-  CompilerError,
-  CompilerErrorDetail,
-  ErrorSeverity,
-} from '../CompilerError';
+import {CompilerError, ErrorSeverity} from '../CompilerError';
 import {ExternalFunction, ReactFunctionType} from '../HIR/Environment';
 import {CodegenFunction} from '../ReactiveScopes';
 import {isComponentDeclaration} from '../Utils/ComponentDeclaration';
@@ -32,6 +28,7 @@ import {
 } from './Suppression';
 import {GeneratedSource} from '../HIR';
 import {Err, Ok, Result} from '../Utils/Result';
+import {ErrorCode} from '../Utils/CompilerErrorCodes';
 
 export type CompilerPass = {
   opts: PluginOptions;
@@ -101,12 +98,9 @@ function findDirectivesDynamicGating(
       if (t.isValidIdentifier(maybeMatch[1])) {
         result.push({directive, match: maybeMatch[1]});
       } else {
-        errors.push({
-          reason: `Dynamic gating directive is not a valid JavaScript identifier`,
+        errors.pushErrorCode(ErrorCode.DYNAMIC_GATING_IS_NOT_IDENTIFIER, {
           description: `Found '${directive.value.value}'`,
-          severity: ErrorSeverity.InvalidReact,
           loc: directive.loc ?? null,
-          suggestions: null,
         });
       }
     }
@@ -115,14 +109,11 @@ function findDirectivesDynamicGating(
     return Err(errors);
   } else if (result.length > 1) {
     const error = new CompilerError();
-    error.push({
-      reason: `Multiple dynamic gating directives found`,
+    error.pushErrorCode(ErrorCode.DYNAMIC_GATING_MULTIPLE_DIRECTIVES, {
       description: `Expected a single directive but found [${result
         .map(r => r.directive.value.value)
         .join(', ')}]`,
-      severity: ErrorSeverity.InvalidReact,
       loc: result[0].directive.loc ?? null,
-      suggestions: null,
     });
     return Err(error);
   } else if (result.length === 1) {
@@ -451,14 +442,12 @@ export function compileProgram(
   if (programContext.hasModuleScopeOptOut) {
     if (compiledFns.length > 0) {
       const error = new CompilerError();
-      error.pushErrorDetail(
-        new CompilerErrorDetail({
-          reason:
-            'Unexpected compiled functions when module scope opt-out is present',
-          severity: ErrorSeverity.Invariant,
-          loc: null,
-        }),
-      );
+      error.push({
+        reason:
+          'Unexpected compiled functions when module scope opt-out is present',
+        severity: ErrorSeverity.Invariant,
+        loc: null,
+      });
       handleError(error, programContext, null);
     }
     return null;
@@ -591,9 +580,7 @@ function processFn(
   let compiledFn: CodegenFunction;
   const compileResult = tryCompileFunction(fn, fnType, programContext);
   if (compileResult.kind === 'error') {
-    if (directives.optOut != null) {
-      logError(compileResult.error, programContext, fn.node.loc ?? null);
-    } else {
+    if (directives.optOut == null) {
       handleError(compileResult.error, programContext, fn.node.loc ?? null);
     }
     const retryResult = retryCompileFunction(fn, fnType, programContext);
@@ -692,7 +679,7 @@ function tryCompileFunction(
         fn,
         programContext.opts.environment,
         fnType,
-        'all_features',
+        programContext.opts.noEmit ? 'lint_only' : 'all_features',
         programContext,
         programContext.opts.logger,
         programContext.filename,
@@ -805,15 +792,7 @@ function shouldSkipCompilation(
   if (pass.opts.sources) {
     if (pass.filename === null) {
       const error = new CompilerError();
-      error.pushErrorDetail(
-        new CompilerErrorDetail({
-          reason: `Expected a filename but found none.`,
-          description:
-            "When the 'sources' config options is specified, the React compiler will only compile files with a name",
-          severity: ErrorSeverity.InvalidConfig,
-          loc: null,
-        }),
-      );
+      error.pushErrorCode(ErrorCode.FILENAME_NOT_SET);
       handleError(error, pass, null);
       return true;
     }

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

@@ -11,7 +11,7 @@ import {
   CompilerDiagnostic,
   CompilerError,
   CompilerSuggestionOperation,
-  ErrorSeverity,
+  ErrorCode,
 } from '../CompilerError';
 import {assertExhaustive} from '../Utils/utils';
 import {GeneratedSource} from '../HIR';
@@ -165,14 +165,12 @@ export function suppressionsToCompilerError(
     let reason, suggestion;
     switch (suppressionRange.source) {
       case 'Eslint':
-        reason =
-          'React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled';
+        reason = ErrorCode.BAILOUT_ESLINT_SUPPRESSION;
         suggestion =
           'Remove the ESLint suppression and address the React error';
         break;
       case 'Flow':
-        reason =
-          'React Compiler has skipped optimizing this component because one or more React rule violations were reported by Flow';
+        reason = ErrorCode.BAILOUT_FLOW_SUPPRESSION;
         suggestion = 'Remove the Flow suppression and address the React error';
         break;
       default:
@@ -182,10 +180,8 @@ export function suppressionsToCompilerError(
         );
     }
     error.pushDiagnostic(
-      CompilerDiagnostic.create({
-        category: reason,
-        description: `React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior. Found suppression \`${suppressionRange.disableComment.value.trim()}\``,
-        severity: ErrorSeverity.InvalidReact,
+      CompilerDiagnostic.fromCode(reason, {
+        description: `Found suppression \`${suppressionRange.disableComment.value.trim()}\``,
         suggestions: [
           {
             description: suggestion,

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts

@@ -8,27 +8,24 @@
 import {NodePath} from '@babel/core';
 import * as t from '@babel/types';
 
-import {CompilerError, EnvironmentConfig, ErrorSeverity, Logger} from '..';
+import {CompilerError, EnvironmentConfig, Logger} from '..';
 import {getOrInsertWith} from '../Utils/utils';
 import {Environment, GeneratedSource} from '../HIR';
 import {DEFAULT_EXPORT} from '../HIR/Environment';
 import {CompileProgramMetadata} from './Program';
-import {CompilerDiagnostic, CompilerDiagnosticOptions} from '../CompilerError';
+import {CompilerDiagnostic} from '../CompilerError';
+import {ErrorCode} from '../Utils/CompilerErrorCodes';
 
-function throwInvalidReact(
-  options: Omit<CompilerDiagnosticOptions, 'severity'>,
+function logAndThrowDiagnostic(
+  diagnostic: CompilerDiagnostic,
   {logger, filename}: TraversalState,
 ): never {
-  const detail: CompilerDiagnosticOptions = {
-    severity: ErrorSeverity.InvalidReact,
-    ...options,
-  };
   logger?.logEvent(filename, {
     kind: 'CompileError',
     fnLoc: null,
-    detail: new CompilerDiagnostic(detail),
+    detail: diagnostic,
   });
-  CompilerError.throwDiagnostic(detail);
+  CompilerError.throwDiagnostic(diagnostic);
 }
 
 function isAutodepsSigil(
@@ -90,21 +87,17 @@ function assertValidEffectImportReference(
          * as it may have already been transformed by the compiler (and not
          * memoized).
          */
-        throwInvalidReact(
-          {
-            category:
-              'Cannot infer dependencies of this effect. This will break your build!',
-            description:
-              'To resolve, either pass a dependency array or fix reported compiler bailout diagnostics.' +
-              (maybeErrorDiagnostic ? ` ${maybeErrorDiagnostic}` : ''),
+        logAndThrowDiagnostic(
+          CompilerDiagnostic.fromCode(ErrorCode.DID_NOT_INFER_DEPS, {
+            description: maybeErrorDiagnostic ? `${maybeErrorDiagnostic}` : '',
             details: [
               {
                 kind: 'error',
                 message: 'Cannot infer dependencies',
                 loc: parent.node.loc ?? GeneratedSource,
               },
             ],
-          },
+          }),
           context,
         );
       }
@@ -121,10 +114,8 @@ function assertValidFireImportReference(
       paths[0],
       context.transformErrors,
     );
-    throwInvalidReact(
-      {
-        category:
-          '[Fire] Untransformed reference to compiler-required feature.',
+    logAndThrowDiagnostic(
+      CompilerDiagnostic.fromCode(ErrorCode.CANNOT_COMPILE_FIRE, {
         description:
           'Either remove this `fire` call or ensure it is successfully transformed by the compiler' +
           maybeErrorDiagnostic
@@ -137,7 +128,7 @@ function assertValidFireImportReference(
             loc: paths[0].node.loc ?? GeneratedSource,
           },
         ],
-      },
+      }),
       context,
     );
   }