From 34451e85b1db9037a57fe8c0cfdd235af9270735 Mon Sep 17 00:00:00 2001 From: Antoine Doubovetzky Date: Tue, 8 Nov 2022 11:55:23 -0800 Subject: [PATCH] Replace ternary in assertGenericTypeAnnotationHasExactlyOneTypeParameter with typeParameterInstantiation attribute in parser (#35157) Summary: Part of https://github.com/facebook/react-native/issues/34872: > Create a new function typeParameterInstantiation in the [parsers.js file](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parser.js) and add documentation to it. Implement it properly in the [FlowParser.js](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/flow/parser.js#L15) and in the [TypeScriptParser.js](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/typescript/parser.js#L15). Update the signature of [assertGenericTypeAnnotationHasExactlyOneTypeParameter](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parsers-commons.js#L67) function to take the Parser instead of the language and use the new function in place of the [ternary operator ?:](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parsers-commons.js#L83). There are 3 things I'm not sure about: 1. The issue suggests to create a new function. For this case I believe an attribute is simpler. Is there a reason to prefer a function ? 2. To update the tests I had to create a mocked parser. I created a new file `parserMock` (I took example on [AnimatedMock](https://github.com/facebook/react-native/blob/main/Libraries/Animated/AnimatedMock.js)). Does it seem ok ? 3. I'm not sure what to add in the documentation of `typeParameterInstantiation` ## Changelog [Internal] [Changed] - Replace ternary in assertGenericTypeAnnotationHasExactlyOneTypeParameter with typeParameterInstantiation attribute in parser Pull Request resolved: https://github.com/facebook/react-native/pull/35157 Test Plan: I tested using Jest and Flow commands. Reviewed By: rshest Differential Revision: D40889856 Pulled By: cipolleschi fbshipit-source-id: 8d9a8e087852f98dcc3fc0ecf1d4a7153f482ce7 --- .../parsers/__tests__/parsers-commons-test.js | 80 +++---------------- .../__tests__/parsers-primitives-test.js | 11 ++- .../src/parsers/flow/modules/index.js | 11 +-- .../src/parsers/flow/parser.js | 2 + .../src/parsers/parser.js | 5 ++ .../src/parsers/parserMock.js | 36 +++++++++ .../src/parsers/parsers-commons.js | 11 +-- .../src/parsers/parsers-primitives.js | 5 +- .../src/parsers/typescript/modules/index.js | 9 +-- .../src/parsers/typescript/parser.js | 2 + 10 files changed, 74 insertions(+), 98 deletions(-) create mode 100644 packages/react-native-codegen/src/parsers/parserMock.js diff --git a/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js b/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js index 232a75c4fdc13e..a8da4a6fd07262 100644 --- a/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js +++ b/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js @@ -20,6 +20,9 @@ const { } = require('../parsers-commons.js'); const {UnsupportedUnionTypeAnnotationParserError} = require('../errors'); import type {UnionTypeAnnotationMemberType} from '../../CodegenSchema'; +import {MockedParser} from '../parserMock'; + +const parser = new MockedParser(); describe('wrapNullable', () => { describe('when nullable is true', () => { @@ -101,7 +104,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => { assertGenericTypeAnnotationHasExactlyOneTypeParameter( moduleName, typeAnnotation, - 'Flow', + parser, ), ).not.toThrow(); }); @@ -117,14 +120,14 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => { assertGenericTypeAnnotationHasExactlyOneTypeParameter( moduleName, typeAnnotation, - 'Flow', + parser, ), ).toThrowErrorMatchingInlineSnapshot( `"Module testModuleName: Generic 'typeAnnotationName' must have type parameters."`, ); }); - it('throws an error if typeAnnotation.typeParameters.type is not TypeParameterInstantiation when language is Flow', () => { + it('throws an error if typeAnnotation.typeParameters.type is not equal to parser.typeParameterInstantiation', () => { const flowTypeAnnotation = { typeParameters: { type: 'wrongType', @@ -138,36 +141,14 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => { assertGenericTypeAnnotationHasExactlyOneTypeParameter( moduleName, flowTypeAnnotation, - 'Flow', + parser, ), ).toThrowErrorMatchingInlineSnapshot( `"assertGenericTypeAnnotationHasExactlyOneTypeParameter: Type parameters must be an AST node of type 'TypeParameterInstantiation'"`, ); }); - it('throws an error if typeAnnotation.typeParameters.type is not TSTypeParameterInstantiation when language is TypeScript', () => { - const typeScriptTypeAnnotation = { - typeParameters: { - type: 'wrongType', - params: [1], - }, - typeName: { - name: 'typeAnnotationName', - }, - }; - expect(() => - assertGenericTypeAnnotationHasExactlyOneTypeParameter( - moduleName, - typeScriptTypeAnnotation, - 'TypeScript', - ), - ).toThrowErrorMatchingInlineSnapshot( - `"assertGenericTypeAnnotationHasExactlyOneTypeParameter: Type parameters must be an AST node of type 'TSTypeParameterInstantiation'"`, - ); - }); - - it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter for Flow", () => { - const language: ParserType = 'Flow'; + it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter", () => { const typeAnnotationWithTwoParams = { typeParameters: { params: [1, 2], @@ -181,7 +162,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => { assertGenericTypeAnnotationHasExactlyOneTypeParameter( moduleName, typeAnnotationWithTwoParams, - language, + parser, ), ).toThrowErrorMatchingInlineSnapshot( `"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`, @@ -200,48 +181,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => { assertGenericTypeAnnotationHasExactlyOneTypeParameter( moduleName, typeAnnotationWithNoParams, - language, - ), - ).toThrowErrorMatchingInlineSnapshot( - `"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`, - ); - }); - - it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter for TS", () => { - const language: ParserType = 'TypeScript'; - const typeAnnotationWithTwoParams = { - typeParameters: { - params: [1, 2], - type: 'TSTypeParameterInstantiation', - }, - typeName: { - name: 'typeAnnotationName', - }, - }; - expect(() => - assertGenericTypeAnnotationHasExactlyOneTypeParameter( - moduleName, - typeAnnotationWithTwoParams, - language, - ), - ).toThrowErrorMatchingInlineSnapshot( - `"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`, - ); - - const typeAnnotationWithNoParams = { - typeParameters: { - params: [], - type: 'TSTypeParameterInstantiation', - }, - typeName: { - name: 'typeAnnotationName', - }, - }; - expect(() => - assertGenericTypeAnnotationHasExactlyOneTypeParameter( - moduleName, - typeAnnotationWithNoParams, - language, + parser, ), ).toThrowErrorMatchingInlineSnapshot( `"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`, diff --git a/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js b/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js index ecd36400bc6448..082af17c1df361 100644 --- a/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js +++ b/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js @@ -26,6 +26,9 @@ const { emitMixedTypeAnnotation, typeAliasResolution, } = require('../parsers-primitives.js'); +import {MockedParser} from '../parserMock'; + +const parser = new MockedParser(); describe('emitBoolean', () => { describe('when nullable is true', () => { @@ -334,7 +337,7 @@ describe('typeAliasResolution', () => { describe('emitPromise', () => { const moduleName = 'testModuleName'; - const language = 'Flow'; + describe("when typeAnnotation doesn't have exactly one typeParameter", () => { const typeAnnotation = { typeParameters: { @@ -348,7 +351,7 @@ describe('emitPromise', () => { it('throws an IncorrectlyParameterizedGenericParserError error', () => { const nullable = false; expect(() => - emitPromise(moduleName, typeAnnotation, language, nullable), + emitPromise(moduleName, typeAnnotation, parser, nullable), ).toThrow(); }); }); @@ -370,7 +373,7 @@ describe('emitPromise', () => { const result = emitPromise( moduleName, typeAnnotation, - language, + parser, nullable, ); const expected = { @@ -389,7 +392,7 @@ describe('emitPromise', () => { const result = emitPromise( moduleName, typeAnnotation, - language, + parser, nullable, ); const expected = { diff --git a/packages/react-native-codegen/src/parsers/flow/modules/index.js b/packages/react-native-codegen/src/parsers/flow/modules/index.js index 5c2538adf7c341..9ff2dcb5a0a6a0 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/index.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/index.js @@ -184,19 +184,14 @@ function translateTypeAnnotation( return emitRootTag(nullable); } case 'Promise': { - return emitPromise( - hasteModuleName, - typeAnnotation, - language, - nullable, - ); + return emitPromise(hasteModuleName, typeAnnotation, parser, nullable); } case 'Array': case '$ReadOnlyArray': { assertGenericTypeAnnotationHasExactlyOneTypeParameter( hasteModuleName, typeAnnotation, - language, + parser, ); return translateArrayTypeAnnotation( @@ -213,7 +208,7 @@ function translateTypeAnnotation( assertGenericTypeAnnotationHasExactlyOneTypeParameter( hasteModuleName, typeAnnotation, - language, + parser, ); const [paramType, isParamNullable] = unwrapNullable( diff --git a/packages/react-native-codegen/src/parsers/flow/parser.js b/packages/react-native-codegen/src/parsers/flow/parser.js index 5315411ecf49a5..6dbe099a289016 100644 --- a/packages/react-native-codegen/src/parsers/flow/parser.js +++ b/packages/react-native-codegen/src/parsers/flow/parser.js @@ -14,6 +14,8 @@ import type {ParserType} from '../errors'; import type {Parser} from '../parser'; class FlowParser implements Parser { + typeParameterInstantiation: string = 'TypeParameterInstantiation'; + getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string { return maybeEnumDeclaration.body.type .replace('EnumNumberBody', 'NumberTypeAnnotation') diff --git a/packages/react-native-codegen/src/parsers/parser.js b/packages/react-native-codegen/src/parsers/parser.js index eec4faea60b700..92562a9538c29a 100644 --- a/packages/react-native-codegen/src/parsers/parser.js +++ b/packages/react-native-codegen/src/parsers/parser.js @@ -17,6 +17,11 @@ import type {ParserType} from './errors'; * It exposes all the methods that contain language-specific logic. */ export interface Parser { + /** + * This is the TypeParameterInstantiation value + */ + typeParameterInstantiation: string; + /** * Given a type declaration, it possibly returns the name of the Enum type. * @parameter maybeEnumDeclaration: an object possibly containing an Enum declaration. diff --git a/packages/react-native-codegen/src/parsers/parserMock.js b/packages/react-native-codegen/src/parsers/parserMock.js new file mode 100644 index 00000000000000..beee67d115cf77 --- /dev/null +++ b/packages/react-native-codegen/src/parsers/parserMock.js @@ -0,0 +1,36 @@ +/** + * 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'; + +import type {Parser} from './parser'; +import type {ParserType} from './errors'; + +export class MockedParser implements Parser { + typeParameterInstantiation: string = 'TypeParameterInstantiation'; + + getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string { + return maybeEnumDeclaration.body.type + .replace('EnumNumberBody', 'NumberTypeAnnotation') + .replace('EnumStringBody', 'StringTypeAnnotation'); + } + + isEnumDeclaration(maybeEnumDeclaration: $FlowFixMe): boolean { + return maybeEnumDeclaration.type === 'EnumDeclaration'; + } + + language(): ParserType { + return 'Flow'; + } + + nameForGenericTypeAnnotation(typeAnnotation: $FlowFixMe): string { + return typeAnnotation.id.name; + } +} diff --git a/packages/react-native-codegen/src/parsers/parsers-commons.js b/packages/react-native-codegen/src/parsers/parsers-commons.js index cdeb0c7bf5c147..0d1af17fc526ec 100644 --- a/packages/react-native-codegen/src/parsers/parsers-commons.js +++ b/packages/react-native-codegen/src/parsers/parsers-commons.js @@ -77,20 +77,17 @@ function assertGenericTypeAnnotationHasExactlyOneTypeParameter( * TODO(T108222691): Use flow-types for @babel/parser */ typeAnnotation: $FlowFixMe, - language: ParserType, + parser: Parser, ) { if (typeAnnotation.typeParameters == null) { throw new MissingTypeParameterGenericParserError( moduleName, typeAnnotation, - language, + parser.language(), ); } - const typeAnnotationType = - language === 'TypeScript' - ? 'TSTypeParameterInstantiation' - : 'TypeParameterInstantiation'; + const typeAnnotationType = parser.typeParameterInstantiation; invariant( typeAnnotation.typeParameters.type === typeAnnotationType, @@ -101,7 +98,7 @@ function assertGenericTypeAnnotationHasExactlyOneTypeParameter( throw new MoreThanOneTypeParameterGenericParserError( moduleName, typeAnnotation, - language, + parser.language(), ); } } diff --git a/packages/react-native-codegen/src/parsers/parsers-primitives.js b/packages/react-native-codegen/src/parsers/parsers-primitives.js index 0d845ddbb7cea8..2fe7f984cd0d32 100644 --- a/packages/react-native-codegen/src/parsers/parsers-primitives.js +++ b/packages/react-native-codegen/src/parsers/parsers-primitives.js @@ -32,6 +32,7 @@ import type { NamedShape, } from '../CodegenSchema'; import type {ParserType} from './errors'; +import type {Parser} from './parser'; import type { ParserErrorCapturer, TypeAliasResolutionStatus, @@ -174,13 +175,13 @@ function typeAliasResolution( function emitPromise( hasteModuleName: string, typeAnnotation: $FlowFixMe, - language: ParserType, + parser: Parser, nullable: boolean, ): Nullable { assertGenericTypeAnnotationHasExactlyOneTypeParameter( hasteModuleName, typeAnnotation, - language, + parser, ); return wrapNullable(nullable, { diff --git a/packages/react-native-codegen/src/parsers/typescript/modules/index.js b/packages/react-native-codegen/src/parsers/typescript/modules/index.js index e58366297b16f9..37b9b70a2ba506 100644 --- a/packages/react-native-codegen/src/parsers/typescript/modules/index.js +++ b/packages/react-native-codegen/src/parsers/typescript/modules/index.js @@ -213,19 +213,14 @@ function translateTypeAnnotation( return emitRootTag(nullable); } case 'Promise': { - return emitPromise( - hasteModuleName, - typeAnnotation, - language, - nullable, - ); + return emitPromise(hasteModuleName, typeAnnotation, parser, nullable); } case 'Array': case 'ReadonlyArray': { assertGenericTypeAnnotationHasExactlyOneTypeParameter( hasteModuleName, typeAnnotation, - language, + parser, ); return translateArrayTypeAnnotation( diff --git a/packages/react-native-codegen/src/parsers/typescript/parser.js b/packages/react-native-codegen/src/parsers/typescript/parser.js index 3bcf2cdb6415e6..aa561c63f624d7 100644 --- a/packages/react-native-codegen/src/parsers/typescript/parser.js +++ b/packages/react-native-codegen/src/parsers/typescript/parser.js @@ -14,6 +14,8 @@ import type {ParserType} from '../errors'; import type {Parser} from '../parser'; class TypeScriptParser implements Parser { + typeParameterInstantiation: string = 'TSTypeParameterInstantiation'; + getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string { if (maybeEnumDeclaration.members[0].initializer) { return maybeEnumDeclaration.members[0].initializer.type