Skip to content

Commit

Permalink
[fixes #2323] javadoc @return generation updated.
Browse files Browse the repository at this point in the history
  • Loading branch information
rzwitserloot committed Jan 7, 2020
1 parent 3f620e3 commit 0b0656a
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 8 deletions.
1 change: 1 addition & 0 deletions doc/changelog.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Lombok Changelog
* FEATURE: You can now configure a builder's 'setter' prefixes via `@Builder(setterPrefix = "set")` for example. We discourage doing this, but if some library you use requires them, have at it. [Pull Request #2174](https://github.com/rzwitserloot/lombok/pull/2174], [Issue #1805](https://github.com/rzwitserloot/lombok/issues/1805).
* BUGFIX: Referring to an inner class inside the generics on a class marked with `@SuperBuilder` would cause the error `wrong number of type arguments; required 3` [Issue #2262](https://github.com/rzwitserloot/lombok/issues/2262); fixed by github user [`@Lekanich`](https://github.com/rzwitserloot/lombok/issues/2262) - thank you!
* BUGFIX: Some of the code generated by `@Builder` did not include `this.` prefixes when accessing fields. While semantically it didn't matter, if you use the 'add this prefix for field accesses' save action in eclipse, the save action would break. [Issue #2327](https://github.com/rzwitserloot/lombok/issues/2327)
* BUGFIX: When lombok copies javadoc from fields to relevant methods, it should generate an appropriate `@return this` line if lombok copies the javadoc to a generated setter that is chainable (returns itself). It didn't do that when generating the 'setters' in a `@Builder`. Lombok also didn't generate an appropriate `@return` item for `@With` methods. The javadoc has also been updated slightly (the `this` reference in the javadoc is now rendered in a code tag).[Issue #2323](https://github.com/rzwitserloot/lombok/issues/2323)
* IMPROBABLE BREAKING CHANGE: Lombok now generates qualified types (so, `Outer.Inner` instead of just `Inner`) in most type signatures that it generates; this should avoid exotic scenarios where the types lombok puts in signatures end up referring to unintended other types, which can occur if your class implements an interface that itself defines a type with the same name as one defined in your source file. I told you it was exotic. Thanks to Hunter Anderson for doing some preliminary work on this change. [Issue #2268](https://github.com/rzwitserloot/lombok/issues/2268)


Expand Down
2 changes: 1 addition & 1 deletion src/core/lombok/javac/handlers/HandleBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ private void makePrefixedSetterMethodForBuilder(CheckerFrameworkVersion cfv, Jav
newMethod.params = List.of(recv, newMethod.params.get(0));
}
recursiveSetGeneratedBy(newMethod, source.get(), builderType.getContext());
copyJavadoc(originalFieldNode, newMethod, CopyJavadoc.SETTER);
copyJavadoc(originalFieldNode, newMethod, CopyJavadoc.SETTER, true);

injectMethod(builderType, newMethod);
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/lombok/javac/handlers/HandleSetter.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2009-2019 The Project Lombok Authors.
* Copyright (C) 2009-2020 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 @@ -279,7 +279,7 @@ public static JCMethodDecl createSetter(long access, boolean deprecate, JavacNod

JCMethodDecl decl = recursiveSetGeneratedBy(treeMaker.MethodDef(treeMaker.Modifiers(access, annsOnMethod), methodName, methodType,
methodGenericParams, parameters, throwsClauses, methodBody, annotationMethodDefaultValue), source.get(), field.getContext());
copyJavadoc(field, decl, CopyJavadoc.SETTER);
copyJavadoc(field, decl, CopyJavadoc.SETTER, returnStatement != null);
return decl;
}
}
20 changes: 16 additions & 4 deletions src/core/lombok/javac/handlers/JavacHandlerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -1962,7 +1962,7 @@ public static enum CopyJavadoc {
},
WITH {
@Override public String apply(final JCCompilationUnit cu, final JavacNode node) {
return applySetter(cu, node, "WITH|WITHER");
return addReturnsUpdatedSelfIfNeeded(applySetter(cu, node, "WITH|WITHER"));
}
};

Expand Down Expand Up @@ -1992,6 +1992,9 @@ private static String applySetter(final JCCompilationUnit cu, JavacNode node, St
}
}

public static void copyJavadoc(JavacNode from, JCTree to, CopyJavadoc copyMode) {
copyJavadoc(from, to, copyMode, false);
}
/**
* Copies javadoc on one node to the other.
*
Expand All @@ -2001,20 +2004,29 @@ private static String applySetter(final JCCompilationUnit cu, JavacNode node, St
*
* in 'SETTER' mode, stripping works similarly to 'GETTER' mode, except {@code param} are copied and stripped from the original and {@code @return} are skipped.
*/
public static void copyJavadoc(JavacNode from, JCTree to, CopyJavadoc copyMode) {
public static void copyJavadoc(JavacNode from, JCTree to, CopyJavadoc copyMode, boolean forceAddReturn) {
if (copyMode == null) copyMode = CopyJavadoc.VERBATIM;
try {
JCCompilationUnit cu = ((JCCompilationUnit) from.top().get());
String newJavadoc = copyMode.apply(cu, from);
if (newJavadoc != null) Javac.setDocComment(cu, to, newJavadoc);
if (newJavadoc != null) {
if (forceAddReturn) newJavadoc = addReturnsThisIfNeeded(newJavadoc);
Javac.setDocComment(cu, to, newJavadoc);
}
} catch (Exception ignore) {}
}

private static final Pattern FIND_RETURN = Pattern.compile("^\\s*\\**\\s*@returns?\\s+.*$", Pattern.MULTILINE | Pattern.CASE_INSENSITIVE);
static String addReturnsThisIfNeeded(String in) {
if (FIND_RETURN.matcher(in).find()) return in;

return addJavadocLine(in, "@return this");
return addJavadocLine(in, "@return {@code this}.");
}

static String addReturnsUpdatedSelfIfNeeded(String in) {
if (FIND_RETURN.matcher(in).find()) return in;

return addJavadocLine(in, "@return a clone of this object, except with this updated property (returns {@code this} if an identical value is passed).");
}

static String addJavadocLine(String in, String line) {
Expand Down
3 changes: 3 additions & 0 deletions test/transform/resource/after-delombok/BuilderJavadoc.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public BuilderJavadocBuilder<T> predefWithJavadoc(final int x) {
* basic gets only a builder setter.
* @see #getsetwith
* @param tag is moved to the setter.
* @return {@code this}.
*/
@java.lang.SuppressWarnings("all")
public BuilderJavadoc.BuilderJavadocBuilder<T> basic(final int basic) {
Expand All @@ -61,6 +62,7 @@ public BuilderJavadoc.BuilderJavadocBuilder<T> basic(final int basic) {
/**
* getsetwith gets a builder setter, an instance getter and setter, and a wither.
* @param tag is moved to the setters and wither.
* @return {@code this}.
*/
@java.lang.SuppressWarnings("all")
public BuilderJavadoc.BuilderJavadocBuilder<T> getsetwith(final int getsetwith) {
Expand Down Expand Up @@ -108,6 +110,7 @@ public void setGetsetwith(final int getsetwith) {
/**
* getsetwith gets a builder setter, an instance getter and setter, and a wither.
* @param tag is moved to the setters and wither.
* @return a clone of this object, except with this updated property (returns {@code this} if an identical value is passed).
*/
@java.lang.SuppressWarnings("all")
public BuilderJavadoc<T> withGetsetwith(final int getsetwith) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public static class BuilderWithDeprecatedBuilder {
}
/**
* @deprecated since always
* @return {@code this}.
*/
@java.lang.Deprecated
@java.lang.SuppressWarnings("all")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public int fieldName() {
* Some text
*
* @param fieldName Hello, World5
* @return this
* @return {@code this}.
*/
@java.lang.SuppressWarnings("all")
public GetterSetterJavadoc4 fieldName(final int fieldName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public void setI(final int i) {
/**
* Some value.
* @param the new value
* @return a clone of this object, except with this updated property (returns {@code this} if an identical value is passed).
*/
@java.lang.SuppressWarnings("all")
public SetterAndWithMethodJavadoc withI(final int i) {
Expand All @@ -38,6 +39,7 @@ public void setJ(final int j) {
/**
* Reinstantiate with some other value.
* @param the other new other value
* @return a clone of this object, except with this updated property (returns {@code this} if an identical value is passed).
*/
@java.lang.SuppressWarnings("all")
public SetterAndWithMethodJavadoc withJ(final int j) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public WithMethodMarkedDeprecated withAnnotation(final int annotation) {
}
/**
* @deprecated
* @return a clone of this object, except with this updated property (returns {@code this} if an identical value is passed).
*/
@java.lang.Deprecated
@java.lang.SuppressWarnings("all")
Expand Down

0 comments on commit 0b0656a

Please sign in to comment.