From 0b0656ae3c38ae915c86e17c2744d5e3d8fc805c Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Tue, 7 Jan 2020 23:29:04 +0100 Subject: [fixes #2323] javadoc `@return` generation updated. --- doc/changelog.markdown | 1 + src/core/lombok/javac/handlers/HandleBuilder.java | 2 +- src/core/lombok/javac/handlers/HandleSetter.java | 4 ++-- src/core/lombok/javac/handlers/JavacHandlerUtil.java | 20 ++++++++++++++++---- .../resource/after-delombok/BuilderJavadoc.java | 3 +++ .../after-delombok/BuilderWithDeprecated.java | 1 + .../resource/after-delombok/GetterSetterJavadoc.java | 2 +- .../after-delombok/SetterAndWithMethodJavadoc.java | 2 ++ .../after-delombok/WithMethodMarkedDeprecated.java | 1 + 9 files changed, 28 insertions(+), 8 deletions(-) diff --git a/doc/changelog.markdown b/doc/changelog.markdown index 40a0e120..988f1053 100644 --- a/doc/changelog.markdown +++ b/doc/changelog.markdown @@ -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) diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java index 11666048..1e8b8a22 100644 --- a/src/core/lombok/javac/handlers/HandleBuilder.java +++ b/src/core/lombok/javac/handlers/HandleBuilder.java @@ -819,7 +819,7 @@ public class HandleBuilder extends JavacAnnotationHandler { 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); } diff --git a/src/core/lombok/javac/handlers/HandleSetter.java b/src/core/lombok/javac/handlers/HandleSetter.java index 926e94d9..9c7ce042 100644 --- a/src/core/lombok/javac/handlers/HandleSetter.java +++ b/src/core/lombok/javac/handlers/HandleSetter.java @@ -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 @@ -279,7 +279,7 @@ public class HandleSetter extends JavacAnnotationHandler { 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; } } diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java index c8fe5278..b3a5cf90 100644 --- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java +++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java @@ -1962,7 +1962,7 @@ public class JavacHandlerUtil { }, WITH { @Override public String apply(final JCCompilationUnit cu, final JavacNode node) { - return applySetter(cu, node, "WITH|WITHER"); + return addReturnsUpdatedSelfIfNeeded(applySetter(cu, node, "WITH|WITHER")); } }; @@ -1992,6 +1992,9 @@ public class JavacHandlerUtil { } } + public static void copyJavadoc(JavacNode from, JCTree to, CopyJavadoc copyMode) { + copyJavadoc(from, to, copyMode, false); + } /** * Copies javadoc on one node to the other. * @@ -2001,12 +2004,15 @@ public class JavacHandlerUtil { * * 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) {} } @@ -2014,7 +2020,13 @@ public class JavacHandlerUtil { 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) { diff --git a/test/transform/resource/after-delombok/BuilderJavadoc.java b/test/transform/resource/after-delombok/BuilderJavadoc.java index e64bed5f..c99bfe90 100644 --- a/test/transform/resource/after-delombok/BuilderJavadoc.java +++ b/test/transform/resource/after-delombok/BuilderJavadoc.java @@ -52,6 +52,7 @@ class BuilderJavadoc { * 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 basic(final int basic) { @@ -61,6 +62,7 @@ class BuilderJavadoc { /** * 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 getsetwith(final int getsetwith) { @@ -108,6 +110,7 @@ class BuilderJavadoc { /** * 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 withGetsetwith(final int getsetwith) { diff --git a/test/transform/resource/after-delombok/BuilderWithDeprecated.java b/test/transform/resource/after-delombok/BuilderWithDeprecated.java index d61619ed..65f8e4e7 100644 --- a/test/transform/resource/after-delombok/BuilderWithDeprecated.java +++ b/test/transform/resource/after-delombok/BuilderWithDeprecated.java @@ -32,6 +32,7 @@ public class BuilderWithDeprecated { } /** * @deprecated since always + * @return {@code this}. */ @java.lang.Deprecated @java.lang.SuppressWarnings("all") diff --git a/test/transform/resource/after-delombok/GetterSetterJavadoc.java b/test/transform/resource/after-delombok/GetterSetterJavadoc.java index 019b3c37..ae662da7 100644 --- a/test/transform/resource/after-delombok/GetterSetterJavadoc.java +++ b/test/transform/resource/after-delombok/GetterSetterJavadoc.java @@ -116,7 +116,7 @@ class GetterSetterJavadoc4 { * Some text * * @param fieldName Hello, World5 - * @return this + * @return {@code this}. */ @java.lang.SuppressWarnings("all") public GetterSetterJavadoc4 fieldName(final int fieldName) { diff --git a/test/transform/resource/after-delombok/SetterAndWithMethodJavadoc.java b/test/transform/resource/after-delombok/SetterAndWithMethodJavadoc.java index 97524b9d..7831e42c 100644 --- a/test/transform/resource/after-delombok/SetterAndWithMethodJavadoc.java +++ b/test/transform/resource/after-delombok/SetterAndWithMethodJavadoc.java @@ -22,6 +22,7 @@ class SetterAndWithMethodJavadoc { /** * 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) { @@ -38,6 +39,7 @@ class SetterAndWithMethodJavadoc { /** * 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) { diff --git a/test/transform/resource/after-delombok/WithMethodMarkedDeprecated.java b/test/transform/resource/after-delombok/WithMethodMarkedDeprecated.java index 03b96d33..d81e76ed 100644 --- a/test/transform/resource/after-delombok/WithMethodMarkedDeprecated.java +++ b/test/transform/resource/after-delombok/WithMethodMarkedDeprecated.java @@ -14,6 +14,7 @@ class WithMethodMarkedDeprecated { } /** * @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") -- cgit