From 6db39951755cef82f06e23a5464cf1caf53c7966 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Mon, 7 Nov 2022 05:13:30 -0800 Subject: [PATCH] Improve @Nullable annotions in Java TurboModule codegen Summary: Noticed these types could be improved based on the tests added in D40979066 (https://github.com/facebook/react-native/commit/e81c98c842380d8b72c1dc8d4a6e64f760e2a58c). Changelog: [Android][Fixed] Corrected Nullable annotations for parameters and return values in TurboModules codegen Reviewed By: mdvacca, cipolleschi Differential Revision: D40979940 fbshipit-source-id: cfc352a9e7eb9f59e2cce3d7da110a9a8d32db4b --- .../modules/GenerateModuleJavaSpec.js | 68 +++++++++---------- .../GenerateModuleJavaSpec-test.js.snap | 4 +- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js index 0b807bbbfa966e..8ef4278dbc9954 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js @@ -104,14 +104,15 @@ function translateFunctionParamToJavaType( unwrapNullable(nullableTypeAnnotation); const isRequired = !optional && !nullable; - function wrapIntoNullableIfNeeded(generatedType: string) { + function wrapNullable(javaType: string, nullableType?: string) { if (!isRequired) { imports.add('javax.annotation.Nullable'); - return `@Nullable ${generatedType}`; + return `@Nullable ${nullableType ?? javaType}`; } - return generatedType; + return javaType; } + // FIXME: support class alias for args let realTypeAnnotation = typeAnnotation; if (realTypeAnnotation.type === 'TypeAliasTypeAnnotation') { realTypeAnnotation = resolveAlias(realTypeAnnotation.name); @@ -121,49 +122,45 @@ function translateFunctionParamToJavaType( case 'ReservedTypeAnnotation': switch (realTypeAnnotation.name) { case 'RootTag': - return !isRequired ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); default: (realTypeAnnotation.name: empty); throw new Error(createErrorMessage(realTypeAnnotation.name)); } case 'StringTypeAnnotation': - return wrapIntoNullableIfNeeded('String'); + return wrapNullable('String'); case 'NumberTypeAnnotation': - return !isRequired ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'FloatTypeAnnotation': - return !isRequired ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'DoubleTypeAnnotation': - return !isRequired ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'Int32TypeAnnotation': - return !isRequired ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'BooleanTypeAnnotation': - return !isRequired ? 'Boolean' : 'boolean'; + return wrapNullable('boolean', 'Boolean'); case 'EnumDeclaration': switch (realTypeAnnotation.memberType) { case 'NumberTypeAnnotation': - return !isRequired ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'StringTypeAnnotation': - return wrapIntoNullableIfNeeded('String'); + return wrapNullable('String'); default: throw new Error(createErrorMessage(realTypeAnnotation.type)); } case 'ObjectTypeAnnotation': imports.add('com.facebook.react.bridge.ReadableMap'); - if (typeAnnotation.type === 'TypeAliasTypeAnnotation') { - // No class alias for args, so it still falls under ReadableMap. - return 'ReadableMap'; - } - return 'ReadableMap'; + return wrapNullable('ReadableMap'); case 'GenericObjectTypeAnnotation': // Treat this the same as ObjectTypeAnnotation for now. imports.add('com.facebook.react.bridge.ReadableMap'); - return 'ReadableMap'; + return wrapNullable('ReadableMap'); case 'ArrayTypeAnnotation': imports.add('com.facebook.react.bridge.ReadableArray'); - return 'ReadableArray'; + return wrapNullable('ReadableArray'); case 'FunctionTypeAnnotation': imports.add('com.facebook.react.bridge.Callback'); - return 'Callback'; + return wrapNullable('Callback'); default: (realTypeAnnotation.type: | 'EnumDeclaration' @@ -184,14 +181,15 @@ function translateFunctionReturnTypeToJavaType( nullableReturnTypeAnnotation, ); - function wrapIntoNullableIfNeeded(generatedType: string) { + function wrapNullable(javaType: string, nullableType?: string) { if (nullable) { imports.add('javax.annotation.Nullable'); - return `@Nullable ${generatedType}`; + return `@Nullable ${nullableType ?? javaType}`; } - return generatedType; + return javaType; } + // FIXME: support class alias for args let realTypeAnnotation = returnTypeAnnotation; if (realTypeAnnotation.type === 'TypeAliasTypeAnnotation') { realTypeAnnotation = resolveAlias(realTypeAnnotation.name); @@ -201,7 +199,7 @@ function translateFunctionReturnTypeToJavaType( case 'ReservedTypeAnnotation': switch (realTypeAnnotation.name) { case 'RootTag': - return nullable ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); default: (realTypeAnnotation.name: empty); throw new Error(createErrorMessage(realTypeAnnotation.name)); @@ -211,35 +209,35 @@ function translateFunctionReturnTypeToJavaType( case 'PromiseTypeAnnotation': return 'void'; case 'StringTypeAnnotation': - return wrapIntoNullableIfNeeded('String'); + return wrapNullable('String'); case 'NumberTypeAnnotation': - return nullable ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'FloatTypeAnnotation': - return nullable ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'DoubleTypeAnnotation': - return nullable ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'Int32TypeAnnotation': - return nullable ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'BooleanTypeAnnotation': - return nullable ? 'Boolean' : 'boolean'; + return wrapNullable('boolean', 'Boolean'); case 'EnumDeclaration': switch (realTypeAnnotation.memberType) { case 'NumberTypeAnnotation': - return nullable ? 'Double' : 'double'; + return wrapNullable('double', 'Double'); case 'StringTypeAnnotation': - return wrapIntoNullableIfNeeded('String'); + return wrapNullable('String'); default: throw new Error(createErrorMessage(realTypeAnnotation.type)); } case 'ObjectTypeAnnotation': imports.add('com.facebook.react.bridge.WritableMap'); - return wrapIntoNullableIfNeeded('WritableMap'); + return wrapNullable('WritableMap'); case 'GenericObjectTypeAnnotation': imports.add('com.facebook.react.bridge.WritableMap'); - return wrapIntoNullableIfNeeded('WritableMap'); + return wrapNullable('WritableMap'); case 'ArrayTypeAnnotation': imports.add('com.facebook.react.bridge.WritableArray'); - return wrapIntoNullableIfNeeded('WritableArray'); + return wrapNullable('WritableArray'); default: (realTypeAnnotation.type: | 'EnumDeclaration' diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap index 473878ebefa929..159c207e5d0814 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap @@ -78,7 +78,7 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo @ReactMethod @DoNotStrip - public void optionalMethod(ReadableMap options, Callback callback, ReadableArray extras) {} + public void optionalMethod(ReadableMap options, Callback callback, @Nullable ReadableArray extras) {} @ReactMethod @DoNotStrip @@ -379,7 +379,7 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo @ReactMethod @DoNotStrip - public abstract void getValueWithOptionalArg(ReadableMap parameter, Promise promise); + public abstract void getValueWithOptionalArg(@Nullable ReadableMap parameter, Promise promise); @ReactMethod(isBlockingSynchronousMethod = true) @DoNotStrip