Skip to content

Commit

Permalink
[Fixes #2115] builder fields tracking a property that has a default s…
Browse files Browse the repository at this point in the history 
…et is now called `$value` in order to convey that you shouldnt manually mess with it.
  • Loading branch information
@rzwitserloot
rzwitserloot committed Jul 15, 2019
1 parent b3824c9 commit b439e4c
Show file tree
Hide file tree
Showing 23 changed files with 205 additions and 173 deletions.
3 changes: 1 addition & 2 deletions doc/changelog.markdown
View file
Expand Up @@ -11,8 +11,7 @@ Lombok Changelog
* BUGFIX: Javac would generate the wrong equals and hashCode if a type-use annotation was put on an array type field [Issue #2165](https://github.com/rzwitserloot/lombok/issues/2165)
* BUGFIX: Eclipse 2019-06 + JDK-12 compatibility + an `@Singular` builder entry would produce a cascade of error dialogs. [Issue #2169](https://github.com/rzwitserloot/lombok/issues/2169)
* IMPROBABLE BREAKING CHANGE: Stricter validation of configuration keys dealing with identifiers and types (`lombok.log.fieldName`, `lombok.fieldNameConstants.innerTypeName`, `lombok.copyableAnnotations`).
>>>>>>> customlog
* IMPROBABLE BREAKING CHANGE: The fields generated inside builders for fields with defaults (with `@Builder` on a class with fields marked `@Default`) now have `$value` as the name; direct manipulation of these fields is not advised because there is an associated `$set` variable that also needs to be taken into account. [Issue #2115](https://github.com/rzwitserloot/lombok/issues/2115)
### v1.18.8 (May 7th, 2019)
* FEATURE: You can now configure `@FieldNameConstants` to `CONSTANT_CASE` the generated constants, using a `lombok.config` option. See the [FieldNameConstants documentation](https://projectlombok.org/features/experimental/FieldNameConstants). [Issue #2092](https://github.com/rzwitserloot/lombok/issues/2092).
* FEATURE: You can now suppress generation of the `builder` method when using `@Builder`; usually because you're only interested in the `toBuilder` method. As a convenience we won't emit warnings about missing `@Builder.Default` annotations when you do this. [Issue #2046](https://github.com/rzwitserloot/lombok/issues/2046)
Expand Down
20 changes: 15 additions & 5 deletions src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
View file
Expand Up @@ -1816,12 +1816,12 @@ private static Annotation[] addAnnotation(ASTNode source, Annotation[] originalA
}

/**
* Generates a new statement that checks if the given variable is null, and if so, throws a specified exception with the
* Generates a new statement that checks if the given local variable is null, and if so, throws a specified exception with the
* variable name as message.
*
* @param exName The name of the exception to throw; normally {@code java.lang.NullPointerException}.
*/
public static Statement generateNullCheck(AbstractVariableDeclaration variable, EclipseNode sourceNode) {
public static Statement generateNullCheck(TypeReference type, char[] variable, EclipseNode sourceNode) {
NullCheckExceptionType exceptionType = sourceNode.getAst().readConfiguration(ConfigurationKeys.NON_NULL_EXCEPTION_TYPE);
if (exceptionType == null) exceptionType = NullCheckExceptionType.NULL_POINTER_EXCEPTION;

Expand All @@ -1830,11 +1830,11 @@ public static Statement generateNullCheck(AbstractVariableDeclaration variable,
int pS = source.sourceStart, pE = source.sourceEnd;
long p = (long)pS << 32 | pE;

if (isPrimitive(variable.type)) return null;
if (isPrimitive(type)) return null;
AllocationExpression exception = new AllocationExpression();
setGeneratedBy(exception, source);

SingleNameReference varName = new SingleNameReference(variable.name, p);
SingleNameReference varName = new SingleNameReference(variable, p);
setGeneratedBy(varName, source);
NullLiteral nullLiteral = new NullLiteral(pS, pE);
setGeneratedBy(nullLiteral, source);
Expand All @@ -1844,7 +1844,7 @@ public static Statement generateNullCheck(AbstractVariableDeclaration variable,
equalExpression.sourceStart = pS; equalExpression.statementEnd = equalExpression.sourceEnd = pE;
setGeneratedBy(equalExpression, source);

StringLiteral message = new StringLiteral(exceptionType.toExceptionMessage(new String(variable.name)).toCharArray(), pS, pE, 0);
StringLiteral message = new StringLiteral(exceptionType.toExceptionMessage(new String(variable)).toCharArray(), pS, pE, 0);
setGeneratedBy(message, source);

if (exceptionType == NullCheckExceptionType.ASSERTION) {
Expand Down Expand Up @@ -1875,6 +1875,16 @@ public static Statement generateNullCheck(AbstractVariableDeclaration variable,
return ifStatement;
}

/**
* Generates a new statement that checks if the given variable is null, and if so, throws a specified exception with the
* variable name as message.
*
* @param exName The name of the exception to throw; normally {@code java.lang.NullPointerException}.
*/
public static Statement generateNullCheck(AbstractVariableDeclaration variable, EclipseNode sourceNode) {
return generateNullCheck(variable.type, variable.name, sourceNode);
}

/**
* Create an annotation of the given name, and is marked as being generated by the given source.
*/
Expand Down
23 changes: 14 additions & 9 deletions src/core/lombok/eclipse/handlers/HandleBuilder.java
View file
Expand Up @@ -113,6 +113,7 @@ static class BuilderFieldData {
TypeReference type;
char[] rawName;
char[] name;
char[] builderFieldName;
char[] nameOfDefaultProvider;
char[] nameOfSetFlag;
SingularData singularData;
Expand Down Expand Up @@ -143,6 +144,7 @@ private static boolean equals(String a, char[][] b) {

private static final char[] DEFAULT_PREFIX = {'$', 'd', 'e', 'f', 'a', 'u', 'l', 't', '$'};
private static final char[] SET_PREFIX = {'$', 's', 'e', 't'};
private static final char[] VALUE_PREFIX = {'$', 'v', 'a', 'l', 'u', 'e'};

private static final char[] prefixWith(char[] prefix, char[] name) {
char[] out = new char[prefix.length + name.length];
Expand Down Expand Up @@ -229,6 +231,7 @@ private static final char[] prefixWith(char[] prefix, char[] name) {
BuilderFieldData bfd = new BuilderFieldData();
bfd.rawName = fieldNode.getName().toCharArray();
bfd.name = removePrefixFromField(fieldNode);
bfd.builderFieldName = bfd.name;
bfd.annotations = copyAnnotations(fd, copyableAnnotations);
bfd.type = fd.type;
bfd.singularData = getSingularData(fieldNode, ast);
Expand All @@ -253,6 +256,7 @@ private static final char[] prefixWith(char[] prefix, char[] name) {
if (isDefault != null) {
bfd.nameOfDefaultProvider = prefixWith(DEFAULT_PREFIX, bfd.name);
bfd.nameOfSetFlag = prefixWith(bfd.name, SET_PREFIX);
bfd.builderFieldName = prefixWith(bfd.name, VALUE_PREFIX);

MethodDeclaration md = generateDefaultProvider(bfd.nameOfDefaultProvider, td.typeParameters, fieldNode, ast);
if (md != null) injectMethod(tdParent, md);
Expand Down Expand Up @@ -410,6 +414,7 @@ private static final char[] prefixWith(char[] prefix, char[] name) {

bfd.rawName = arg.name;
bfd.name = arg.name;
bfd.builderFieldName = bfd.name;
bfd.annotations = copyAnnotations(arg, copyableAnnotations);
bfd.type = arg.type;
bfd.singularData = getSingularData(param, ast);
Expand Down Expand Up @@ -661,7 +666,7 @@ public MethodDeclaration generateBuildMethod(EclipseNode tdParent, boolean isSta

for (BuilderFieldData bfd : builderFields) {
if (bfd.singularData != null && bfd.singularData.getSingularizer() != null) {
bfd.singularData.getSingularizer().appendBuildCode(bfd.singularData, type, statements, bfd.name, "this");
bfd.singularData.getSingularizer().appendBuildCode(bfd.singularData, type, statements, bfd.builderFieldName, "this");
}
}

Expand All @@ -677,10 +682,10 @@ public MethodDeclaration generateBuildMethod(EclipseNode tdParent, boolean isSta

args.add(new ConditionalExpression(
new SingleNameReference(bfd.nameOfSetFlag, 0L),
new SingleNameReference(bfd.name, 0L),
new SingleNameReference(bfd.builderFieldName, 0L),
inv));
} else {
args.add(new SingleNameReference(bfd.name, 0L));
args.add(new SingleNameReference(bfd.builderFieldName, 0L));
}
}

Expand Down Expand Up @@ -781,12 +786,12 @@ public void generateBuilderFields(EclipseNode builderType, List<BuilderFieldData
EclipseNode field = null, setFlag = null;
for (EclipseNode exists : existing) {
char[] n = ((FieldDeclaration) exists.get()).name;
if (Arrays.equals(n, bfd.name)) field = exists;
if (Arrays.equals(n, bfd.builderFieldName)) field = exists;
if (bfd.nameOfSetFlag != null && Arrays.equals(n, bfd.nameOfSetFlag)) setFlag = exists;
}

if (field == null) {
FieldDeclaration fd = new FieldDeclaration(bfd.name, 0, 0);
FieldDeclaration fd = new FieldDeclaration(bfd.builderFieldName, 0, 0);
fd.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG;
fd.modifiers = ClassFileConstants.AccPrivate;
fd.type = copyType(bfd.type);
Expand All @@ -811,13 +816,13 @@ public void generateBuilderFields(EclipseNode builderType, List<BuilderFieldData
public void makeSetterMethodsForBuilder(EclipseNode builderType, BuilderFieldData bfd, EclipseNode sourceNode, boolean fluent, boolean chain, AccessLevel access, EclipseNode originalFieldNode) {
boolean deprecate = isFieldDeprecated(bfd.originalFieldNode);
if (bfd.singularData == null || bfd.singularData.getSingularizer() == null) {
makeSimpleSetterMethodForBuilder(builderType, deprecate, bfd.createdFields.get(0), bfd.nameOfSetFlag, sourceNode, fluent, chain, bfd.annotations, access, originalFieldNode);
makeSimpleSetterMethodForBuilder(builderType, deprecate, bfd.createdFields.get(0), bfd.name, bfd.nameOfSetFlag, sourceNode, fluent, chain, bfd.annotations, access, originalFieldNode);
} else {
bfd.singularData.getSingularizer().generateMethods(bfd.singularData, deprecate, builderType, fluent, chain, access);
}
}

private void makeSimpleSetterMethodForBuilder(EclipseNode builderType, boolean deprecate, EclipseNode fieldNode, char[] nameOfSetFlag, EclipseNode sourceNode, boolean fluent, boolean chain, Annotation[] annotations, AccessLevel access, EclipseNode originalFieldNode) {
private void makeSimpleSetterMethodForBuilder(EclipseNode builderType, boolean deprecate, EclipseNode fieldNode, char[] paramName, char[] nameOfSetFlag, EclipseNode sourceNode, boolean fluent, boolean chain, Annotation[] annotations, AccessLevel access, EclipseNode originalFieldNode) {
TypeDeclaration td = (TypeDeclaration) builderType.get();
AbstractMethodDeclaration[] existing = td.methods;
if (existing == null) existing = EMPTY;
Expand All @@ -831,12 +836,12 @@ private void makeSimpleSetterMethodForBuilder(EclipseNode builderType, boolean d
if (Arrays.equals(name, existingName) && !isTolerate(fieldNode, existing[i])) return;
}

String setterName = fluent ? fieldNode.getName() : HandlerUtil.buildAccessorName("set", fieldNode.getName());
String setterName = fluent ? new String(paramName) : HandlerUtil.buildAccessorName("set", new String(paramName));

List<Annotation> methodAnnsList = Collections.<Annotation>emptyList();
Annotation[] methodAnns = EclipseHandlerUtil.findCopyableToSetterAnnotations(originalFieldNode);
if (methodAnns != null && methodAnns.length > 0) methodAnnsList = Arrays.asList(methodAnns);
MethodDeclaration setter = HandleSetter.createSetter(td, deprecate, fieldNode, setterName, nameOfSetFlag, chain, toEclipseModifier(access),
MethodDeclaration setter = HandleSetter.createSetter(td, deprecate, fieldNode, setterName, paramName, nameOfSetFlag, chain, toEclipseModifier(access),
sourceNode, methodAnnsList, annotations != null ? Arrays.asList(copyAnnotations(sourceNode.get(), annotations)) : Collections.<Annotation>emptyList());
injectMethod(builderType, setter);
}
Expand Down
19 changes: 10 additions & 9 deletions src/core/lombok/eclipse/handlers/HandleSetter.java
View file
Expand Up @@ -184,11 +184,11 @@ public void createSetterForField(
}
}

MethodDeclaration method = createSetter((TypeDeclaration) fieldNode.up().get(), false, fieldNode, setterName, null, shouldReturnThis, modifier, sourceNode, onMethod, onParam);
MethodDeclaration method = createSetter((TypeDeclaration) fieldNode.up().get(), false, fieldNode, setterName, null, null, shouldReturnThis, modifier, sourceNode, onMethod, onParam);
injectMethod(fieldNode.up(), method);
}

static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate, EclipseNode fieldNode, String name, char[] booleanFieldToSet, boolean shouldReturnThis, int modifier, EclipseNode sourceNode, List<Annotation> onMethod, List<Annotation> onParam) {
static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate, EclipseNode fieldNode, String name, char[] paramName, char[] booleanFieldToSet, boolean shouldReturnThis, int modifier, EclipseNode sourceNode, List<Annotation> onMethod, List<Annotation> onParam) {
ASTNode source = sourceNode.get();
int pS = source.sourceStart, pE = source.sourceEnd;

Expand All @@ -200,14 +200,15 @@ static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate,
returnThis = new ReturnStatement(thisRef, pS, pE);
}

return createSetter(parent, deprecate, fieldNode, name, booleanFieldToSet, returnType, returnThis, modifier, sourceNode, onMethod, onParam);
return createSetter(parent, deprecate, fieldNode, name, paramName, booleanFieldToSet, returnType, returnThis, modifier, sourceNode, onMethod, onParam);
}

static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate, EclipseNode fieldNode, String name, char[] booleanFieldToSet, TypeReference returnType, Statement returnStatement, int modifier, EclipseNode sourceNode, List<Annotation> onMethod, List<Annotation> onParam) {
static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate, EclipseNode fieldNode, String name, char[] paramName, char[] booleanFieldToSet, TypeReference returnType, Statement returnStatement, int modifier, EclipseNode sourceNode, List<Annotation> onMethod, List<Annotation> onParam) {
FieldDeclaration field = (FieldDeclaration) fieldNode.get();
if (paramName == null) paramName = field.name;
ASTNode source = sourceNode.get();
int pS = source.sourceStart, pE = source.sourceEnd;
long p = (long)pS << 32 | pE;
long p = (long) pS << 32 | pE;
MethodDeclaration method = new MethodDeclaration(parent.compilationResult);
method.modifiers = modifier;
if (returnType != null) {
Expand All @@ -221,7 +222,7 @@ static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate,
deprecated = new Annotation[] { generateDeprecatedAnnotation(source) };
}
method.annotations = mergeAnnotations(copyAnnotations(source, onMethod.toArray(new Annotation[0]), deprecated), findCopyableToSetterAnnotations(fieldNode));
Argument param = new Argument(field.name, p, copyType(field.type, source), Modifier.FINAL);
Argument param = new Argument(paramName, p, copyType(field.type, source), Modifier.FINAL);
param.sourceStart = pS; param.sourceEnd = pE;
method.arguments = new Argument[] { param };
method.selector = name.toCharArray();
Expand All @@ -230,8 +231,8 @@ static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate,
method.typeParameters = null;
method.bits |= ECLIPSE_DO_NOT_TOUCH_FLAG;
Expression fieldRef = createFieldAccessor(fieldNode, FieldAccess.ALWAYS_FIELD, source);
NameReference fieldNameRef = new SingleNameReference(field.name, p);
Assignment assignment = new Assignment(fieldRef, fieldNameRef, (int)p);
NameReference fieldNameRef = new SingleNameReference(paramName, p);
Assignment assignment = new Assignment(fieldRef, fieldNameRef, (int) p);
assignment.sourceStart = pS; assignment.sourceEnd = assignment.statementEnd = pE;
method.bodyStart = method.declarationSourceStart = method.sourceStart = source.sourceStart;
method.bodyEnd = method.declarationSourceEnd = method.sourceEnd = source.sourceEnd;
Expand All @@ -241,7 +242,7 @@ static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate,
if (!hasNonNullAnnotations(fieldNode) && !hasNonNullAnnotations(fieldNode, onParam)) {
statements.add(assignment);
} else {
Statement nullCheck = generateNullCheck(field, sourceNode);
Statement nullCheck = generateNullCheck(field.type, paramName, sourceNode);
if (nullCheck != null) statements.add(nullCheck);
statements.add(assignment);
}
Expand Down

0 comments on commit b439e4c

Please sign in to comment.