Skip to content
This repository was archived by the owner on Oct 16, 2024. It is now read-only.

Commit 14373b3

Browse files
alicederynalicederyn
authored
Merge pull request #256 from google/invalid.code
Fix two instances of invalid code generation
2 parents c82cc39 + 53264af commit 14373b3

File tree

48 files changed

+567
-331
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+567
-331
lines changed

‎src/main/java/org/inferred/freebuilder/processor/BuildablePropertyFactory.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,14 @@
3030

3131
import com.google.common.annotations.VisibleForTesting;
3232
import com.google.common.base.Optional;
33-
import com.google.common.base.Preconditions;
3433
import com.google.common.base.Predicate;
3534
import com.google.common.collect.FluentIterable;
3635

3736
import org.inferred.freebuilder.processor.Metadata.Property;
3837
import org.inferred.freebuilder.processor.PropertyCodeGenerator.Config;
3938
import org.inferred.freebuilder.processor.util.Block;
4039
import org.inferred.freebuilder.processor.util.ParameterizedType;
40+
import org.inferred.freebuilder.processor.util.PreconditionExcerpts;
4141
import org.inferred.freebuilder.processor.util.SourceBuilder;
4242

4343
import java.util.List;
@@ -214,9 +214,9 @@ private void addSetter(SourceBuilder code, Metadata metadata) {
214214
setter(property),
215215
property.getType(),
216216
property.getName())
217+
.add(PreconditionExcerpts.checkNotNull(property.getName()))
217218
.addLine(" this.%s.clear();", property.getName())
218-
.addLine(" this.%1$s.mergeFrom(%2$s.checkNotNull(%1$s));",
219-
property.getName(), Preconditions.class)
219+
.addLine(" this.%s.mergeFrom(%s);", property.getName(), property.getName())
220220
.addLine(" return (%s) this;", metadata.getBuilder())
221221
.addLine("}");
222222
}

‎src/main/java/org/inferred/freebuilder/processor/SortedSetPropertyFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import static org.inferred.freebuilder.processor.util.feature.GuavaLibrary.GUAVA;
3535
import static org.inferred.freebuilder.processor.util.feature.SourceLevel.SOURCE_LEVEL;
3636
import static org.inferred.freebuilder.processor.util.feature.SourceLevel.diamondOperator;
37+
import static org.inferred.freebuilder.processor.util.feature.SourceLevel.nestedDiamondOperator;
3738

3839
import com.google.common.annotations.VisibleForTesting;
3940
import com.google.common.base.Optional;
@@ -454,7 +455,7 @@ public void addFinalFieldAssignment(SourceBuilder code, String finalField, Strin
454455
finalField,
455456
Collections.class,
456457
TreeSet.class,
457-
diamondOperator(elementType))
458+
nestedDiamondOperator(elementType))
458459
.addLine("} else {")
459460
.addLine(" %s = %s.unmodifiableSortedSet(new %s%s(%s.%s));",
460461
finalField,

‎src/main/java/org/inferred/freebuilder/processor/util/feature/SourceLevel.java

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
package org.inferred.freebuilder.processor.util.feature;
1717

1818
import com.google.common.base.Optional;
19+
import com.google.common.collect.ImmutableList;
1920

2021
import org.inferred.freebuilder.processor.util.Excerpt;
2122
import org.inferred.freebuilder.processor.util.QualifiedName;
2223
import org.inferred.freebuilder.processor.util.Shading;
2324
import org.inferred.freebuilder.processor.util.SourceBuilder;
2425

26+
import java.util.List;
27+
2528
import javax.annotation.processing.ProcessingEnvironment;
2629
import javax.lang.model.SourceVersion;
2730
import javax.lang.model.util.Elements;
@@ -36,7 +39,7 @@
3639
*/
3740
public enum SourceLevel implements Feature<SourceLevel> {
3841

39-
JAVA_6("Java 6"), JAVA_7("Java 7"), JAVA_8("Java 8+");
42+
JAVA_6("Java 6", 6), JAVA_7("Java 7", 7), JAVA_8("Java 8+", 8);
4043

4144
/**
4245
* Constant to pass to {@link SourceBuilder#feature(FeatureType)} to get the current
@@ -73,20 +76,35 @@ protected SourceLevel forEnvironment(ProcessingEnvironment env, FeatureSet featu
7376
private static final String ECLIPSE_DISPATCHER =
7477
Shading.unshadedName("org.eclipse.jdt.internal.compiler.apt.dispatch.RoundDispatcher");
7578

76-
public static Excerpt diamondOperator(final Object type) {
77-
return new DiamondOperator(type);
79+
/**
80+
* An excerpt that uses the diamond operator (&lt;&gt;) whenever available.
81+
*/
82+
public static Excerpt diamondOperator(Object type) {
83+
return new DiamondOperator("diamondOperator", type, JAVA_7);
84+
}
85+
86+
/**
87+
* An excerpt that uses the diamond operator (&lt;&gt;) whenever nested inference is available
88+
* (Java 8+).
89+
*/
90+
public static Excerpt nestedDiamondOperator(Object type) {
91+
return new DiamondOperator("nestedDiamondOperator", type, JAVA_8);
7892
}
7993

8094
private static final class DiamondOperator extends Excerpt {
95+
private final String methodName;
8196
private final Object type;
97+
private final SourceLevel minimumSourceLevel;
8298

83-
private DiamondOperator(Object type) {
99+
private DiamondOperator(String methodName, Object type, SourceLevel minimumSourceLevel) {
100+
this.methodName = methodName;
84101
this.type = type;
102+
this.minimumSourceLevel = minimumSourceLevel;
85103
}
86104

87105
@Override
88106
public void addTo(SourceBuilder source) {
89-
if (source.feature(SOURCE_LEVEL).compareTo(JAVA_7) >= 0) {
107+
if (source.feature(SOURCE_LEVEL).compareTo(minimumSourceLevel) >= 0) {
90108
source.add("<>");
91109
} else {
92110
source.add("<%s>", type);
@@ -95,19 +113,23 @@ public void addTo(SourceBuilder source) {
95113

96114
@Override
97115
public String toString() {
98-
return "diamondOperator(" + type + ")";
116+
return methodName + "(" + type + ")";
99117
}
100118

101119
@Override
102120
protected void addFields(FieldReceiver fields) {
121+
fields.add("methodName", methodName);
103122
fields.add("type", type);
123+
fields.add("minimumSourceLevel", minimumSourceLevel);
104124
}
105125
}
106126

107127
private final String humanReadableFormat;
128+
private final int version;
108129

109-
SourceLevel(String humanReadableFormat) {
130+
SourceLevel(String humanReadableFormat, int version) {
110131
this.humanReadableFormat = humanReadableFormat;
132+
this.version = version;
111133
}
112134

113135
public Optional<QualifiedName> safeVarargs() {
@@ -167,6 +189,10 @@ public Optional<QualifiedName> spliterator() {
167189
}
168190
}
169191

192+
public List<String> javacArguments() {
193+
return ImmutableList.of("-source", Integer.toString(version));
194+
}
195+
170196
@Override
171197
public String toString() {
172198
return humanReadableFormat;

‎src/test/java/org/inferred/freebuilder/processor/AbstractBuilderTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@
1616
package org.inferred.freebuilder.processor;
1717

1818
import org.inferred.freebuilder.FreeBuilder;
19+
import org.inferred.freebuilder.processor.util.feature.StaticFeatureSet;
1920
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
2021
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
2122
import org.junit.Test;
2223

2324
import javax.tools.JavaFileObject;
2425

2526
public class AbstractBuilderTest {
26-
private final BehaviorTester behaviorTester = BehaviorTester.create();
27+
private final BehaviorTester behaviorTester = BehaviorTester.create(new StaticFeatureSet());
2728

2829
private static final JavaFileObject TYPE_WITH_ABSTRACT_BUILDER = new SourceBuilder()
2930
.addLine("package com.example;")

‎src/test/java/org/inferred/freebuilder/processor/BuildableBeanPropertyTest.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
2626
import org.inferred.freebuilder.FreeBuilder;
2727
import org.inferred.freebuilder.processor.util.feature.FeatureSet;
28-
import org.inferred.freebuilder.processor.util.testing.BehaviorTestRunner.Shared;
2928
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
3029
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory;
30+
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory.Shared;
3131
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
3232
import org.inferred.freebuilder.processor.util.testing.TestBuilder;
3333
import org.junit.Rule;
@@ -40,6 +40,7 @@
4040
import org.junit.runners.Parameterized.UseParametersRunnerFactory;
4141

4242
import java.util.ArrayList;
43+
import java.util.Collections;
4344
import java.util.List;
4445

4546
import javax.tools.JavaFileObject;
@@ -118,7 +119,7 @@ public static List<FeatureSet> featureSets() {
118119
.addLine("@%s", FreeBuilder.class)
119120
.addLine("public interface DataType {")
120121
.addLine(" class Item {")
121-
.addLine(" public %s<String> getNames() {", ImmutableList.class)
122+
.addLine(" public %s<String> getNames() {", List.class)
122123
.addLine(" return names;")
123124
.addLine(" }")
124125
.addLine("")
@@ -158,10 +159,11 @@ public static List<FeatureSet> featureSets() {
158159
.addLine(" private Builder() {}")
159160
.addLine(" }")
160161
.addLine("")
161-
.addLine(" private final %s<String> names;", ImmutableList.class)
162+
.addLine(" private final %s<String> names;", List.class)
162163
.addLine("")
163-
.addLine(" private Item(Iterable<String> names) {")
164-
.addLine(" this.names = %s.copyOf(names);", ImmutableList.class)
164+
.addLine(" private Item(%s<String> names) {", List.class)
165+
.addLine(" this.names = %s.unmodifiableList(new %s<String>(names));",
166+
Collections.class, ArrayList.class)
165167
.addLine(" }")
166168
.addLine(" }")
167169
.addLine("")
@@ -209,13 +211,12 @@ public static List<FeatureSet> featureSets() {
209211
.addLine(" public DataType.Item build() { return new Value(this); }")
210212
.addLine(" public DataType.Item buildPartial() { return new Value(this); }")
211213
.addLine(" private class Value implements DataType.Item {")
212-
.addLine(" private %s<String> names;", ImmutableList.class)
214+
.addLine(" private %s<String> names;", List.class)
213215
.addLine(" Value(DataType_Item_Builder builder) {")
214-
.addLine(" names = %s.copyOf(builder.names);",
215-
ImmutableList.class)
216+
.addLine(" names = %s.unmodifiableList(new %s<String>(builder.names));",
217+
Collections.class, ArrayList.class)
216218
.addLine(" }")
217-
.addLine(" @%s public %s<String> getNames() { return names; }",
218-
Override.class, ImmutableList.class)
219+
.addLine(" @%s public %s<String> getNames() { return names; }", Override.class, List.class)
219220
.addLine(" }")
220221
.addLine("}")
221222
.build();

‎src/test/java/org/inferred/freebuilder/processor/BuildablePrefixlessPropertyTest.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@
2626
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
2727
import org.inferred.freebuilder.FreeBuilder;
2828
import org.inferred.freebuilder.processor.util.feature.FeatureSet;
29-
import org.inferred.freebuilder.processor.util.testing.BehaviorTestRunner.Shared;
3029
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
3130
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory;
31+
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory.Shared;
3232
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
3333
import org.inferred.freebuilder.processor.util.testing.TestBuilder;
3434
import org.junit.Rule;
@@ -41,6 +41,7 @@
4141
import org.junit.runners.Parameterized.UseParametersRunnerFactory;
4242

4343
import java.util.ArrayList;
44+
import java.util.Collections;
4445
import java.util.List;
4546

4647
import javax.tools.JavaFileObject;
@@ -119,7 +120,7 @@ public static List<FeatureSet> featureSets() {
119120
.addLine("@%s", FreeBuilder.class)
120121
.addLine("public interface DataType {")
121122
.addLine(" class Item {")
122-
.addLine(" public %s<String> names() {", ImmutableList.class)
123+
.addLine(" public %s<String> names() {", List.class)
123124
.addLine(" return names;")
124125
.addLine(" }")
125126
.addLine("")
@@ -159,10 +160,11 @@ public static List<FeatureSet> featureSets() {
159160
.addLine(" private Builder() {}")
160161
.addLine(" }")
161162
.addLine("")
162-
.addLine(" private final %s<String> names;", ImmutableList.class)
163+
.addLine(" private final %s<String> names;", List.class)
163164
.addLine("")
164-
.addLine(" private Item(Iterable<String> names) {")
165-
.addLine(" this.names = %s.copyOf(names);", ImmutableList.class)
165+
.addLine(" private Item(%s<String> names) {", List.class)
166+
.addLine(" this.names = %s.unmodifiableList(new %s<String>(names));",
167+
Collections.class, ArrayList.class)
166168
.addLine(" }")
167169
.addLine(" }")
168170
.addLine("")
@@ -210,13 +212,12 @@ public static List<FeatureSet> featureSets() {
210212
.addLine(" public DataType.Item build() { return new Value(this); }")
211213
.addLine(" public DataType.Item buildPartial() { return new Value(this); }")
212214
.addLine(" private class Value implements DataType.Item {")
213-
.addLine(" private %s<String> names;", ImmutableList.class)
215+
.addLine(" private %s<String> names;", List.class)
214216
.addLine(" Value(DataType_Item_Builder builder) {")
215-
.addLine(" names = %s.copyOf(builder.names);",
216-
ImmutableList.class)
217+
.addLine(" names = %s.unmodifiableList(new %s<String>(builder.names));",
218+
Collections.class, ArrayList.class)
217219
.addLine(" }")
218-
.addLine(" @%s public %s<String> names() { return names; }",
219-
Override.class, ImmutableList.class)
220+
.addLine(" @%s public %s<String> names() { return names; }", Override.class, List.class)
220221
.addLine(" }")
221222
.addLine("}")
222223
.build();

‎src/test/java/org/inferred/freebuilder/processor/DefaultMapperMethodTest.java

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,11 @@
1515
*/
1616
package org.inferred.freebuilder.processor;
1717

18-
import com.google.common.base.Preconditions;
19-
2018
import org.inferred.freebuilder.FreeBuilder;
2119
import org.inferred.freebuilder.processor.util.feature.FeatureSet;
22-
import org.inferred.freebuilder.processor.util.testing.BehaviorTestRunner.Shared;
2320
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
2421
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory;
22+
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory.Shared;
2523
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
2624
import org.inferred.freebuilder.processor.util.testing.TestBuilder;
2725
import org.junit.Rule;
@@ -142,8 +140,10 @@ public void mapDelegatesToSetterForValidationForRequiredProperty_bean() {
142140
.addLine("")
143141
.addLine(" public static class Builder extends DataType_Builder {")
144142
.addLine(" @Override public Builder setProperty(int property) {")
145-
.addLine(" %s.checkArgument(property >= 0, \"property must be non-negative\");",
146-
Preconditions.class)
143+
.addLine(" if (property < 0) {")
144+
.addLine(" throw new IllegalArgumentException(")
145+
.addLine(" \"property must be non-negative\");")
146+
.addLine(" }")
147147
.addLine(" return super.setProperty(property);")
148148
.addLine(" }")
149149
.addLine(" }")
@@ -175,8 +175,10 @@ public void mapDelegatesToSetterForValidationForDefaultProperty_bean() {
175175
.addLine(" }")
176176
.addLine("")
177177
.addLine(" @Override public Builder setProperty(int property) {")
178-
.addLine(" %s.checkArgument(property >= 0, \"property must be non-negative\");",
179-
Preconditions.class)
178+
.addLine(" if (property < 0) {")
179+
.addLine(" throw new IllegalArgumentException(")
180+
.addLine(" \"property must be non-negative\");")
181+
.addLine(" }")
180182
.addLine(" return super.setProperty(property);")
181183
.addLine(" }")
182184
.addLine(" }")
@@ -313,8 +315,10 @@ public void mapDelegatesToSetterForValidationForRequiredProperty_prefixless() {
313315
.addLine("")
314316
.addLine(" public static class Builder extends DataType_Builder {")
315317
.addLine(" @Override public Builder property(int property) {")
316-
.addLine(" %s.checkArgument(property >= 0, \"property must be non-negative\");",
317-
Preconditions.class)
318+
.addLine(" if (property < 0) {")
319+
.addLine(" throw new IllegalArgumentException(")
320+
.addLine(" \"property must be non-negative\");")
321+
.addLine(" }")
318322
.addLine(" return super.property(property);")
319323
.addLine(" }")
320324
.addLine(" }")
@@ -346,8 +350,10 @@ public void mapDelegatesToSetterForValidationForDefaultProperty_prefixless() {
346350
.addLine(" }")
347351
.addLine("")
348352
.addLine(" @Override public Builder property(int property) {")
349-
.addLine(" %s.checkArgument(property >= 0, \"property must be non-negative\");",
350-
Preconditions.class)
353+
.addLine(" if (property < 0) {")
354+
.addLine(" throw new IllegalArgumentException(")
355+
.addLine(" \"property must be non-negative\");")
356+
.addLine(" }")
351357
.addLine(" return super.property(property);")
352358
.addLine(" }")
353359
.addLine(" }")

‎src/test/java/org/inferred/freebuilder/processor/DefaultedBeanPropertiesTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020

2121
import org.inferred.freebuilder.FreeBuilder;
2222
import org.inferred.freebuilder.processor.util.feature.FeatureSet;
23-
import org.inferred.freebuilder.processor.util.testing.BehaviorTestRunner.Shared;
2423
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
2524
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory;
25+
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory.Shared;
2626
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
2727
import org.inferred.freebuilder.processor.util.testing.TestBuilder;
2828
import org.junit.Rule;

‎src/test/java/org/inferred/freebuilder/processor/DefaultedPrefixlessPropertiesTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020

2121
import org.inferred.freebuilder.FreeBuilder;
2222
import org.inferred.freebuilder.processor.util.feature.FeatureSet;
23-
import org.inferred.freebuilder.processor.util.testing.BehaviorTestRunner.Shared;
2423
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
2524
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory;
25+
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory.Shared;
2626
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
2727
import org.inferred.freebuilder.processor.util.testing.TestBuilder;
2828
import org.junit.Rule;

‎src/test/java/org/inferred/freebuilder/processor/GenericTypeTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717

1818
import org.inferred.freebuilder.FreeBuilder;
1919
import org.inferred.freebuilder.processor.util.feature.FeatureSet;
20-
import org.inferred.freebuilder.processor.util.testing.BehaviorTestRunner.Shared;
2120
import org.inferred.freebuilder.processor.util.testing.BehaviorTester;
2221
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory;
22+
import org.inferred.freebuilder.processor.util.testing.ParameterizedBehaviorTestFactory.Shared;
2323
import org.inferred.freebuilder.processor.util.testing.SourceBuilder;
2424
import org.inferred.freebuilder.processor.util.testing.TestBuilder;
2525
import org.junit.Test;

0 commit comments

Comments
 (0)