Skip to content

Commit 56d7a87

Browse files
Antoine Doubovetzkyfacebook-github-bot
Antoine Doubovetzky
authored and
facebook-github-bot
committed
Fix assertGenericTypeAnnotationHasExactlyOneTypeParameter throwing wrong error (facebook#35134)
Summary: 1. I noticed there was a mistake in the [IncorrectlyParameterizedGenericParserError](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/errors.js#L159): ``` if ( genericTypeAnnotation.typeParameters.type === 'TypeParameterInstantiation' && genericTypeAnnotation.typeParameters.params.length !== 1 ) { ``` Here we should replace ` 'TypeParameterInstantiation'` with ` 'TSTypeParameterInstantiation'` when the language is `TypeScript`. The result is that we get a ["Couldn't create IncorrectlyParameterizedGenericParserError"](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/errors.js#L172) error instead of ["Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parsers-commons.js#L88). I added a [test case](facebook@2f16116) to cover this case: <img width="1008" alt="Capture d’écran 2022-10-30 à 13 55 56" src="https://user-images.githubusercontent.com/17070498/198879598-ab5a6092-8cbf-422a-9993-2f3f92c9d84c.png"> 2. Looking closely at where IncorrectlyParameterizedGenericParserError is used, I noticed that the logic was duplicated in [assertGenericTypeAnnotationHasExactlyOneTypeParameter](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parsers-commons.js#L65). I believe that the logic should reside in `assertGenericTypeAnnotationHasExactlyOneTypeParameter` so I split the `IncorrectlyParameterizedGenericParserError` in 2 different errors. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Fix assertGenericTypeAnnotationHasExactlyOneTypeParameter throwing wrong error Pull Request resolved: facebook#35134 Test Plan: I tested using Jest and Flow commands. Reviewed By: rshest Differential Revision: D40853200 Pulled By: cipolleschi fbshipit-source-id: 7040e57e0a2f511ba23fd4c54beae2ccff2fa89d
1 parent ea55e3b commit 56d7a87

File tree

5 files changed

+84
-46
lines changed

5 files changed

+84
-46
lines changed

‎packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
167167
);
168168
});
169169

170-
it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter", () => {
170+
it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter for Flow", () => {
171+
const language: ParserType = 'Flow';
171172
const typeAnnotationWithTwoParams = {
172173
typeParameters: {
173174
params: [1, 2],
@@ -181,7 +182,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
181182
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
182183
moduleName,
183184
typeAnnotationWithTwoParams,
184-
'Flow',
185+
language,
185186
),
186187
).toThrowErrorMatchingInlineSnapshot(
187188
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
@@ -200,7 +201,48 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
200201
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
201202
moduleName,
202203
typeAnnotationWithNoParams,
203-
'Flow',
204+
language,
205+
),
206+
).toThrowErrorMatchingInlineSnapshot(
207+
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
208+
);
209+
});
210+
211+
it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter for TS", () => {
212+
const language: ParserType = 'TypeScript';
213+
const typeAnnotationWithTwoParams = {
214+
typeParameters: {
215+
params: [1, 2],
216+
type: 'TSTypeParameterInstantiation',
217+
},
218+
typeName: {
219+
name: 'typeAnnotationName',
220+
},
221+
};
222+
expect(() =>
223+
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
224+
moduleName,
225+
typeAnnotationWithTwoParams,
226+
language,
227+
),
228+
).toThrowErrorMatchingInlineSnapshot(
229+
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
230+
);
231+
232+
const typeAnnotationWithNoParams = {
233+
typeParameters: {
234+
params: [],
235+
type: 'TSTypeParameterInstantiation',
236+
},
237+
typeName: {
238+
name: 'typeAnnotationName',
239+
},
240+
};
241+
expect(() =>
242+
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
243+
moduleName,
244+
typeAnnotationWithNoParams,
245+
language,
204246
),
205247
).toThrowErrorMatchingInlineSnapshot(
206248
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,

‎packages/react-native-codegen/src/parsers/errors.js

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
import type {UnionTypeAnnotationMemberType} from '../CodegenSchema';
1414

15-
const invariant = require('invariant');
1615
import type {Parser} from './parser';
1716
export type ParserType = 'Flow' | 'TypeScript';
1817

@@ -131,11 +130,7 @@ class UnsupportedGenericParserError extends ParserError {
131130
}
132131
}
133132

134-
class IncorrectlyParameterizedGenericParserError extends ParserError {
135-
+genericName: string;
136-
+numTypeParameters: number;
137-
138-
// $FlowFixMe[missing-local-annot]
133+
class MissingTypeParameterGenericParserError extends ParserError {
139134
constructor(
140135
nativeModuleName: string,
141136
genericTypeAnnotation: $FlowFixMe,
@@ -145,31 +140,30 @@ class IncorrectlyParameterizedGenericParserError extends ParserError {
145140
language === 'TypeScript'
146141
? genericTypeAnnotation.typeName.name
147142
: genericTypeAnnotation.id.name;
148-
if (genericTypeAnnotation.typeParameters == null) {
149-
super(
150-
nativeModuleName,
151-
genericTypeAnnotation,
152-
`Generic '${genericName}' must have type parameters.`,
153-
);
154-
return;
155-
}
156143

157-
if (
158-
genericTypeAnnotation.typeParameters.type ===
159-
'TypeParameterInstantiation' &&
160-
genericTypeAnnotation.typeParameters.params.length !== 1
161-
) {
162-
super(
163-
nativeModuleName,
164-
genericTypeAnnotation.typeParameters,
165-
`Generic '${genericName}' must have exactly one type parameter.`,
166-
);
167-
return;
168-
}
144+
super(
145+
nativeModuleName,
146+
genericTypeAnnotation,
147+
`Generic '${genericName}' must have type parameters.`,
148+
);
149+
}
150+
}
151+
152+
class MoreThanOneTypeParameterGenericParserError extends ParserError {
153+
constructor(
154+
nativeModuleName: string,
155+
genericTypeAnnotation: $FlowFixMe,
156+
language: ParserType,
157+
) {
158+
const genericName =
159+
language === 'TypeScript'
160+
? genericTypeAnnotation.typeName.name
161+
: genericTypeAnnotation.id.name;
169162

170-
invariant(
171-
false,
172-
"Couldn't create IncorrectlyParameterizedGenericParserError",
163+
super(
164+
nativeModuleName,
165+
genericTypeAnnotation,
166+
`Generic '${genericName}' must have exactly one type parameter.`,
173167
);
174168
}
175169
}
@@ -422,7 +416,8 @@ class IncorrectModuleRegistryCallArgumentTypeParserError extends ParserError {
422416

423417
module.exports = {
424418
ParserError,
425-
IncorrectlyParameterizedGenericParserError,
419+
MissingTypeParameterGenericParserError,
420+
MoreThanOneTypeParameterGenericParserError,
426421
MisnamedModuleInterfaceParserError,
427422
ModuleInterfaceNotFoundParserError,
428423
MoreThanOneModuleInterfaceParserError,

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const {
2121
UnsupportedGenericParserError,
2222
UnsupportedTypeAnnotationParserError,
2323
UnnamedFunctionParamParserError,
24-
IncorrectlyParameterizedGenericParserError,
24+
MissingTypeParameterGenericParserError,
2525
} = require('../../../errors');
2626
const invariant = require('invariant');
2727

@@ -212,7 +212,7 @@ describe('Flow Module Parser', () => {
212212
describe('Array Types', () => {
213213
it(`should not parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Array'`, () => {
214214
expect(() => parseParamType('arg', 'Array')).toThrow(
215-
IncorrectlyParameterizedGenericParserError,
215+
MissingTypeParameterGenericParserError,
216216
);
217217
});
218218

@@ -510,7 +510,7 @@ describe('Flow Module Parser', () => {
510510
it(`should not parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array`, () => {
511511
expect(() =>
512512
parseParamTypeObjectLiteralProp('prop', 'Array'),
513-
).toThrow(IncorrectlyParameterizedGenericParserError);
513+
).toThrow(MissingTypeParameterGenericParserError);
514514
});
515515

516516
function parseArrayElementType(
@@ -782,7 +782,7 @@ describe('Flow Module Parser', () => {
782782
describe('Array Types', () => {
783783
it(`should not parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Array'`, () => {
784784
expect(() => parseReturnType('Array')).toThrow(
785-
IncorrectlyParameterizedGenericParserError,
785+
MissingTypeParameterGenericParserError,
786786
);
787787
});
788788

@@ -1050,7 +1050,7 @@ describe('Flow Module Parser', () => {
10501050
it(`should not parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array`, () => {
10511051
expect(() =>
10521052
parseObjectLiteralReturnTypeProp('prop', 'Array'),
1053-
).toThrow(IncorrectlyParameterizedGenericParserError);
1053+
).toThrow(MissingTypeParameterGenericParserError);
10541054
});
10551055

10561056
function parseArrayElementType(

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import type {
2020
NativeModuleUnionTypeAnnotation,
2121
} from '../CodegenSchema.js';
2222
const {
23-
IncorrectlyParameterizedGenericParserError,
23+
MissingTypeParameterGenericParserError,
24+
MoreThanOneTypeParameterGenericParserError,
2425
UnsupportedUnionTypeAnnotationParserError,
2526
} = require('./errors');
2627
import type {ParserType} from './errors';
@@ -80,7 +81,7 @@ function assertGenericTypeAnnotationHasExactlyOneTypeParameter(
8081
language: ParserType,
8182
) {
8283
if (typeAnnotation.typeParameters == null) {
83-
throw new IncorrectlyParameterizedGenericParserError(
84+
throw new MissingTypeParameterGenericParserError(
8485
moduleName,
8586
typeAnnotation,
8687
language,
@@ -98,7 +99,7 @@ function assertGenericTypeAnnotationHasExactlyOneTypeParameter(
9899
);
99100

100101
if (typeAnnotation.typeParameters.params.length !== 1) {
101-
throw new IncorrectlyParameterizedGenericParserError(
102+
throw new MoreThanOneTypeParameterGenericParserError(
102103
moduleName,
103104
typeAnnotation,
104105
language,

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const {
2121
UnsupportedGenericParserError,
2222
UnsupportedTypeAnnotationParserError,
2323
UnnamedFunctionParamParserError,
24-
IncorrectlyParameterizedGenericParserError,
24+
MissingTypeParameterGenericParserError,
2525
} = require('../../../errors');
2626
const invariant = require('invariant');
2727

@@ -212,7 +212,7 @@ describe('TypeScript Module Parser', () => {
212212
describe('Array Types', () => {
213213
it(`should not parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Array'`, () => {
214214
expect(() => parseParamType('arg', 'Array')).toThrow(
215-
IncorrectlyParameterizedGenericParserError,
215+
MissingTypeParameterGenericParserError,
216216
);
217217
});
218218

@@ -510,7 +510,7 @@ describe('TypeScript Module Parser', () => {
510510
it(`should not parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array`, () => {
511511
expect(() =>
512512
parseParamTypeObjectLiteralProp('prop', 'Array'),
513-
).toThrow(IncorrectlyParameterizedGenericParserError);
513+
).toThrow(MissingTypeParameterGenericParserError);
514514
});
515515

516516
function parseArrayElementType(
@@ -780,7 +780,7 @@ describe('TypeScript Module Parser', () => {
780780
describe('Array Types', () => {
781781
it(`should not parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Array'`, () => {
782782
expect(() => parseReturnType('Array')).toThrow(
783-
IncorrectlyParameterizedGenericParserError,
783+
MissingTypeParameterGenericParserError,
784784
);
785785
});
786786

@@ -1049,7 +1049,7 @@ describe('TypeScript Module Parser', () => {
10491049
it(`should not parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array`, () => {
10501050
expect(() =>
10511051
parseObjectLiteralReturnTypeProp('prop', 'Array'),
1052-
).toThrow(IncorrectlyParameterizedGenericParserError);
1052+
).toThrow(MissingTypeParameterGenericParserError);
10531053
});
10541054

10551055
function parseArrayElementType(

0 commit comments

Comments
 (0)