Skip to content

Commit

Permalink
BREAKING: Throwing validation error at compile time when passing Comp…
Browse files Browse the repository at this point in the history
…onent.Builder or any of its inherited classes

Summary:
Changed PropValidation and tests to throw validation errors when component
build or component inherited classes are declared as prop argument.

Reviewed By: thurn

Differential Revision: D16622495

fbshipit-source-id: 9a30bfde5a63cb6a4400059495de0825799eebeb
  • Loading branch information
rahulsbisen authored and facebook-github-bot committed Aug 10, 2019
1 parent fa3466b commit 5da7121
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.facebook.litho.annotations.ResType;
import com.facebook.litho.specmodels.internal.ImmutableList;
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeName;
import java.util.ArrayList;
Expand All @@ -49,6 +50,8 @@ public void setup() {
when(mPropModel2.getName()).thenReturn("name2");
when(mPropModel1.getTypeName()).thenReturn(TypeName.BOOLEAN);
when(mPropModel2.getTypeName()).thenReturn(TypeName.INT);
when(mPropModel1.getTypeSpec()).thenReturn(new TypeSpec(TypeName.BOOLEAN));
when(mPropModel2.getTypeSpec()).thenReturn(new TypeSpec(TypeName.INT));
when(mPropModel1.isOptional()).thenReturn(false);
when(mPropModel2.isOptional()).thenReturn(false);
when(mPropModel1.getResType()).thenReturn(ResType.NONE);
Expand Down Expand Up @@ -179,17 +182,51 @@ public void testPropMarkedOverrideCommonButNotCommon() {

@Test
public void testPropWithReservedType() {
when(mPropModel1.getTypeName()).thenReturn(ClassNames.COMPONENT_LAYOUT);
when(mPropModel1.getTypeSpec())
.thenReturn(
new TypeSpec.DeclaredTypeSpec(
ClassNames.COMPONENT_BUILDER,
ClassNames.COMPONENT_BUILDER.toString(),
() -> new TypeSpec(TypeName.OBJECT),
ImmutableList.of(),
ImmutableList.of()));

List<SpecModelValidationError> validationErrors =
PropValidation.validate(
mSpecModel, PropValidation.COMMON_PROP_NAMES, PropValidation.VALID_COMMON_PROPS);
assertThat(validationErrors).hasSize(1);
assertThat(validationErrors.get(0).element).isEqualTo(mRepresentedObject1);
assertThat(validationErrors.get(0).message).isEqualTo(
"Props may not be declared with the following argument types: " +
"[com.facebook.litho.ComponentLayout, " +
"com.facebook.litho.Component.Builder].");
assertThat(validationErrors.get(0).message)
.isEqualTo(
"Props may not be declared with argument type: com.facebook.litho.Component.Builder or its inherited types.");
}

@Test
public void testPropInheritedFromReservedType() {
when(mPropModel1.getTypeSpec())
.thenReturn(
new TypeSpec.DeclaredTypeSpec(
ClassName.bestGuess("com.facebook.litho.Text.Builder"),
ClassName.bestGuess("com.facebook.litho.Text.Builder").toString(),
() ->
new TypeSpec.DeclaredTypeSpec(
ClassNames.COMPONENT_BUILDER,
ClassNames.COMPONENT_BUILDER.toString(),
() -> new TypeSpec(TypeName.OBJECT),
ImmutableList.of(),
ImmutableList.of()),
ImmutableList.of(),
ImmutableList.of()));

List<SpecModelValidationError> validationErrors =
PropValidation.validate(
mSpecModel, PropValidation.COMMON_PROP_NAMES, PropValidation.VALID_COMMON_PROPS);
assertThat(validationErrors).hasSize(1);
assertThat(validationErrors.get(0).element).isEqualTo(mRepresentedObject1);
assertThat(validationErrors.get(0).message)
.isEqualTo(
"Props may not be declared with argument type: com.facebook.litho.Component.Builder or its inherited types. "
+ "com.facebook.litho.Text.Builder is an inherited type of com.facebook.litho.Component.Builder");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ static List<SpecModelValidationError> validate(
}
}

TypeName argumentType = null;
if (prop.hasVarArgs()) {
TypeName typeName = prop.getTypeName();
if (typeName instanceof ParameterizedTypeName) {
Expand All @@ -377,25 +376,36 @@ static List<SpecModelValidationError> validate(
prop.getRepresentedObject(),
prop.getName() + " is a variable argument, and thus should be a List<> type."));
}
argumentType = parameterizedTypeName.typeArguments.get(0);
} else {
validationErrors.add(
new SpecModelValidationError(
prop.getRepresentedObject(),
prop.getName()
+ " is a variable argument, and thus requires a parameterized List type."));
}
} else {
argumentType = prop.getTypeName();
}

if (ILLEGAL_PROP_TYPES.contains(argumentType)) {
validationErrors.add(
new SpecModelValidationError(
prop.getRepresentedObject(),
"Props may not be declared with the following argument types: "
+ ILLEGAL_PROP_TYPES
+ "."));
TypeSpec typeSpec = prop.getTypeSpec();
for (TypeName illegalPropType : ILLEGAL_PROP_TYPES) {

if (typeSpec.isSameDeclaredType(illegalPropType)) {
validationErrors.add(
new SpecModelValidationError(
prop.getRepresentedObject(),
"Props may not be declared with argument type: "
+ illegalPropType
+ " or its inherited types."));
} else if (typeSpec.isSubType(illegalPropType)) {
validationErrors.add(
new SpecModelValidationError(
prop.getRepresentedObject(),
"Props may not be declared with argument type: "
+ illegalPropType
+ " or its inherited types. "
+ typeSpec.getTypeName()
+ " is an inherited type of "
+ illegalPropType));
}
}

if (!prop.isOptional() && prop.hasDefault(specModel.getPropDefaults())) {
Expand Down

0 comments on commit 5da7121

Please sign in to comment.