From a1d374e504e8e4ea9802f46a6f05c46165f09ad9 Mon Sep 17 00:00:00 2001 From: Kevin Gozali Date: Mon, 26 Oct 2020 21:59:20 -0700 Subject: [PATCH] Codegen Android: optional NativeModule method should not be marked abstract Summary: This fixed a bug in the JS Java spec generator. Optional methods were still marked `abstract` before this fix. Instead it should be a normal method with potentially falsy return value. The JavaPoet version does this correctly already, but there was a minor typo with void return type vs optional. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D24524432 fbshipit-source-id: 57a248580a78bc255f34d0492ebe3a4691e66667 --- .../resolver/FunctionResolvedType.java | 1 + .../generator/resolver/VoidResolvedType.java | 2 +- .../modules/GenerateModuleJavaSpec.js | 126 +++++++++++++++--- .../GenerateModuleJavaSpec-test.js.snap | 6 +- 4 files changed, 115 insertions(+), 20 deletions(-) diff --git a/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/FunctionResolvedType.java b/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/FunctionResolvedType.java index 067f0b3c7a2..2a7468ce11e 100644 --- a/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/FunctionResolvedType.java +++ b/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/FunctionResolvedType.java @@ -107,6 +107,7 @@ public final class FunctionResolvedType extends ResolvedType { } private static @Nullable String getFalsyReturnStatement(TypeName returnType) { + // TODO: Handle nullable falsy return. if (returnType == TypeName.BOOLEAN) { return "return false"; } else if (returnType == TypeName.DOUBLE) { diff --git a/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/VoidResolvedType.java b/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/VoidResolvedType.java index 4a4e0c823de..0665ddf548a 100644 --- a/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/VoidResolvedType.java +++ b/packages/react-native-codegen/android/gradlePlugin-build/gradlePlugin/src/main/java/com/facebook/react/codegen/generator/resolver/VoidResolvedType.java @@ -26,7 +26,7 @@ public final class VoidResolvedType extends ResolvedType { @Override public TypeName getNativeType(final NativeTypeContext typeContext) { - return TypeUtils.makeNullable(TypeName.VOID, mNullable); + return TypeName.VOID; } @Override diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js index 9d2932e1e3d..dcb39b11ebd 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js @@ -26,17 +26,15 @@ const {unwrapNullable} = require('../../parsers/flow/modules/utils'); type FilesOutput = Map; -const FileTemplate = ({ - packageName, - className, - methods, - imports, -}: $ReadOnly<{| - packageName: string, - className: string, - methods: string, - imports: string, -|}>) => { +function FileTemplate( + config: $ReadOnly<{| + packageName: string, + className: string, + methods: string, + imports: string, + |}>, +): string { + const {packageName, className, methods, imports} = config; return ` /** * ${'C'}opyright (c) Facebook, Inc. and its affiliates. @@ -61,7 +59,37 @@ public abstract class ${className} extends ReactContextBaseJavaModule implements ${methods} } `; -}; +} + +function MethodTemplate( + config: $ReadOnly<{| + abstract: boolean, + methodBody: ?string, + methodJavaAnnotation: string, + methodName: string, + translatedReturnType: string, + traversedArgs: Array, + |}>, +): string { + const { + abstract, + methodBody, + methodJavaAnnotation, + methodName, + translatedReturnType, + traversedArgs, + } = config; + const methodQualifier = abstract ? 'abstract ' : ''; + const methodClosing = abstract + ? ';' + : methodBody != null && methodBody.length > 0 + ? ` { ${methodBody} }` + : ' {}'; + return ` ${methodJavaAnnotation} + public ${methodQualifier}${translatedReturnType} ${methodName}(${traversedArgs.join( + ', ', + )})${methodClosing}`; +} function translateFunctionParamToJavaType( param: NativeModuleMethodParamSchema, @@ -199,6 +227,60 @@ function translateFunctionReturnTypeToJavaType( } } +function getFalsyReturnStatementFromReturnType( + nullableReturnTypeAnnotation: Nullable, + createErrorMessage: (typeName: string) => string, + resolveAlias: AliasResolver, +): string { + const [ + returnTypeAnnotation, + nullable, + ] = unwrapNullable( + nullableReturnTypeAnnotation, + ); + + let realTypeAnnotation = returnTypeAnnotation; + if (realTypeAnnotation.type === 'TypeAliasTypeAnnotation') { + realTypeAnnotation = resolveAlias(realTypeAnnotation.name); + } + + switch (realTypeAnnotation.type) { + case 'ReservedFunctionValueTypeAnnotation': + switch (realTypeAnnotation.name) { + case 'RootTag': + return 'return 0.0;'; + default: + (realTypeAnnotation.name: empty); + throw new Error(createErrorMessage(realTypeAnnotation.name)); + } + case 'VoidTypeAnnotation': + return ''; + case 'PromiseTypeAnnotation': + return ''; + case 'NumberTypeAnnotation': + return nullable ? 'return null;' : 'return 0;'; + case 'FloatTypeAnnotation': + return nullable ? 'return null;' : 'return 0.0;'; + case 'DoubleTypeAnnotation': + return nullable ? 'return null;' : 'return 0.0;'; + case 'Int32TypeAnnotation': + return nullable ? 'return null;' : 'return 0;'; + case 'BooleanTypeAnnotation': + return nullable ? 'return null;' : 'return false;'; + case 'StringTypeAnnotation': + return nullable ? 'return null;' : 'return "";'; + case 'ObjectTypeAnnotation': + return 'return null;'; + case 'GenericObjectTypeAnnotation': + return 'return null;'; + case 'ArrayTypeAnnotation': + return 'return null;'; + default: + (realTypeAnnotation.type: empty); + throw new Error(createErrorMessage(realTypeAnnotation.type)); + } +} + // Build special-cased runtime check for getConstants(). function buildGetConstantsMethod( method: NativeModulePropertySchema, @@ -360,10 +442,22 @@ module.exports = { const methodJavaAnnotation = `@ReactMethod${ isSyncMethod ? '(isBlockingSynchronousMethod = true)' : '' }`; - return ` ${methodJavaAnnotation} - public abstract ${translatedReturnType} ${method.name}(${traversedArgs.join( - ', ', - )});`; + const methodBody = method.optional + ? getFalsyReturnStatementFromReturnType( + methodTypeAnnotation.returnTypeAnnotation, + typeName => + `Cannot build falsy return statement for return type for method ${method.name}. Found: ${typeName}`, + resolveAlias, + ) + : null; + return MethodTemplate({ + abstract: !method.optional, + methodBody, + methodJavaAnnotation, + methodName: method.name, + translatedReturnType, + traversedArgs, + }); }); files.set( 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 f66fdb0e275..a5564d2fbaa 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 @@ -38,7 +38,7 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo public abstract void optionals(ReadableMap A); @ReactMethod - public abstract void optionalMethod(ReadableMap options, Callback callback, ReadableArray extras); + public void optionalMethod(ReadableMap options, Callback callback, ReadableArray extras) {} @ReactMethod public abstract void getArrays(ReadableMap options); @@ -189,13 +189,13 @@ public abstract class NativeExceptionsManagerSpec extends ReactContextBaseJavaMo public abstract void reportSoftException(String message, ReadableArray stack, double exceptionId); @ReactMethod - public abstract void reportException(ReadableMap data); + public void reportException(ReadableMap data) {} @ReactMethod public abstract void updateExceptionMessage(String message, ReadableArray stack, double exceptionId); @ReactMethod - public abstract void dismissRedbox(); + public void dismissRedbox() {} } ", }