Option to bail on Flow react-rule suppressions

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

@@ -102,6 +102,8 @@ export type PluginOptions = {
    * even if the default ESLint is suppressed), pass an empty array.
    */
   eslintSuppressionRules?: Array<string> | null | undefined;
+
+  flowSuppressions: boolean;
 };
 
 const CompilationModeSchema = z.enum([
@@ -169,6 +171,7 @@ export const defaultOptions: PluginOptions = {
   noEmit: false,
   enableUseMemoCachePolyfill: false,
   eslintSuppressionRules: null,
+  flowSuppressions: false,
 } as const;
 
 export function parsePluginOptions(obj: unknown): PluginOptions {

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

@@ -20,15 +20,15 @@ import {
 import { CodegenFunction } from "../ReactiveScopes";
 import { isComponentDeclaration } from "../Utils/ComponentDeclaration";
 import { assertExhaustive } from "../Utils/utils";
-import {
-  filterEslintSuppressionsThatAffectFunction,
-  findProgramEslintSuppressions,
-  suppressionsToCompilerError,
-} from "./EslintSuppression";
 import { insertGatedFunctionDeclaration } from "./Gating";
 import { addImportsToProgram, updateUseMemoCacheImport } from "./Imports";
 import { PluginOptions, parsePluginOptions } from "./Options";
 import { compileFn } from "./Pipeline";
+import {
+  filterSuppressionsThatAffectFunction,
+  findProgramSuppressions,
+  suppressionsToCompilerError,
+} from "./Suppression";
 
 export type CompilerPass = {
   opts: PluginOptions;
@@ -196,11 +196,12 @@ export function compileProgram(
    * we may still need to run Forget's analysis on every function (even if we
    * have already encountered errors) for reporting.
    */
-  const eslintSuppressions = findProgramEslintSuppressions(
+  const suppressions = findProgramSuppressions(
     pass.comments,
-    options.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS
+    options.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS,
+    options.flowSuppressions,
   );
-  const lintError = suppressionsToCompilerError(eslintSuppressions);
+  const lintError = suppressionsToCompilerError(suppressions);
   let hasCriticalError = lintError != null;
   const compiledFns: CompileResult[] = [];
 
@@ -223,9 +224,9 @@ export function compileProgram(
        * Program node itself. We need to figure out whether an eslint suppression range
        * applies to this function first.
        */
-      const eslintSuppressionsInFunction =
-        filterEslintSuppressionsThatAffectFunction(eslintSuppressions, fn);
-      if (eslintSuppressionsInFunction.length > 0) {
+      const suppressionsInFunction =
+        filterSuppressionsThatAffectFunction(suppressions, fn);
+      if (suppressionsInFunction.length > 0) {
         handleError(lintError, pass, fn.node.loc ?? null);
       }
     }

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

@@ -16,26 +16,31 @@ import {
 
 /**
  * Captures the start and end range of a pair of eslint-disable ... eslint-enable comments. In the
- * case of a CommentLine, both the disable and enable point to the same comment.
+ * case of a CommentLine or a relevant Flow suppression, both the disable and enable point to the
+ * same comment.
  *
  * The enable comment can be missing in the case where only a disable block is present, ie the rest
  * of the file has potential React violations.
  */
-export type EslintSuppressionRange = {
+export type SuppressionRange = {
   disableComment: t.Comment;
   enableComment: t.Comment | null;
+  source: SuppressionSource;
 };
 
+type SuppressionSource =
+  'Eslint' | 'Flow'
+
 /**
- * An eslint suppression affects a function if:
+ * An suppression affects a function if:
  *   1. The suppression is within the function's body; or
  *   2. The suppression wraps the function
  */
-export function filterEslintSuppressionsThatAffectFunction(
-  suppressionRanges: Array<EslintSuppressionRange>,
+export function filterSuppressionsThatAffectFunction(
+  suppressionRanges: Array<SuppressionRange>,
   fn: NodePath<t.Function>
-): Array<EslintSuppressionRange> {
-  const suppressionsInScope: Array<EslintSuppressionRange> = [];
+): Array<SuppressionRange> {
+  const suppressionsInScope: Array<SuppressionRange> = [];
   const fnNode = fn.node;
   for (const suppressionRange of suppressionRanges) {
     if (
@@ -70,13 +75,15 @@ export function filterEslintSuppressionsThatAffectFunction(
   return suppressionsInScope;
 }
 
-export function findProgramEslintSuppressions(
+export function findProgramSuppressions(
   programComments: Array<t.Comment>,
-  ruleNames: Array<string>
-): Array<EslintSuppressionRange> {
-  const suppressionRanges: Array<EslintSuppressionRange> = [];
+  ruleNames: Array<string>,
+  flowSuppressions: boolean,
+): Array<SuppressionRange> {
+  const suppressionRanges: Array<SuppressionRange> = [];
   let disableComment: t.Comment | null = null;
   let enableComment: t.Comment | null = null;
+  let source: SuppressionSource | null = null;
 
   const rulePattern = `(${ruleNames.join("|")})`;
   const disableNextLinePattern = new RegExp(
@@ -84,6 +91,9 @@ export function findProgramEslintSuppressions(
   );
   const disablePattern = new RegExp(`eslint-disable ${rulePattern}`);
   const enablePattern = new RegExp(`eslint-enable ${rulePattern}`);
+  const flowSuppressionPattern = new RegExp(
+    '\\$(FlowFixMe\\w*|FlowExpectedError|FlowIssue)\\[react\\-rule'
+  );
 
   for (const comment of programComments) {
     if (comment.start == null || comment.end == null) {
@@ -100,36 +110,48 @@ export function findProgramEslintSuppressions(
     ) {
       disableComment = comment;
       enableComment = comment;
+      source = 'Eslint';
+    }
+
+    if (
+      flowSuppressions &&
+      disableComment == null &&
+      flowSuppressionPattern.test(comment.value)
+    ) {
+      disableComment = comment;
+      enableComment = comment;
+      source = 'Flow';
     }
 
     if (disablePattern.test(comment.value)) {
       disableComment = comment;
+      source = 'Eslint';
     }
 
-    if (enablePattern.test(comment.value)) {
+    if (enablePattern.test(comment.value) && source === 'Eslint') {
       enableComment = comment;
     }
 
-    if (disableComment != null) {
+    if (disableComment != null && source != null) {
       suppressionRanges.push({
         disableComment: disableComment,
         enableComment: enableComment,
+        source,
       });
       disableComment = null;
       enableComment = null;
+      source = null;
     }
   }
   return suppressionRanges;
 }
 
 export function suppressionsToCompilerError(
-  suppressionRanges: Array<EslintSuppressionRange>
+  suppressionRanges: Array<SuppressionRange>
 ): CompilerError | null {
   if (suppressionRanges.length === 0) {
     return null;
   }
-  const reason =
-    "React Forget has bailed out of optimizing this component as one or more React eslint rules were disabled. React Forget only works when your components follow all the rules of React, disabling them may result in undefined behavior";
   const error = new CompilerError();
   for (const suppressionRange of suppressionRanges) {
     if (
@@ -138,15 +160,25 @@ export function suppressionsToCompilerError(
     ) {
       continue;
     }
+    let reason, suggestion;
+    switch (suppressionRange.source) {
+      case 'Eslint':
+        reason = "React Forget has bailed out of optimizing this component as one or more React eslint rules were disabled";
+        suggestion = "Remove the eslint disable";
+        break;
+      case 'Flow':
+        reason = "React Forget has bailed out of optimizing this component as one or more React rule violations were reported by Flow";
+        suggestion = "Remove the Flow suppression and address the React error";
+    }
     error.pushErrorDetail(
       new CompilerErrorDetail({
-        reason,
+        reason: `${reason}. React Forget only works when your components follow all the rules of React, disabling them may result in undefined behavior`,
         description: suppressionRange.disableComment.value.trim(),
         severity: ErrorSeverity.InvalidReact,
         loc: suppressionRange.disableComment.loc ?? null,
         suggestions: [
           {
-            description: "Remove the eslint disable",
+            description: suggestion,
             range: [
               suppressionRange.disableComment.start,
               suppressionRange.disableComment.end,

compiler/packages/babel-plugin-react-forget/src/Entrypoint/index.ts

@@ -5,9 +5,9 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-export * from "./EslintSuppression";
 export * from "./Gating";
 export * from "./Imports";
 export * from "./Options";
 export * from "./Pipeline";
 export * from "./Program";
+export * from "./Suppression";

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-flow-suppression.expect.md

@@ -0,0 +1,22 @@
+
+## Input
+
+```javascript
+// @enableFlowSuppressions
+
+function Foo(props) {
+    // $FlowFixMe[react-rule-hook]
+    useX();
+    return null;
+}
+
+```
+
+
+## Error
+
+```
+[ReactForget] InvalidReact: React Forget has bailed out of optimizing this component as one or more React rule violations were reported by Flow. React Forget only works when your components follow all the rules of React, disabling them may result in undefined behavior. $FlowFixMe[react-rule-hook] (4:4)
+```
+          
+      

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-flow-suppression.js

@@ -0,0 +1,7 @@
+// @enableFlowSuppressions
+
+function Foo(props) {
+    // $FlowFixMe[react-rule-hook]
+    useX();
+    return null;
+}

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/no-flow-bailout-unrelated.expect.md

@@ -0,0 +1,39 @@
+
+## Input
+
+```javascript
+// @enableFlowSuppressions
+
+function Foo(props) {
+    // $FlowFixMe[incompatible-type]
+    useX();
+    const x = new Foo(...props.foo, null, ...[props.bar]);
+    return x;
+}
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react"; // @enableFlowSuppressions
+
+function Foo(props) {
+  const $ = useMemoCache(3);
+
+  useX();
+  let t0;
+  if ($[0] !== props.bar || $[1] !== props.foo) {
+    t0 = new Foo(...props.foo, null, ...[props.bar]);
+    $[0] = props.bar;
+    $[1] = props.foo;
+    $[2] = t0;
+  } else {
+    t0 = $[2];
+  }
+  const x = t0;
+  return x;
+}
+
+```
+      

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/no-flow-bailout-unrelated.js

@@ -0,0 +1,8 @@
+// @enableFlowSuppressions
+
+function Foo(props) {
+    // $FlowFixMe[incompatible-type]
+    useX();
+    const x = new Foo(...props.foo, null, ...[props.bar]);
+    return x;
+}

compiler/packages/fixture-test-utils/src/compiler-utils.ts

@@ -93,6 +93,12 @@ export function transformFixtureInput(
     eslintSuppressionRules = eslintSuppressionMatch[1].split("|");
   }
 
+  let flowSuppressions: boolean = false;
+  if (firstLine.includes("@enableFlowSuppressions")) {
+    flowSuppressions = true;
+  }
+
+
   const hookPatternMatch = /@hookPattern:"([^"]+)"/.exec(firstLine);
   if (
     hookPatternMatch &&
@@ -156,6 +162,7 @@ export function transformFixtureInput(
       noEmit: false,
       enableUseMemoCachePolyfill,
       eslintSuppressionRules,
+      flowSuppressions,
     },
     includeAst
   );
@@ -165,9 +172,9 @@ export function transformFixtureInput(
     code:
       result.code != null
         ? prettier.format(result.code, {
-            semi: true,
-            parser: language === "typescript" ? "babel-ts" : "flow",
-          })
+          semi: true,
+          parser: language === "typescript" ? "babel-ts" : "flow",
+        })
         : result.code,
   };
 }