Skip to content

Commit

Permalink
Revert the changes for special-casing the Optional type.
Browse files Browse the repository at this point in the history
  • Loading branch information
skinny85 committed Jan 26, 2024
1 parent ac69cd0 commit 585a595
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 88 deletions.
14 changes: 5 additions & 9 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,11 @@ User user = UserBuilder.user()
```

In addition to the `@Opt` annotation,
a property will always be considered optional if either:

* The field or parameter it represents is annotated with a `@Nullable` annotation.
All types of `@Nullable` annotations are supported,
including `javax.annotation.Nullable` from [JSR-305](https://mvnrepository.com/artifact/com.google.code.findbugs/jsr305),
`org.jetbrains.annotations.Nullable` from [JetBrains annotations](https://mvnrepository.com/artifact/org.jetbrains/annotations),
and others.
* Or, the field or parameter is of the type `java.util.Optional`,
introduced in [Java 8](https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html).
a property will always be considered optional if the field or parameter it was generated from is annotated with a `@Nullable` annotation.
All types of `@Nullable` annotations are supported,
including `javax.annotation.Nullable` from [JSR-305](https://mvnrepository.com/artifact/com.google.code.findbugs/jsr305),
`org.jetbrains.annotations.Nullable` from [JetBrains annotations](https://mvnrepository.com/artifact/org.jetbrains/annotations),
and others.

##### 'Staged, but preserving order' Builder style

Expand Down
17 changes: 5 additions & 12 deletions src/main/java/org/jilt/Opt.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,11 @@
* for the given optional property when constructing an instance of the target class.
* <p>
* In addition to using this annotation,
* a property will also be considered optional if either:
* <ul>
* <li>
* The field or parameter it represents is annotated with a {@code @Nullable} annotation.
* All types of {@code Nullable} annotations are supported,
* including {@code javax.annotation.Nullable}, {@code org.jetbrains.annotations.Nullable},
* and others.
* </li>
* <li>
* Or, the field or parameter is of the type {@code java.util.Optional}.
* </li>
* </ul>
* a property will also be considered optional if the field or parameter it was
* generated from is annotated with a {@code @Nullable} annotation.
* All types of {@code Nullable} annotations are supported,
* including {@code javax.annotation.Nullable}, {@code org.jetbrains.annotations.Nullable}, and others.
* <p>
* How exactly does the API for skipping optional properties look like for the consumers of the generated Builder class
* depends on the choice of the {@link Builder#style} attribute used -
* see the {@link BuilderStyle} enum values documentation for details.
Expand Down
46 changes: 2 additions & 44 deletions src/main/java/org/jilt/internal/AbstractBuilderGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import javax.lang.model.element.VariableElement;
import javax.lang.model.util.Elements;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -141,8 +142,6 @@ private List<MethodSpec> generateBuilderSetterMethods(VariableElement attribute)

protected final List<MethodSpec> generateSetterMethods(VariableElement attribute, boolean mangleTypeParameters,
boolean abstractMethod) {
List<MethodSpec> ret = new ArrayList<MethodSpec>(2);

String fieldName = this.attributeSimpleName(attribute);
TypeName parameterType = this.attributeType(attribute, mangleTypeParameters);
MethodSpec.Builder setter = MethodSpec
Expand All @@ -156,37 +155,8 @@ protected final List<MethodSpec> generateSetterMethods(VariableElement attribute
setter.addStatement("this.$1L = $1L", fieldName)
.addStatement("return this");
}
ret.add(setter.build());

if (this.typeIsJavaUtilOptional(parameterType)) {
TypeName optionalValueType;
if (parameterType instanceof ParameterizedTypeName) {
ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) parameterType;
optionalValueType = parameterizedTypeName.typeArguments.get(0);
if (optionalValueType instanceof WildcardTypeName) {
// for wildcards, we use java.lang.Object too
optionalValueType = ClassName.OBJECT;
}
} else {
// if Optional is used as a raw type, use java.lang.Object as the type argument
optionalValueType = ClassName.OBJECT;
}
MethodSpec.Builder optionSetter = MethodSpec
.methodBuilder(this.setterMethodName(attribute))
.addModifiers(Modifier.PUBLIC)
.returns(this.returnTypeForSetterFor(attribute, mangleTypeParameters))
.addParameter(this.setterParameterSpec(attribute, optionalValueType));
if (abstractMethod) {
optionSetter.addModifiers(Modifier.ABSTRACT);
} else {
optionSetter.addStatement("this.$1L = $2T.ofNullable($1L)",
fieldName, ClassName.get("java.util", "Optional"))
.addStatement("return this");
}
ret.add(optionSetter.build());
}

return ret;
return Collections.singletonList(setter.build());
}

protected abstract TypeName returnTypeForSetterFor(VariableElement attribute, boolean withMangledTypeParameters);
Expand Down Expand Up @@ -275,21 +245,9 @@ private boolean determineIfAttributeIsOptional(VariableElement attribute) {
if (this.firstAnnotationCalledNullable(attribute) != null) {
return true;
}
if (this.typeIsJavaUtilOptional(ClassName.get(attribute.asType()))) {
return true;
}
return false;
}

private boolean typeIsJavaUtilOptional(TypeName attributeType) {
// Optional most likely has a type parameter, so use toString() and startsWith()
// for comparison, disregarding the type parameter
String attributeTypeString = attributeType.toString();
return attributeTypeString.startsWith("java.util.Optional<")
// Optional can be used as a raw type
|| "java.util.Optional".equals(attributeTypeString);
}

private AnnotationMirror firstAnnotationCalledNullable(VariableElement attribute) {
for (AnnotationMirror annotation : attribute.getAnnotationMirrors()) {
if (annotationIsCalledNullable(annotation)) {
Expand Down
31 changes: 10 additions & 21 deletions src/test/java/org/jilt/test/OptionalsValueTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,56 +17,45 @@

public class OptionalsValueTest {
@Test
public void optional_type_property_is_implicitly_optional() {
public void optional_type_property_is_not_implicitly_optional() {
OptionalsValue<String> value = OptionalsValueBuilder.<String>optionalsValue()
.v(null)
.optional(Optional.of("abc"))
.build();

assertThat(value.optional).contains("abc");
assertThat(value.v).isNull();
}

@Test
public void optional_property_has_unwrapped_setter() {
OptionalsValue<Integer> value = OptionalsValueBuilder.<Integer>optionalsValue()
.v(null)
.optional(33)
.build();

assertThat(value.optional).contains(33);
assertThat(value.optional).contains("abc");
assertThat(value.v).isNull();
}

@Test
public void raw_optional_uses_java_lang_object_in_unwrapped_setter() {
public void raw_optional_uses_java_lang_object_in_setter() {
Object someObject = new Object();
OptionalsRawValue value = OptionalsRawValueBuilder.optionalsRawValue()
.rawOptional(someObject)
.rawOptional(Optional.of(someObject))
.build();

//noinspection unchecked
assertThat(value.rawOptional).contains(someObject);
}

@Test
public void wildcard_optional_uses_java_lang_object_in_unwrapped_setter() {
public void wildcard_optional_uses_java_lang_object_in_setter() {
Object someObject = new Object();
OptionalsWildcardValue value = OptionalsWildcardValueBuilder.optionalsWildcardValue()
.wildcardOptional(someObject)
.wildcardOptional(Optional.of(someObject))
.build();

assertThat(value.wildcardOptional).isEqualTo(Optional.of(someObject));
}

@Test
public void null_can_be_passed_to_optional_unwrapped_setter() {
List<String> nullList = null;
OptionalsWithOrderValue<CharSequence> value = OptionalsWithOrderValueBuilder.<CharSequence>optionalsWithOrderValue()
public void null_can_be_passed_to_optional_setter() {
Optional<List<CharSequence>> nullList = null;
OptionalsWithOrderValue<String> value = OptionalsWithOrderValueBuilder.<String>optionalsWithOrderValue()
.optional(nullList)
.v(null)
.build();

assertThat(value.optional).isEmpty();
assertThat(value.optional).isNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

@Builder(style = BuilderStyle.STAGED_PRESERVING_ORDER)
public final class OptionalsWithOrderValue<T> {
public final Optional<List<? extends T>> optional;
public final Optional<? extends List<? super T>> optional;
public final Void v;

public OptionalsWithOrderValue(Optional<List<? extends T>> optional, Void v) {
public OptionalsWithOrderValue(Optional<? extends List<? super T>> optional, Void v) {
this.optional = optional;
this.v = v;
}
Expand Down

0 comments on commit 585a595

Please sign in to comment.