Skip to content

Commit

Permalink
[fixes #2104] superbuilder + non-list-singulars wouldn’t work due to …
Browse files Browse the repository at this point in the history
…hardcoded call to emptyList.
  • Loading branch information
rzwitserloot committed May 1, 2019
1 parent bf04992 commit 2335f25
Show file tree
Hide file tree
Showing 16 changed files with 167 additions and 57 deletions.
1 change: 1 addition & 0 deletions doc/changelog.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Lombok Changelog
* BUGFIX: Fix for java6 regression if a field has javadoc. [Issue #2066](https://github.com/rzwitserloot/lombok/issues/2066)
* BUGFIX: Delombok now delomboks java10's own `var` as `var` and not as the actual underlying type. [Issue #2049](https://github.com/rzwitserloot/lombok/issues/2049)
* BUGFIX: If you use `@Builder` and manually write the `build()` method in your builder class, javac would error out instead of deferring to your implementation. [Issue #2050](https://github.com/rzwitserloot/lombok/issues/2050) [Issue #2061](https://github.com/rzwitserloot/lombok/issues/2061)
* BUGFIX: `@SuperBuilder` together with `@Singular` on non-lists would produce an erroneous `emptyList` call. [Issue #2104](https://github.com/rzwitserloot/lombok/issues/2104).
* IMPROBABLE BREAKING CHANGE: For fields and parameters marked non-null, if the method body starts with an assert statement to ensure the value isn't null, no code to throw an exception will be generated.

### v1.18.6 (February 12th, 2019)
Expand Down
3 changes: 3 additions & 0 deletions src/core/lombok/eclipse/handlers/EclipseSingularsRecipes.java
Original file line number Diff line number Diff line change
Expand Up @@ -395,5 +395,8 @@ protected static Reference getBuilderReference(String builderVariable) {
return new SingleNameReference(builderVariable.toCharArray(), 0);
}
}

protected abstract char[][] getEmptyMakerReceiver(String targetFqn);
protected abstract char[] getEmptyMakerSelector(String targetFqn);
}
}
11 changes: 5 additions & 6 deletions src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2013-2018 The Project Lombok Authors.
* Copyright (C) 2013-2019 The Project Lombok Authors.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -108,7 +108,6 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
private static final char[] TO_BUILDER_METHOD_NAME = TO_BUILDER_METHOD_NAME_STRING.toCharArray();
private static final char[] FILL_VALUES_METHOD_NAME = "$fillValuesFrom".toCharArray();
private static final char[] FILL_VALUES_STATIC_METHOD_NAME = "$fillValuesFromInstanceIntoBuilder".toCharArray();
private static final char[] EMPTY_LIST = "emptyList".toCharArray();
private static final char[] INSTANCE_VARIABLE_NAME = "instance".toCharArray();
private static final String BUILDER_VARIABLE_NAME_STRING = "b";
private static final char[] BUILDER_VARIABLE_NAME = BUILDER_VARIABLE_NAME_STRING.toCharArray();
Expand Down Expand Up @@ -741,10 +740,10 @@ private MessageSend createSetterCallWithInstanceValue(BuilderFieldData bfd, Ecli
ms.arguments = tgt;
} else {
Expression ifNull = new EqualExpression(tgt[0], new NullLiteral(0, 0), OperatorIds.EQUAL_EQUAL);
MessageSend emptyList = new MessageSend();
emptyList.receiver = generateQualifiedNameRef(source, TypeConstants.JAVA, TypeConstants.UTIL, "Collections".toCharArray());
emptyList.selector = EMPTY_LIST;
ms.arguments = new Expression[] {new ConditionalExpression(ifNull, emptyList, tgt[1])};
MessageSend emptyCollection = new MessageSend();
emptyCollection.receiver = generateQualifiedNameRef(source, bfd.singularData.getSingularizer().getEmptyMakerReceiver(bfd.singularData.getTargetFqn()));
emptyCollection.selector = bfd.singularData.getSingularizer().getEmptyMakerSelector(bfd.singularData.getTargetFqn());
ms.arguments = new Expression[] {new ConditionalExpression(ifNull, emptyCollection, tgt[1])};
}
ms.receiver = new SingleNameReference(BUILDER_VARIABLE_NAME, 0);
ms.selector = setterName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;

abstract class EclipseGuavaSingularizer extends EclipseSingularizer {
protected static final char[] OF = {'o', 'f'};
protected static final char[][] CGCC = {{'c', 'o', 'm'}, {'g', 'o', 'o', 'g', 'l', 'e'}, {'c', 'o', 'm', 'm', 'o', 'n'}, {'c', 'o', 'l', 'l', 'e', 'c', 't'}};

protected String getSimpleTargetTypeName(SingularData data) {
return GuavaTypeMap.getGuavaTypeName(data.getTargetFqn());
}
Expand All @@ -75,15 +78,23 @@ protected char[] getBuilderMethodName(SingularData data) {

protected char[][] makeGuavaTypeName(String simpleName, boolean addBuilder) {
char[][] tokenizedName = new char[addBuilder ? 6 : 5][];
tokenizedName[0] = new char[] {'c', 'o', 'm'};
tokenizedName[1] = new char[] {'g', 'o', 'o', 'g', 'l', 'e'};
tokenizedName[2] = new char[] {'c', 'o', 'm', 'm', 'o', 'n'};
tokenizedName[3] = new char[] {'c', 'o', 'l', 'l', 'e', 'c', 't'};
tokenizedName[0] = CGCC[0];
tokenizedName[1] = CGCC[1];
tokenizedName[2] = CGCC[2];
tokenizedName[3] = CGCC[3];
tokenizedName[4] = simpleName.toCharArray();
if (addBuilder) tokenizedName[5] = new char[] { 'B', 'u', 'i', 'l', 'd', 'e', 'r'};
return tokenizedName;
}

@Override protected char[] getEmptyMakerSelector(String targetFqn) {
return OF;
}

@Override protected char[][] getEmptyMakerReceiver(String targetFqn) {
return CGCC;
}

@Override public List<EclipseNode> generateFields(SingularData data, EclipseNode builderType) {
String simpleTypeName = getSimpleTargetTypeName(data);
char[][] tokenizedName = makeGuavaTypeName(simpleTypeName, true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2015 The Project Lombok Authors.
* Copyright (C) 2015-2019 The Project Lombok Authors.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -55,6 +55,16 @@ public class EclipseJavaUtilListSingularizer extends EclipseJavaUtilListSetSingu
return LombokImmutableList.of("java.util.List", "java.util.Collection", "java.lang.Iterable");
}

private static final char[] EMPTY_LIST = {'e', 'm', 'p', 't', 'y', 'L', 'i', 's', 't'};

@Override protected char[][] getEmptyMakerReceiver(String targetFqn) {
return JAVA_UTIL_COLLECTIONS;
}

@Override protected char[] getEmptyMakerSelector(String targetFqn) {
return EMPTY_LIST;
}

@Override public void appendBuildCode(SingularData data, EclipseNode builderType, List<Statement> statements, char[] targetVariableName, String builderVariable) {
if (useGuavaInstead(builderType)) {
guavaListSetSingularizer.appendBuildCode(data, builderType, statements, targetVariableName, builderVariable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,20 @@ public class EclipseJavaUtilMapSingularizer extends EclipseJavaUtilSingularizer
return LombokImmutableList.of("java.util.Map", "java.util.SortedMap", "java.util.NavigableMap");
}

private static final char[] EMPTY_SORTED_MAP = {'e', 'm', 'p', 't', 'y', 'S', 'o', 'r', 't', 'e', 'd', 'M', 'a', 'p'};
private static final char[] EMPTY_NAVIGABLE_MAP = {'e', 'm', 'p', 't', 'y', 'N', 'a', 'v', 'i', 'g', 'a', 'b', 'l', 'e', 'M', 'a', 'p'};
private static final char[] EMPTY_MAP = {'e', 'm', 'p', 't', 'y', 'M', 'a', 'p'};

@Override protected char[][] getEmptyMakerReceiver(String targetFqn) {
return JAVA_UTIL_COLLECTIONS;
}

@Override protected char[] getEmptyMakerSelector(String targetFqn) {
if (targetFqn.endsWith("SortedMap")) return EMPTY_SORTED_MAP;
if (targetFqn.endsWith("NavigableMap")) return EMPTY_NAVIGABLE_MAP;
return EMPTY_MAP;
}

@Override public List<char[]> listFieldsToBeGenerated(SingularData data, EclipseNode builderType) {
if (useGuavaInstead(builderType)) {
return guavaMapSingularizer.listFieldsToBeGenerated(data, builderType);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2015 The Project Lombok Authors.
* Copyright (C) 2015-2019 The Project Lombok Authors.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -37,6 +37,20 @@ public class EclipseJavaUtilSetSingularizer extends EclipseJavaUtilListSetSingul
return LombokImmutableList.of("java.util.Set", "java.util.SortedSet", "java.util.NavigableSet");
}

private static final char[] EMPTY_SORTED_SET = {'e', 'm', 'p', 't', 'y', 'S', 'o', 'r', 't', 'e', 'd', 'S', 'e', 't'};
private static final char[] EMPTY_NAVIGABLE_SET = {'e', 'm', 'p', 't', 'y', 'N', 'a', 'v', 'i', 'g', 'a', 'b', 'l', 'e', 'S', 'e', 't'};
private static final char[] EMPTY_SET = {'e', 'm', 'p', 't', 'y', 'S', 'e', 't'};

@Override protected char[][] getEmptyMakerReceiver(String targetFqn) {
return JAVA_UTIL_COLLECTIONS;
}

@Override protected char[] getEmptyMakerSelector(String targetFqn) {
if (targetFqn.endsWith("SortedSet")) return EMPTY_SORTED_SET;
if (targetFqn.endsWith("NavigableSet")) return EMPTY_NAVIGABLE_SET;
return EMPTY_SET;
}

@Override public void appendBuildCode(SingularData data, EclipseNode builderType, List<Statement> statements, char[] targetVariableName, String builderVariable) {
if (useGuavaInstead(builderType)) {
guavaListSetSingularizer.appendBuildCode(data, builderType, statements, targetVariableName, builderVariable);
Expand Down
7 changes: 4 additions & 3 deletions src/core/lombok/javac/handlers/HandleSuperBuilder.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2013-2018 The Project Lombok Authors.
* Copyright (C) 2013-2019 The Project Lombok Authors.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -725,8 +725,9 @@ private JCExpressionStatement createSetterCallWithInstanceValue(BuilderFieldData
arg = tgt[0];
} else {
JCExpression eqNull = maker.Binary(CTC_EQUAL, tgt[0], maker.Literal(CTC_BOT, null));
JCExpression emptyList = maker.Apply(List.<JCExpression>nil(), chainDots(type, "java", "util", "Collections", "emptyList"), List.<JCExpression>nil());
arg = maker.Conditional(eqNull, emptyList, tgt[1]);
String emptyMaker = bfd.singularData.getSingularizer().getEmptyMaker(bfd.singularData.getTargetFqn());
JCExpression emptyCollection = maker.Apply(List.<JCExpression>nil(), chainDots(type, emptyMaker.split("\\.")), List.<JCExpression>nil());
arg = maker.Conditional(eqNull, emptyCollection, tgt[1]);
}
JCMethodInvocation apply = maker.Apply(List.<JCExpression>nil(), maker.Select(maker.Ident(type.toName(BUILDER_VARIABLE_NAME)), bfd.name), List.of(arg));
JCExpressionStatement exec = maker.Exec(apply);
Expand Down
2 changes: 2 additions & 0 deletions src/core/lombok/javac/handlers/JavacSingularsRecipes.java
Original file line number Diff line number Diff line change
Expand Up @@ -437,5 +437,7 @@ protected JCExpression cloneParamType(int index, JavacTreeMaker maker, List<JCEx
protected abstract String getAddMethodName();

protected abstract int getTypeArgumentsCount();

protected abstract String getEmptyMaker(String target);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ protected String getSimpleTargetTypeName(SingularData data) {
return GuavaTypeMap.getGuavaTypeName(data.getTargetFqn());
}

@Override protected String getEmptyMaker(String target) {
return target + ".of";
}

protected String getBuilderMethodName(SingularData data) {
String simpleTypeName = getSimpleTargetTypeName(data);
if ("ImmutableSortedSet".equals(simpleTypeName) || "ImmutableSortedMap".equals(simpleTypeName)) return "naturalOrder";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ public class JavacJavaUtilListSingularizer extends JavacJavaUtilListSetSingulari
return LombokImmutableList.of("java.util.List", "java.util.Collection", "java.lang.Iterable");
}

@Override protected String getEmptyMaker(String target) {
return "java.util.Collections.emptyList";
}

@Override public void appendBuildCode(SingularData data, JavacNode builderType, JCTree source, ListBuffer<JCStatement> statements, Name targetVariableName, String builderVariable) {
JavacTreeMaker maker = builderType.getTreeMaker();
List<JCExpression> jceBlank = List.nil();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ public class JavacJavaUtilMapSingularizer extends JavacJavaUtilSingularizer {
return LombokImmutableList.of("java.util.Map", "java.util.SortedMap", "java.util.NavigableMap");
}

@Override protected String getEmptyMaker(String target) {
if (target.endsWith("NavigableMap")) return "java.util.Collections.emptyNavigableMap";
if (target.endsWith("SortedMap")) return "java.util.Collections.emptySortedMap";
return "java.util.Collections.emptyMap";
}

@Override protected JavacSingularizer getGuavaInstead(JavacNode node) {
return new JavacGuavaMapSingularizer();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ public class JavacJavaUtilSetSingularizer extends JavacJavaUtilListSetSingulariz
return LombokImmutableList.of("java.util.Set", "java.util.SortedSet", "java.util.NavigableSet");
}

@Override protected String getEmptyMaker(String target) {
if (target.endsWith("SortedSet")) return "java.util.Collections.emptySortedSet";
if (target.endsWith("NavigableSet")) return "java.util.Collections.emptyNavigableSet";
return "java.util.Collections.emptySet";
}

@Override public void appendBuildCode(SingularData data, JavacNode builderType, JCTree source, ListBuffer<JCStatement> statements, Name targetVariableName, String builderVariable) {
JavacTreeMaker maker = builderType.getTreeMaker();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import java.util.List;
import java.util.Map;
public class SuperBuilderWithGenericsAndToBuilder {
public static class Parent<A> {
A field1;
List<String> items;
Map<Integer, String> items;
@java.lang.SuppressWarnings("all")
public static abstract class ParentBuilder<A, C extends Parent<A>, B extends ParentBuilder<A, C, B>> {
@java.lang.SuppressWarnings("all")
private A field1;
@java.lang.SuppressWarnings("all")
private java.util.ArrayList<String> items;
private java.util.ArrayList<Integer> items$key;
@java.lang.SuppressWarnings("all")
private java.util.ArrayList<String> items$value;
@java.lang.SuppressWarnings("all")
protected B $fillValuesFrom(final C instance) {
ParentBuilder.$fillValuesFromInstanceIntoBuilder(instance, this);
Expand All @@ -17,7 +19,7 @@ public static abstract class ParentBuilder<A, C extends Parent<A>, B extends Par
@java.lang.SuppressWarnings("all")
private static <A> void $fillValuesFromInstanceIntoBuilder(final Parent<A> instance, final ParentBuilder<A, ?, ?> b) {
b.field1(instance.field1);
b.items(instance.items == null ? java.util.Collections.emptyList() : instance.items);
b.items(instance.items == null ? java.util.Collections.emptyMap() : instance.items);
}
@java.lang.SuppressWarnings("all")
protected abstract B self();
Expand All @@ -29,26 +31,39 @@ public B field1(final A field1) {
return self();
}
@java.lang.SuppressWarnings("all")
public B item(final String item) {
if (this.items == null) this.items = new java.util.ArrayList<String>();
this.items.add(item);
public B item(final Integer itemKey, final String itemValue) {
if (this.items$key == null) {
this.items$key = new java.util.ArrayList<Integer>();
this.items$value = new java.util.ArrayList<String>();
}
this.items$key.add(itemKey);
this.items$value.add(itemValue);
return self();
}
@java.lang.SuppressWarnings("all")
public B items(final java.util.Collection<? extends String> items) {
if (this.items == null) this.items = new java.util.ArrayList<String>();
this.items.addAll(items);
public B items(final java.util.Map<? extends Integer, ? extends String> items) {
if (this.items$key == null) {
this.items$key = new java.util.ArrayList<Integer>();
this.items$value = new java.util.ArrayList<String>();
}
for (final java.util.Map.Entry<? extends Integer, ? extends String> $lombokEntry : items.entrySet()) {
this.items$key.add($lombokEntry.getKey());
this.items$value.add($lombokEntry.getValue());
}
return self();
}
@java.lang.SuppressWarnings("all")
public B clearItems() {
if (this.items != null) this.items.clear();
if (this.items$key != null) {
this.items$key.clear();
this.items$value.clear();
}
return self();
}
@java.lang.Override
@java.lang.SuppressWarnings("all")
public java.lang.String toString() {
return "SuperBuilderWithGenericsAndToBuilder.Parent.ParentBuilder(field1=" + this.field1 + ", items=" + this.items + ")";
return "SuperBuilderWithGenericsAndToBuilder.Parent.ParentBuilder(field1=" + this.field1 + ", items$key=" + this.items$key + ", items$value=" + this.items$value + ")";
}
}
@java.lang.SuppressWarnings("all")
Expand All @@ -70,16 +85,18 @@ public Parent<A> build() {
@java.lang.SuppressWarnings("all")
protected Parent(final ParentBuilder<A, ?, ?> b) {
this.field1 = b.field1;
java.util.List<String> items;
switch (b.items == null ? 0 : b.items.size()) {
java.util.Map<Integer, String> items;
switch (b.items$key == null ? 0 : b.items$key.size()) {
case 0:
items = java.util.Collections.emptyList();
items = java.util.Collections.emptyMap();
break;
case 1:
items = java.util.Collections.singletonList(b.items.get(0));
items = java.util.Collections.singletonMap(b.items$key.get(0), b.items$value.get(0));
break;
default:
items = java.util.Collections.unmodifiableList(new java.util.ArrayList<String>(b.items));
items = new java.util.LinkedHashMap<Integer, String>(b.items$key.size() < 1073741824 ? 1 + b.items$key.size() + (b.items$key.size() - 3) / 3 : java.lang.Integer.MAX_VALUE);
for (int $i = 0; $i < b.items$key.size(); $i++) items.put(b.items$key.get($i), (String) b.items$value.get($i));
items = java.util.Collections.unmodifiableMap(items);
}
this.items = items;
}
Expand Down Expand Up @@ -157,6 +174,6 @@ protected Child(final ChildBuilder<A, ?, ?> b) {
}
}
public static void test() {
Child<Integer> x = Child.<Integer>builder().field3(0.0).field1(5).item("").build().toBuilder().build();
Child<Integer> x = Child.<Integer>builder().field3(0.0).field1(5).item(5, "").build().toBuilder().build();
}
}
Loading

0 comments on commit 2335f25

Please sign in to comment.