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

Commit 610b610

Browse files
alicederynalicederyn
authored
#400 Add some special handling for arrays
2 parents a0024ef + 2c2eb65 commit 610b610

File tree

10 files changed

+334
-101
lines changed

10 files changed

+334
-101
lines changed

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

+5
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,11 @@ private class ConfigImpl implements Config {
484484
this.methodsInvokedInBuilderConstructor = methodsInvokedInBuilderConstructor;
485485
}
486486

487+
@Override
488+
public ExecutableElement getSourceElement() {
489+
return getterMethod;
490+
}
491+
487492
@Override
488493
public DeclaredType getBuilder() {
489494
return builder;

‎src/main/java/org/inferred/freebuilder/processor/property/DefaultProperty.java

+38
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,13 @@
3232
import org.inferred.freebuilder.processor.source.SourceBuilder;
3333
import org.inferred.freebuilder.processor.source.Variable;
3434

35+
import java.util.Arrays;
3536
import java.util.Objects;
3637
import java.util.Optional;
3738

39+
import javax.lang.model.element.Element;
3840
import javax.lang.model.type.TypeKind;
41+
import javax.tools.Diagnostic.Kind;
3942

4043
/** Default {@link PropertyCodeGenerator}, providing reference semantics for any type. */
4144
public class DefaultProperty extends PropertyCodeGenerator {
@@ -47,6 +50,7 @@ public Optional<DefaultProperty> create(Config config) {
4750
Property property = config.getProperty();
4851
boolean hasDefault = config.getMethodsInvokedInBuilderConstructor()
4952
.contains(setter(property));
53+
issueMutabilityWarning(config);
5054
FunctionalType mapperType = functionalTypeAcceptedByMethod(
5155
config.getBuilder(),
5256
mapper(property),
@@ -56,6 +60,31 @@ public Optional<DefaultProperty> create(Config config) {
5660
return Optional.of(new DefaultProperty(
5761
config.getDatatype(), property, hasDefault, mapperType));
5862
}
63+
64+
private static void issueMutabilityWarning(Config config) {
65+
TypeKind kind = config.getProperty().getType().getKind();
66+
if (kind == TypeKind.ARRAY && !mutableWarningsSuppressed(config.getSourceElement())) {
67+
config.getEnvironment().getMessager().printMessage(
68+
Kind.WARNING,
69+
"This property returns a mutable array that can be modified by the caller. "
70+
+ "FreeBuilder will use reference equality for this property. If possible, prefer "
71+
+ "an immutable type like List. You can suppress this warning with "
72+
+ "@SuppressWarnings(\"mutable\").",
73+
config.getSourceElement());
74+
}
75+
}
76+
77+
private static boolean mutableWarningsSuppressed(Element element) {
78+
SuppressWarnings suppressed = element.getAnnotation(SuppressWarnings.class);
79+
if (suppressed != null && Arrays.asList(suppressed.value()).contains("mutable")) {
80+
return true;
81+
}
82+
Element parent = element.getEnclosingElement();
83+
if (parent != null) {
84+
return mutableWarningsSuppressed(parent);
85+
}
86+
return false;
87+
}
5988
}
6089

6190
public static final FieldAccess UNSET_PROPERTIES = new FieldAccess("_unsetProperties");
@@ -264,4 +293,13 @@ public void addClearField(SourceBuilder code) {
264293
code.addLine("%s = %s;", property.getField(), property.getField().on(defaults.get()));
265294
}
266295
}
296+
297+
@Override
298+
public void addToStringValue(SourceBuilder code) {
299+
if (kind == TypeKind.ARRAY) {
300+
code.add("%s.toString(%s)", Arrays.class, property.getField());
301+
} else {
302+
code.add(property.getField());
303+
}
304+
}
267305
}

‎src/main/java/org/inferred/freebuilder/processor/property/PropertyCodeGenerator.java

+4
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
import javax.annotation.processing.ProcessingEnvironment;
3838
import javax.lang.model.element.AnnotationMirror;
39+
import javax.lang.model.element.ExecutableElement;
3940
import javax.lang.model.type.DeclaredType;
4041
import javax.lang.model.util.Elements;
4142
import javax.lang.model.util.Types;
@@ -45,6 +46,9 @@ public abstract class PropertyCodeGenerator {
4546

4647
/** Data available to {@link Factory} instances when creating a {@link PropertyCodeGenerator}. */
4748
public interface Config {
49+
/** Returns the element this property was inferred from. */
50+
ExecutableElement getSourceElement();
51+
4852
/** Returns datatype about the builder being generated. */
4953
Datatype getDatatype();
5054

‎src/main/java/org/inferred/freebuilder/processor/source/ObjectsExcerpts.java

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ private static Excerpt equalsExcerpt(boolean areEqual, Object a, Object b, TypeK
4242
case INT:
4343
case LONG:
4444
case CHAR:
45+
case ARRAY:
4546
return code -> code.add("%s %s %s", a, areEqual ? "==" : "!=", b);
4647

4748
default:

‎src/test/java/org/inferred/freebuilder/processor/property/ArrayPropertiesTest.java

+137
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package org.inferred.freebuilder.processor.property;
2+
3+
import com.google.common.testing.EqualsTester;
4+
5+
import org.inferred.freebuilder.FreeBuilder;
6+
import org.inferred.freebuilder.processor.Processor;
7+
import org.inferred.freebuilder.processor.source.SourceBuilder;
8+
import org.inferred.freebuilder.processor.source.feature.FeatureSet;
9+
import org.inferred.freebuilder.processor.source.feature.StaticFeatureSet;
10+
import org.inferred.freebuilder.processor.source.testing.BehaviorTester;
11+
import org.inferred.freebuilder.processor.source.testing.TestBuilder;
12+
import org.junit.Rule;
13+
import org.junit.Test;
14+
import org.junit.rules.ExpectedException;
15+
16+
public class ArrayPropertiesTest {
17+
18+
@Rule public final ExpectedException thrown = ExpectedException.none();
19+
20+
private final FeatureSet features = new StaticFeatureSet();
21+
private final BehaviorTester behaviorTester = BehaviorTester.create(features);
22+
23+
@Test
24+
public void testToString() {
25+
behaviorTester
26+
.with(new Processor(features))
27+
.with(SourceBuilder.forTesting()
28+
.addLine("package com.example;")
29+
.addLine("@%s", FreeBuilder.class)
30+
.addLine("public interface DataType {")
31+
.addLine(" int[] ints();")
32+
.addLine(" String[] strings();")
33+
.addLine("")
34+
.addLine(" class Builder extends DataType_Builder {}")
35+
.addLine("}"))
36+
.with(testBuilder()
37+
.addLine("DataType value = new DataType.Builder()")
38+
.addLine(" .ints(new int[] { 1, 2, 3 })")
39+
.addLine(" .strings(new String[] { \"a\", \"b\", \"c\" })")
40+
.addLine(" .build();")
41+
.addLine("assertThat(value.toString()).isEqualTo(")
42+
.addLine(" \"DataType{ints=[1, 2, 3], strings=[a, b, c]}\");")
43+
.build())
44+
.runTest();
45+
}
46+
47+
@Test
48+
public void testEquality() {
49+
behaviorTester
50+
.with(new Processor(features))
51+
.with(SourceBuilder.forTesting()
52+
.addLine("package com.example;")
53+
.addLine("@%s", FreeBuilder.class)
54+
.addLine("public interface DataType {")
55+
.addLine(" int[] ints();")
56+
.addLine("")
57+
.addLine(" class Builder extends DataType_Builder {}")
58+
.addLine("}"))
59+
.with(testBuilder()
60+
.addLine("int[] a = { 1, 2, 3 };")
61+
.addLine("int[] b = { 1, 2, 3 };")
62+
.addLine("int[] c = new int[0];")
63+
.addLine("new %s()", EqualsTester.class)
64+
.addLine(" .addEqualityGroup(")
65+
.addLine(" new DataType.Builder().ints(a).build(),")
66+
.addLine(" new DataType.Builder().ints(a).build())")
67+
.addLine(" .addEqualityGroup(")
68+
.addLine(" new DataType.Builder().ints(b).build(),")
69+
.addLine(" new DataType.Builder().ints(b).build())")
70+
.addLine(" .addEqualityGroup(")
71+
.addLine(" new DataType.Builder().ints(c).build(),")
72+
.addLine(" new DataType.Builder().ints(c).build())")
73+
.addLine(" .testEquals();")
74+
.build())
75+
.runTest();
76+
}
77+
78+
@Test
79+
public void issuesMutabilityWarning() {
80+
behaviorTester
81+
.with(new Processor(features))
82+
.with(SourceBuilder.forTesting()
83+
.addLine("package com.example;")
84+
.addLine("@%s", FreeBuilder.class)
85+
.addLine("public interface DataType {")
86+
.addLine(" int[] ints();")
87+
.addLine("")
88+
.addLine(" class Builder extends DataType_Builder {}")
89+
.addLine("}"))
90+
.compiles()
91+
.withWarningThat(warning -> warning
92+
.hasMessage("This property returns a mutable array that can be modified by the caller. "
93+
+ "FreeBuilder will use reference equality for this property. If possible, prefer "
94+
+ "an immutable type like List. You can suppress this warning with "
95+
+ "@SuppressWarnings(\"mutable\").")
96+
.inFile("/com/example/DataType.java")
97+
.onLine(7));
98+
}
99+
100+
@Test
101+
public void canSuppressMutabilityWarningOnClass() {
102+
behaviorTester
103+
.with(new Processor(features))
104+
.with(SourceBuilder.forTesting()
105+
.addLine("package com.example;")
106+
.addLine("@%s", FreeBuilder.class)
107+
.addLine("@SuppressWarnings(\"mutable\")")
108+
.addLine("public interface DataType {")
109+
.addLine(" int[] ints();")
110+
.addLine("")
111+
.addLine(" class Builder extends DataType_Builder {}")
112+
.addLine("}"))
113+
.compiles()
114+
.withNoWarnings();
115+
}
116+
117+
@Test
118+
public void canSuppressMutabilityWarningOnMethod() {
119+
behaviorTester
120+
.with(new Processor(features))
121+
.with(SourceBuilder.forTesting()
122+
.addLine("package com.example;")
123+
.addLine("@%s", FreeBuilder.class)
124+
.addLine("public interface DataType {")
125+
.addLine(" @SuppressWarnings(\"mutable\")")
126+
.addLine(" int[] ints();")
127+
.addLine("")
128+
.addLine(" class Builder extends DataType_Builder {}")
129+
.addLine("}"))
130+
.compiles()
131+
.withNoWarnings();
132+
}
133+
134+
private static TestBuilder testBuilder() {
135+
return new TestBuilder().addImport("com.example.DataType");
136+
}
137+
}

‎src/test/java/org/inferred/freebuilder/processor/source/testing/BehaviorTester.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,11 @@ public interface CompilationSubject {
157157
*/
158158
CompilationSubject withNoWarnings();
159159

160+
/**
161+
* Fails if the compiler did not issue a warning matching {@code diagnosticAssertions}.
162+
*/
163+
CompilationSubject withWarningThat(Consumer<DiagnosticSubject> diagnosticAssertions);
164+
160165
/**
161166
* Loads and tests all test sources.
162167
*
@@ -185,7 +190,7 @@ public interface CompilationFailureSubject {
185190

186191
public interface DiagnosticSubject {
187192
DiagnosticSubject hasMessage(CharSequence expected);
188-
DiagnosticSubject inFile(CharSequence expected);
193+
DiagnosticSubject inFile(String expected);
189194
DiagnosticSubject onLine(long line);
190195
}
191196
}

‎src/test/java/org/inferred/freebuilder/processor/source/testing/CompilationFailureSubjectImpl.java

-99
This file was deleted.

0 commit comments

Comments
 (0)