Skip to content

Commit

Permalink
add missing struct member initialization in Props.h (#44294)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #44294

**Problem:**

It was discovered while testing 3 party library, generated member variables in a C++ `struct` in `Props.h` is not initialized.
Also `WithDefault` would not work as well.
(For the problematic case it was a `boolean` but would also apply to other primitive types.)

If there is no default initialization and the component prop is optional and the user of the native component does not set the prop then the variable is never initialized and this is problematic for primitive types in C++ where no initialization results in an undefined behavior.

**Proposed solution:**

(Following C++Core Guideline of [always initialize](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-always).)
Reusing `generatePropsString()` used by `ClassTemplate` to generate props for `StructTemplate` as well.
updated relevant test snapshots.
This change is only concerning the `Props.h` file.

**Changelog:**
[General][Fixed] - fixed `Props.h` created from codegen missing default initializers in C++ `struct`

Reviewed By: cipolleschi

Differential Revision: D56659457

fbshipit-source-id: 0d21ad20c0491a7e8bb718cd3156da65def72f23
  • Loading branch information
@alanleedev @cipolleschi
alanleedev authored and cipolleschi committed May 1, 2024
1 parent 59e7ed5 commit 1373951
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 52 deletions.
26 changes: 13 additions & 13 deletions ...s/react-native-codegen/e2e/__tests__/components/__snapshots__/GeneratePropsH-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ static inline std::string toString(const ArrayPropsNativeComponentViewSizesMask
return result;
}
struct ArrayPropsNativeComponentViewObjectStruct {
std::string prop;
std::string prop{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentViewObjectStruct &result) {
Expand All @@ -109,8 +109,8 @@ static inline void fromRawValue(const PropsParserContext& context, const RawValu
struct ArrayPropsNativeComponentViewArrayOfObjectsStruct {
Float prop1;
int prop2;
Float prop1{0.0};
int prop2{0};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentViewArrayOfObjectsStruct &result) {
Expand Down Expand Up @@ -756,12 +756,12 @@ static inline std::string toString(const ObjectPropsNativeComponentIntEnumProp &
}
}
struct ObjectPropsNativeComponentObjectPropStruct {
std::string stringProp;
bool booleanProp;
Float floatProp;
int intProp;
ObjectPropsNativeComponentStringEnumProp stringEnumProp;
ObjectPropsNativeComponentIntEnumProp intEnumProp;
std::string stringProp{\\"\\"};
bool booleanProp{false};
Float floatProp{0.0};
int intProp{0};
ObjectPropsNativeComponentStringEnumProp stringEnumProp{ObjectPropsNativeComponentStringEnumProp::Small};
ObjectPropsNativeComponentIntEnumProp intEnumProp{ObjectPropsNativeComponentIntEnumProp::IntEnumProp0};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsNativeComponentObjectPropStruct &result) {
Expand Down Expand Up @@ -798,7 +798,7 @@ static inline std::string toString(const ObjectPropsNativeComponentObjectPropStr
}
struct ObjectPropsNativeComponentObjectArrayPropStruct {
std::vector<std::string> array;
std::vector<std::string> array{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsNativeComponentObjectArrayPropStruct &result) {
Expand All @@ -815,9 +815,9 @@ static inline std::string toString(const ObjectPropsNativeComponentObjectArrayPr
}
struct ObjectPropsNativeComponentObjectPrimitiveRequiredPropStruct {
ImageSource image;
SharedColor color;
Point point;
ImageSource image{};
SharedColor color{};
Point point{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsNativeComponentObjectPrimitiveRequiredPropStruct &result) {
Expand Down
9 changes: 9 additions & 0 deletions packages/react-native-codegen/src/generators/components/CppHelpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ function getEnumMaskName(enumName: string): string {
return `${enumName}Mask`;
}
function getDefaultInitializerString(
componentName: string,
prop: NamedShape<PropTypeAnnotation>,
): string {
const defaultValue = convertDefaultTypeToString(componentName, prop);
return `{${defaultValue}}`;
}
function convertDefaultTypeToString(
componentName: string,
prop: NamedShape<PropTypeAnnotation>,
Expand Down Expand Up @@ -270,6 +278,7 @@ const IncludeTemplate = ({
};

module.exports = {
getDefaultInitializerString,
convertDefaultTypeToString,
getCppArrayTypeForAnnotation,
getCppTypeForAnnotation,
Expand Down
32 changes: 18 additions & 14 deletions packages/react-native-codegen/src/generators/components/GeneratePropsH.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ const {
getNativeTypeFromAnnotation,
} = require('./ComponentsGeneratorUtils.js');
const {
convertDefaultTypeToString,
generateStructName,
getDefaultInitializerString,
getEnumMaskName,
toIntEnumValueName,
} = require('./CppHelpers.js');
Expand Down Expand Up @@ -456,13 +456,21 @@ function generateEnumString(
function generatePropsString(
componentName: string,
props: $ReadOnlyArray<NamedShape<PropTypeAnnotation>>,
nameParts: $ReadOnlyArray<string>,
) {
return props
.map(prop => {
const nativeType = getNativeTypeFromAnnotation(componentName, prop, []);
const defaultValue = convertDefaultTypeToString(componentName, prop);
const nativeType = getNativeTypeFromAnnotation(
componentName,
prop,
nameParts,
);
const defaultInitializer = getDefaultInitializerString(
componentName,
prop,
);

return `${nativeType} ${prop.name}{${defaultValue}};`;
return `${nativeType} ${prop.name}${defaultInitializer};`;
})
.join('\n' + ' ');
}
Expand Down Expand Up @@ -629,16 +637,11 @@ function generateStruct(
): void {
const structNameParts = nameParts;
const structName = generateStructName(componentName, structNameParts);
const fields = properties
.map(property => {
return `${getNativeTypeFromAnnotation(
componentName,
property,
structNameParts,
)} ${property.name};`;
})
.join('\n' + ' ');
const fields = generatePropsString(
componentName,
properties,
structNameParts,
);
properties.forEach((property: NamedShape<PropTypeAnnotation>) => {
const name = property.name;
Expand Down Expand Up @@ -738,6 +741,7 @@ module.exports = {
const propsString = generatePropsString(
componentName,
component.props,
[],
);
const extendString = getClassExtendString(component);
const extendsImports = getExtendsImports(component.extendsProps);
Expand Down
32 changes: 32 additions & 0 deletions packages/react-native-codegen/src/generators/components/__test_fixtures__/fixtures.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,38 @@ const OBJECT_PROPS: SchemaType = {
default: 0,
},
},
{
name: 'stringUserDefaultProp',
optional: true,
typeAnnotation: {
type: 'StringTypeAnnotation',
default: 'user_default',
},
},
{
name: 'booleanUserDefaultProp',
optional: true,
typeAnnotation: {
type: 'BooleanTypeAnnotation',
default: true,
},
},
{
name: 'floatUserDefaultProp',
optional: true,
typeAnnotation: {
type: 'FloatTypeAnnotation',
default: 3.14,
},
},
{
name: 'intUserDefaultProp',
optional: true,
typeAnnotation: {
type: 'Int32TypeAnnotation',
default: 9999,
},
},
{
name: 'stringEnumProp',
optional: true,
Expand Down
70 changes: 45 additions & 25 deletions ...ive-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsH-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static inline std::string toString(const ArrayPropsNativeComponentSizesMask &val
return result;
}
struct ArrayPropsNativeComponentObjectStruct {
std::string stringProp;
std::string stringProp{\\"\\"};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentObjectStruct &result) {
Expand All @@ -108,7 +108,7 @@ static inline void fromRawValue(const PropsParserContext& context, const RawValu
struct ArrayPropsNativeComponentArrayObjectStruct {
std::string stringProp;
std::string stringProp{\\"\\"};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentArrayObjectStruct &result) {
Expand All @@ -135,7 +135,7 @@ static inline void fromRawValue(const PropsParserContext& context, const RawValu
struct ArrayPropsNativeComponentArrayStruct {
std::vector<ArrayPropsNativeComponentArrayObjectStruct> object;
std::vector<ArrayPropsNativeComponentArrayObjectStruct> object{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentArrayStruct &result) {
Expand All @@ -162,7 +162,7 @@ static inline void fromRawValue(const PropsParserContext& context, const RawValu
struct ArrayPropsNativeComponentArrayOfArrayOfObjectStruct {
std::string stringProp;
std::string stringProp{\\"\\"};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentArrayOfArrayOfObjectStruct &result) {
Expand Down Expand Up @@ -242,9 +242,9 @@ Map {
namespace facebook::react {
struct ArrayPropsNativeComponentNativePrimitivesStruct {
std::vector<SharedColor> colors;
std::vector<ImageSource> srcs;
std::vector<Point> points;
std::vector<SharedColor> colors{};
std::vector<ImageSource> srcs{};
std::vector<Point> points{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ArrayPropsNativeComponentNativePrimitivesStruct &result) {
Expand Down Expand Up @@ -1100,7 +1100,7 @@ static inline std::string toString(const ObjectPropsIntEnumProp &value) {
}
}
struct ObjectPropsObjectPropObjectArrayPropStruct {
std::vector<std::string> array;
std::vector<std::string> array{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropObjectArrayPropStruct &result) {
Expand All @@ -1117,9 +1117,9 @@ static inline std::string toString(const ObjectPropsObjectPropObjectArrayPropStr
}
struct ObjectPropsObjectPropObjectPrimitiveRequiredPropStruct {
ImageSource image;
SharedColor color;
Point point;
ImageSource image{};
SharedColor color{};
Point point{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropObjectPrimitiveRequiredPropStruct &result) {
Expand All @@ -1144,7 +1144,7 @@ static inline std::string toString(const ObjectPropsObjectPropObjectPrimitiveReq
}
struct ObjectPropsObjectPropNestedPropANestedPropBStruct {
std::string nestedPropC;
std::string nestedPropC{\\"\\"};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropNestedPropANestedPropBStruct &result) {
Expand All @@ -1161,7 +1161,7 @@ static inline std::string toString(const ObjectPropsObjectPropNestedPropANestedP
}
struct ObjectPropsObjectPropNestedPropAStruct {
ObjectPropsObjectPropNestedPropANestedPropBStruct nestedPropB;
ObjectPropsObjectPropNestedPropANestedPropBStruct nestedPropB{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropNestedPropAStruct &result) {
Expand All @@ -1178,7 +1178,7 @@ static inline std::string toString(const ObjectPropsObjectPropNestedPropAStruct
}
struct ObjectPropsObjectPropNestedArrayAsPropertyArrayPropStruct {
std::string stringProp;
std::string stringProp{\\"\\"};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropNestedArrayAsPropertyArrayPropStruct &result) {
Expand All @@ -1205,7 +1205,7 @@ static inline void fromRawValue(const PropsParserContext& context, const RawValu
struct ObjectPropsObjectPropNestedArrayAsPropertyStruct {
std::vector<ObjectPropsObjectPropNestedArrayAsPropertyArrayPropStruct> arrayProp;
std::vector<ObjectPropsObjectPropNestedArrayAsPropertyArrayPropStruct> arrayProp{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropNestedArrayAsPropertyStruct &result) {
Expand All @@ -1222,16 +1222,20 @@ static inline std::string toString(const ObjectPropsObjectPropNestedArrayAsPrope
}
struct ObjectPropsObjectPropStruct {
std::string stringProp;
bool booleanProp;
Float floatProp;
int intProp;
ObjectPropsStringEnumProp stringEnumProp;
ObjectPropsIntEnumProp intEnumProp;
ObjectPropsObjectPropObjectArrayPropStruct objectArrayProp;
ObjectPropsObjectPropObjectPrimitiveRequiredPropStruct objectPrimitiveRequiredProp;
ObjectPropsObjectPropNestedPropAStruct nestedPropA;
ObjectPropsObjectPropNestedArrayAsPropertyStruct nestedArrayAsProperty;
std::string stringProp{\\"\\"};
bool booleanProp{false};
Float floatProp{0.0};
int intProp{0};
std::string stringUserDefaultProp{\\"user_default\\"};
bool booleanUserDefaultProp{true};
Float floatUserDefaultProp{3.14};
int intUserDefaultProp{9999};
ObjectPropsStringEnumProp stringEnumProp{ObjectPropsStringEnumProp::Option1};
ObjectPropsIntEnumProp intEnumProp{ObjectPropsIntEnumProp::IntEnumProp0};
ObjectPropsObjectPropObjectArrayPropStruct objectArrayProp{};
ObjectPropsObjectPropObjectPrimitiveRequiredPropStruct objectPrimitiveRequiredProp{};
ObjectPropsObjectPropNestedPropAStruct nestedPropA{};
ObjectPropsObjectPropNestedArrayAsPropertyStruct nestedArrayAsProperty{};
};
static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ObjectPropsObjectPropStruct &result) {
Expand All @@ -1253,6 +1257,22 @@ static inline void fromRawValue(const PropsParserContext& context, const RawValu
if (tmp_intProp != map.end()) {
fromRawValue(context, tmp_intProp->second, result.intProp);
}
auto tmp_stringUserDefaultProp = map.find(\\"stringUserDefaultProp\\");
if (tmp_stringUserDefaultProp != map.end()) {
fromRawValue(context, tmp_stringUserDefaultProp->second, result.stringUserDefaultProp);
}
auto tmp_booleanUserDefaultProp = map.find(\\"booleanUserDefaultProp\\");
if (tmp_booleanUserDefaultProp != map.end()) {
fromRawValue(context, tmp_booleanUserDefaultProp->second, result.booleanUserDefaultProp);
}
auto tmp_floatUserDefaultProp = map.find(\\"floatUserDefaultProp\\");
if (tmp_floatUserDefaultProp != map.end()) {
fromRawValue(context, tmp_floatUserDefaultProp->second, result.floatUserDefaultProp);
}
auto tmp_intUserDefaultProp = map.find(\\"intUserDefaultProp\\");
if (tmp_intUserDefaultProp != map.end()) {
fromRawValue(context, tmp_intUserDefaultProp->second, result.intUserDefaultProp);
}
auto tmp_stringEnumProp = map.find(\\"stringEnumProp\\");
if (tmp_stringEnumProp != map.end()) {
fromRawValue(context, tmp_stringEnumProp->second, result.stringEnumProp);
Expand Down
20 changes: 20 additions & 0 deletions ...egen/src/generators/components/__tests__/__snapshots__/GeneratePropsJavaPojo-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,10 @@ public class ObjectPropsPropsObjectProp {
private boolean mBooleanProp;
private float mFloatProp;
private int mIntProp;
private @Nullable String mStringUserDefaultProp;
private boolean mBooleanUserDefaultProp;
private float mFloatUserDefaultProp;
private int mIntUserDefaultProp;
private @Nullable String mStringEnumProp;
private @Nullable Integer mIntEnumProp;
private ObjectPropsPropsObjectPropObjectArrayProp mObjectArrayProp;
Expand All @@ -1047,6 +1051,22 @@ public class ObjectPropsPropsObjectProp {
return mIntProp;
}
@DoNotStrip
public @Nullable String getStringUserDefaultProp() {
return mStringUserDefaultProp;
}
@DoNotStrip
public boolean getBooleanUserDefaultProp() {
return mBooleanUserDefaultProp;
}
@DoNotStrip
public float getFloatUserDefaultProp() {
return mFloatUserDefaultProp;
}
@DoNotStrip
public int getIntUserDefaultProp() {
return mIntUserDefaultProp;
}
@DoNotStrip
public @Nullable String getStringEnumProp() {
return mStringEnumProp;
}
Expand Down

0 comments on commit 1373951

Please sign in to comment.