Throw Flow syntax errors instead of continuing to process the AST (#3…

…9035)

Summary:
Pull Request resolved: #39035

Changelog: [General][Fixed] Flow syntax errors in Codegen specs are no longer ignored.

Instead of throwing errors like most parsers, the `flow-parser` package returns errors as part of the AST (along with a best-effort parse). It turns out that `react-native/codegen` ignores such errors and only detects a subset of them after the fact. Here we change the behaviour to immediately throwing a descriptive error message (containing the file name and a code frame).

**This change is theoretically breaking** for any published packages that already contain broken Flow code (that somehow doesn't happen to affect the Codegen output today). Hopefully, anyone using Flow-flavoured RN Codegen is also typechecking with Flow and/or building with Metro (which would both flag the same errors), so the impact should be fairly contained.

Reviewed By: huntie

Differential Revision: D48385786

fbshipit-source-id: c7e1f5fb64a61fb0eb9e9f8f7501b43264c9626c

packages/eslint-plugin-specs/react-native-modules.js

@@ -154,7 +154,7 @@ function rule(context) {
       const [parsingErrors, tryParse] = createParserErrorCapturer();
 
       const sourceCode = context.getSourceCode().getText();
-      const ast = parser.getAst(sourceCode);
+      const ast = parser.getAst(sourceCode, filename);
 
       tryParse(() => {
         buildModuleSchema(

packages/react-native-codegen/package.json

@@ -30,6 +30,7 @@
   ],
   "dependencies": {
     "@babel/parser": "^7.20.0",
+    "@babel/code-frame": "^7.0.0",
     "flow-parser": "^0.206.0",
     "jscodeshift": "^0.14.0",
     "nullthrows": "^1.1.1"

packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap

@@ -2,7 +2,18 @@
 
 exports[`RN Codegen Flow Parser Fails with error message EMPTY_ENUM_NATIVE_MODULE 1`] = `"Module NativeSampleTurboModule: Failed parsing the enum SomeEnum in NativeSampleTurboModule with the error: Enums should have at least one member and member values can not be mixed- they all must be either blank, number, or string values."`;
 
-exports[`RN Codegen Flow Parser Fails with error message MIXED_VALUES_ENUM_NATIVE_MODULE 1`] = `"Module NativeSampleTurboModule: Failed parsing the enum SomeEnum in NativeSampleTurboModule with the error: Enums should have at least one member and member values can not be mixed- they all must be either blank, number, or string values."`;
+exports[`RN Codegen Flow Parser Fails with error message MIXED_VALUES_ENUM_NATIVE_MODULE 1`] = `
+"Syntax error in path/NativeSampleTurboModule.js: Enum \`SomeEnum\` has inconsistent member initializers. Either use no initializers, or consistently use literals (either booleans, numbers, or strings) for all member initializers.
+  15 | import * as TurboModuleRegistry from '../TurboModuleRegistry';
+  16 |
+> 17 | export enum SomeEnum {
+     |             ^^^^^^^^
+  18 |   NUM = 1,
+  19 |   STR = 'str',
+  20 | }
+
+and 1 other error in the same file."
+`;
 
 exports[`RN Codegen Flow Parser Fails with error message NATIVE_MODULES_WITH_ARRAY_WITH_NO_TYPE_FOR_CONTENT 1`] = `"Module NativeSampleTurboModule: Generic 'Array' must have type parameters."`;
 
@@ -18,7 +29,17 @@ exports[`RN Codegen Flow Parser Fails with error message NATIVE_MODULES_WITH_UNN
 
 exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_EXTENDING_TURBO_MODULE 1`] = `"Module NativeSampleTurboModule: Every NativeModule spec file must declare exactly one NativeModule Flow interface. This file declares 2: 'Spec', and 'Spec2'. Please remove the extraneous Flow interface declarations."`;
 
-exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_MODULES_EXPORTED_WITH_DEFAULT 1`] = `"Module NativeSampleTurboModule: No Flow interfaces extending TurboModule were detected in this NativeModule spec."`;
+exports[`RN Codegen Flow Parser Fails with error message TWO_NATIVE_MODULES_EXPORTED_WITH_DEFAULT 1`] = `
+"Syntax error in path/NativeSampleTurboModule.js: Duplicate export for \`default\`
+  17 |
+  18 | export default TurboModuleRegistry.getEnforcing<Spec1>('SampleTurboModule1');
+> 19 | export default TurboModuleRegistry.getEnforcing<Spec2>('SampleTurboModule2');
+     |        ^^^^^^^
+  20 |
+  21 |
+
+and 1 other error in the same file."
+`;
 
 exports[`RN Codegen Flow Parser can generate fixture ANDROID_ONLY_NATIVE_MODULE 1`] = `
 "{

packages/react-native-codegen/src/parsers/flow/modules/__tests__/module-parser-e2e-test.js

@@ -84,7 +84,7 @@ describe('Flow Module Parser', () => {
           import type {TurboModule} from 'RCTExport';
           import * as TurboModuleRegistry from 'TurboModuleRegistry';
           export interface Spec extends TurboModule {
-            +useArg(arg: any): void;
+            useArg(arg: any): void;
           }
           export default TurboModuleRegistry.get<Spec>('Foo');
         `);
@@ -98,7 +98,7 @@ describe('Flow Module Parser', () => {
           import type {TurboModule} from 'RCTExport';
           import * as TurboModuleRegistry from 'TurboModuleRegistry';
           export interface Spec extends TurboModule {
-            +useArg(boolean): void;
+            useArg(boolean): void;
           }
           export default TurboModuleRegistry.get<Spec>('Foo');
         `);
@@ -145,7 +145,7 @@ describe('Flow Module Parser', () => {
           ${TYPE_ALIAS_DECLARATIONS}
 
           export interface Spec extends TurboModule {
-            +useArg(${annotateArg(paramName, paramType)}): void;
+            useArg(${annotateArg(paramName, paramType)}): void;
           }
           export default TurboModuleRegistry.get<Spec>('Foo');
         `);
@@ -356,7 +356,7 @@ describe('Flow Module Parser', () => {
               type AnimalPointer = Animal;
 
               export interface Spec extends TurboModule {
-                +useArg(${annotateArg('arg', 'AnimalPointer')}): void;
+                useArg(${annotateArg('arg', 'AnimalPointer')}): void;
               }
               export default TurboModuleRegistry.get<Spec>('Foo');
             `);
@@ -693,7 +693,7 @@ describe('Flow Module Parser', () => {
         import type {TurboModule} from 'RCTExport';
         import * as TurboModuleRegistry from 'TurboModuleRegistry';
         export interface Spec extends TurboModule {
-          +useArg(): void;
+          useArg(): void;
         }
         export default TurboModuleRegistry.get<Spec>('Foo');
       `);
@@ -727,7 +727,7 @@ describe('Flow Module Parser', () => {
           ${TYPE_ALIAS_DECLARATIONS}
 
           export interface Spec extends TurboModule {
-            +useArg(): ${annotateRet(flowType)};
+            useArg(): ${annotateRet(flowType)};
           }
           export default TurboModuleRegistry.get<Spec>('Foo');
         `);

packages/react-native-codegen/src/parsers/flow/parseFlowAndThrowErrors.js

@@ -0,0 +1,82 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict
+ * @format
+ */
+
+'use strict';
+
+// $FlowFixMe[untyped-import]
+const {codeFrameColumns} = require('@babel/code-frame');
+// $FlowFixMe[untyped-import] there's no flowtype flow-parser
+const flowParser = require('flow-parser');
+
+class FlowParserSyntaxError extends Error {
+  constructor(
+    code: string,
+    filename: ?string,
+    errors: $ReadOnlyArray<{
+      loc: $ReadOnly<{
+        start: $ReadOnly<{line: number, column: number}>,
+        end: $ReadOnly<{line: number, column: number}>,
+      }>,
+      message: string,
+    }>,
+  ) {
+    const firstError = errors[0];
+    const codeFrame = codeFrameColumns(
+      code,
+      {
+        start: {
+          line: firstError.loc.start.line,
+          // flow-parser returns 0-indexed columns but Babel expects 1-indexed
+          column: firstError.loc.start.column + 1,
+        },
+        end: {
+          line: firstError.loc.end.line,
+          // flow-parser returns 0-indexed columns but Babel expects 1-indexed
+          column: firstError.loc.end.column + 1,
+        },
+      },
+      {
+        forceColor: false,
+      },
+    );
+    const additionalErrorsMessage = errors.length
+      ? '\n\nand ' +
+        errors.length +
+        ' other error' +
+        (errors.length > 1 ? 's' : '') +
+        ' in the same file.'
+      : '';
+
+    super(
+      (filename != null ? `Syntax error in ${filename}: ` : 'Syntax error: ') +
+        firstError.message +
+        '\n' +
+        codeFrame +
+        additionalErrorsMessage,
+    );
+  }
+}
+
+function parseFlowAndThrowErrors(
+  code: string,
+  options: $ReadOnly<{filename?: ?string}> = {},
+): $FlowFixMe {
+  const ast = flowParser.parse(code, {
+    enums: true,
+  });
+  if (ast.errors && ast.errors.length) {
+    throw new FlowParserSyntaxError(code, options.filename, ast.errors);
+  }
+  return ast;
+}
+
+module.exports = {
+  parseFlowAndThrowErrors,
+};

packages/react-native-codegen/src/parsers/flow/parser.js

@@ -55,8 +55,7 @@ const {
   getTypeAnnotation,
 } = require('./components/componentsUtils');
 const {flowTranslateTypeAnnotation} = require('./modules');
-// $FlowFixMe[untyped-import] there's no flowtype flow-parser
-const flowParser = require('flow-parser');
+const {parseFlowAndThrowErrors} = require('./parseFlowAndThrowErrors');
 const fs = require('fs');
 const invariant = require('invariant');
 
@@ -143,10 +142,8 @@ class FlowParser implements Parser {
     return this.parseString(contents, 'path/NativeSampleTurboModule.js');
   }
 
-  getAst(contents: string): $FlowFixMe {
-    return flowParser.parse(contents, {
-      enums: true,
-    });
+  getAst(contents: string, filename?: ?string): $FlowFixMe {
+    return parseFlowAndThrowErrors(contents, {filename});
   }
 
   getFunctionTypeAnnotationParameters(

packages/react-native-codegen/src/parsers/parser.js

@@ -169,9 +169,11 @@ export interface Parser {
   /**
    * Given the content of a file, it returns an AST.
    * @parameter contents: the content of the file.
+   * @parameter filename: the name of the file, if available.
+   * @throws if there is a syntax error.
    * @returns: the AST of the file.
    */
-  getAst(contents: string): $FlowFixMe;
+  getAst(contents: string, filename?: ?string): $FlowFixMe;
 
   /**
    * Given a FunctionTypeAnnotation, it returns an array of its parameters.

packages/react-native-codegen/src/parsers/parserMock.js

@@ -42,10 +42,9 @@ import invariant from 'invariant';
 const {
   UnsupportedObjectPropertyTypeAnnotationParserError,
 } = require('./errors');
+const {parseFlowAndThrowErrors} = require('./flow/parseFlowAndThrowErrors');
 const {buildPropSchema} = require('./parsers-commons');
 const {flattenProperties} = require('./typescript/components/componentsUtils');
-// $FlowFixMe[untyped-import] there's no flowtype flow-parser
-const flowParser = require('flow-parser');
 
 type ExtendsForProp = null | {
   type: 'ReactNativeBuiltInType',
@@ -122,10 +121,8 @@ export class MockedParser implements Parser {
     return schemaMock;
   }
 
-  getAst(contents: string): $FlowFixMe {
-    return flowParser.parse(contents, {
-      enums: true,
-    });
+  getAst(contents: string, filename?: ?string): $FlowFixMe {
+    return parseFlowAndThrowErrors(contents, {filename});
   }
 
   getFunctionTypeAnnotationParameters(

packages/react-native-codegen/src/parsers/parsers-commons.js

@@ -480,7 +480,7 @@ function buildSchema(
     return {modules: {}};
   }
 
-  const ast = parser.getAst(contents);
+  const ast = parser.getAst(contents, filename);
   const configType = getConfigType(ast, Visitor);
 
   return buildSchemaFromConfigType(

packages/react-native-codegen/src/parsers/typescript/parser.js

@@ -142,7 +142,7 @@ class TypeScriptParser implements Parser {
     return this.parseString(contents, 'path/NativeSampleTurboModule.ts');
   }
 
-  getAst(contents: string): $FlowFixMe {
+  getAst(contents: string, filename?: ?string): $FlowFixMe {
     return babelParser.parse(contents, {
       sourceType: 'module',
       plugins: ['typescript'],