diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParameters.java b/src/main/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParameters.java index d076b3a643..1758148c21 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParameters.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParameters.java @@ -78,52 +78,63 @@ public Javadoc visitDocComment(Javadoc.DocComment javadoc, ExecutionContext ctx) List newBody = new ArrayList<>(javadoc.getBody().size()); boolean useNewBody = false; - List body = javadoc.getBody(); + List body = new ArrayList<>(javadoc.getBody()); + // We add a trailing element, to fix elements on the first line without space + // We can use null since this element is never going to be read, and nulls get filtered later on. + body.add(0, null); + for (int i = 0; i < body.size(); i++) { // JavaDocs require a look ahead, because the current element may be an element that exists on the same line as a parameter. // I.E. the space that precedes `* @param` will be a `Javadoc.Text` and needs to be removed along with the empty `@param`. // A `Javadoc` will always precede a parameter even if there is empty space like `*@param`. Javadoc currentDoc = body.get(i); - boolean skipCurrentDoc = false; if (i + 1 < body.size()) { Javadoc nextDoc = body.get(i + 1); if (nextDoc instanceof Javadoc.Parameter) { - Javadoc.Parameter parameter = (Javadoc.Parameter) visitParameter((Javadoc.Parameter) nextDoc, ctx); - if (parameter == null) { + Javadoc.Parameter nextParameter = (Javadoc.Parameter) nextDoc; + if (isEmptyParameter(nextParameter)) { // The `@param` being removed is the last item in the JavaDoc body, and contains // relevant whitespace via the JavaDoc.LineBreak. if (i + 1 == body.size() - 1) { - newBody.add(((Javadoc.Parameter) body.get(i + 1)).getDescription().get(0)); + // If we have a previous LineBreak we need to remove it before adding the new one + if (!newBody.isEmpty() && newBody.get(newBody.size() - 1) instanceof Javadoc.LineBreak) { + newBody.remove(newBody.size() - 1); + } + if (!nextParameter.getDescription().isEmpty()) { + newBody.add(nextParameter.getDescription().get(0)); + } } // No need to reprocess the next element. i += 1; useNewBody = true; - skipCurrentDoc = true; + currentDoc = null; } } else if (nextDoc instanceof Javadoc.Return) { - Javadoc.Return aReturn = (Javadoc.Return) visitReturn((Javadoc.Return) nextDoc, ctx); - if (aReturn == null) { - if (!newBody.isEmpty() && newBody.get(newBody.size() - 1) instanceof Javadoc.LineBreak) { - newBody.remove(newBody.size() - 1); - } - + Javadoc.Return nextReturn = (Javadoc.Return) nextDoc; + if (isEmptyReturn(nextReturn)) { // The `@return` being removed is the last item in the JavaDoc body, and contains // relevant whitespace via the JavaDoc.LineBreak. if (i + 1 == body.size() - 1) { - newBody.add(((Javadoc.Return) body.get(i + 1)).getDescription().get(0)); + // If we have a previous LineBreak we need to remove it before adding the new one + if (!newBody.isEmpty() && newBody.get(newBody.size() - 1) instanceof Javadoc.LineBreak) { + newBody.remove(newBody.size() - 1); + } + if (!nextReturn.getDescription().isEmpty()) { + newBody.add(nextReturn.getDescription().get(0)); + } } // No need to reprocess the next element. i += 1; useNewBody = true; - skipCurrentDoc = true; + currentDoc = null; } } else if (nextDoc instanceof Javadoc.Erroneous) { - Javadoc.Erroneous erroneous = (Javadoc.Erroneous) visitErroneous((Javadoc.Erroneous) nextDoc, ctx); - if (erroneous == null) { + Javadoc.Erroneous nextErroneous = (Javadoc.Erroneous) nextDoc; + if (isEmptyErroneous(nextErroneous)) { if (!newBody.isEmpty() && newBody.get(newBody.size() - 1) instanceof Javadoc.LineBreak) { newBody.remove(newBody.size() - 1); } @@ -132,12 +143,12 @@ public Javadoc visitDocComment(Javadoc.DocComment javadoc, ExecutionContext ctx) i += 1; useNewBody = true; - skipCurrentDoc = true; + currentDoc = null; } } } - if (!skipCurrentDoc) { + if (currentDoc != null) { newBody.add(currentDoc); } } @@ -145,35 +156,23 @@ public Javadoc visitDocComment(Javadoc.DocComment javadoc, ExecutionContext ctx) if (useNewBody) { javadoc = javadoc.withBody(newBody); } - return super.visitDocComment(javadoc, ctx); + // No need to call super visitor, already covered all cases by adding an empty first element when needed. + return javadoc; } - @Override - public Javadoc visitParameter(Javadoc.Parameter parameter, ExecutionContext ctx) { - if (parameter.getDescription().stream().allMatch(it -> it instanceof Javadoc.LineBreak)) { - return null; - } - return super.visitParameter(parameter, ctx); + public boolean isEmptyParameter(Javadoc.Parameter parameter) { + return parameter.getDescription().stream().allMatch(it -> it instanceof Javadoc.LineBreak); } - @Override - public Javadoc visitReturn(Javadoc.Return aReturn, ExecutionContext ctx) { - if (aReturn.getDescription().stream().allMatch(it -> it instanceof Javadoc.LineBreak)) { - return null; - } - return super.visitReturn(aReturn, ctx); + public boolean isEmptyReturn(Javadoc.Return aReturn) { + return aReturn.getDescription().stream().allMatch(it -> it instanceof Javadoc.LineBreak); } - @Override - public Javadoc visitErroneous(Javadoc.Erroneous erroneous, ExecutionContext ctx) { - if (erroneous.getText().size() == 1 && erroneous.getText().get(0) instanceof Javadoc.Text) { - Javadoc.Text text = (Javadoc.Text) erroneous.getText().get(0); - // Empty throws result in an Erroneous type. - if ("@throws".equals(text.getText())) { - return null; - } - } - return super.visitErroneous(erroneous, ctx); + public boolean isEmptyErroneous(Javadoc.Erroneous erroneous) { + // Empty throws result in an Erroneous type. + return erroneous.getText().size() == 1 && + erroneous.getText().get(0) instanceof Javadoc.Text && + "@throws".equals(((Javadoc.Text) erroneous.getText().get(0)).getText()); } } }; diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParametersTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParametersTest.java index c10a9d559c..b2b66d4d72 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParametersTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveEmptyJavaDocParametersTest.java @@ -16,6 +16,7 @@ package org.openrewrite.staticanalysis; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -35,20 +36,23 @@ public void defaults(RecipeSpec spec) { @DocumentExample @Test - void singleLineParam() { + void emptyParam() { rewriteRun( //language=java java( """ class Test { - /**@param arg0*/ + /** + * @param arg0 + */ void method(int arg0) { } } """, """ class Test { - /***/ + /** + */ void method(int arg0) { } } @@ -58,28 +62,42 @@ void method(int arg0) { } @Test - @Issue("https://github.com/openrewrite/rewrite/issues/3078") - void visitingQuarkMustNotFail() { + void emptyReturn() { rewriteRun( - other( + //language=java + java( """ - foo + class Test { + /** + * @return + */ + int method() { + } + } + """, + """ + class Test { + /** + */ + int method() { + } + } """ ) ); } @Test - void removeParamWithNoPrefix() { + void emptyThrows() { rewriteRun( //language=java java( """ class Test { /** - *@param arg0 + * @throws */ - void method(int arg0) { + void method() throws IllegalStateException { } } """, @@ -87,7 +105,7 @@ void method(int arg0) { class Test { /** */ - void method(int arg0) { + void method() throws IllegalStateException { } } """ @@ -156,76 +174,442 @@ void method(int arg0, int arg1) { ); } - @Test - void emptyReturn() { - rewriteRun( - //language=java - java( - """ - class Test { - /** - * @return - */ - int method() { + @Nested + class NoSpace { + @Test + void emptyParamNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /** + *@param arg0 + */ + void method(int arg0) { + } } - } - """, - """ - class Test { - /** - */ - int method() { + """, + """ + class Test { + /** + */ + void method(int arg0) { + } } - } - """ - ) - ); + """ + ) + ); + } + + @Test + void emptyReturnNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /** + *@return + */ + int method() { + } + } + """, + """ + class Test { + /** + */ + int method() { + } + } + """ + ) + ); + } + + @Test + void emptyThrowsNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /** + *@throws + */ + void method() throws IllegalStateException { + } + } + """, + """ + class Test { + /** + */ + void method() throws IllegalStateException { + } + } + """ + ) + ); + } + } - @Test - void emptyThrows() { - rewriteRun( - //language=java - java( - """ - class Test { - /** - * @throws - */ - void method() throws IllegalStateException { + @Nested + class SingleLine { + @Test + void singleLineParam() { + rewriteRun( + //language=java + java( + """ + class Test { + /** @param arg0*/ + void method(int arg0) { + } } - } - """, - """ - class Test { - /** - */ - void method() throws IllegalStateException { + """, + """ + class Test { + /***/ + void method(int arg0) { + } } - } - """ - ) - ); + """ + ) + ); + } + + @Test + void singleLineReturn() { + rewriteRun( + //language=java + java( + """ + class Test { + /** @return*/ + int method() { + } + } + """, + """ + class Test { + /***/ + int method() { + } + } + """ + ) + ); + } + + @Test + void singleLineThrows() { + rewriteRun( + //language=java + java( + """ + class Test { + /** @throws*/ + void method() throws IllegalStateException { + } + } + """, + """ + class Test { + /***/ + void method() throws IllegalStateException { + } + } + """ + ) + ); + } + + @Test + void singleLineParamNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /**@param arg0*/ + void method(int arg0) { + } + } + """, + """ + class Test { + /***/ + void method(int arg0) { + } + } + """ + ) + ); + } + + @Test + void singleLineReturnNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /**@return*/ + int method() { + } + } + """, + """ + class Test { + /***/ + int method() { + } + } + """ + ) + ); + } + + @Test + void singleLineThrowsNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /**@throws*/ + void method() throws IllegalStateException { + } + } + """, + """ + class Test { + /***/ + void method() throws IllegalStateException { + } + } + """ + ) + ); + } } + @Nested + class FirstLine { + @Test + void firstLineParam() { + rewriteRun( + //language=java + java( + """ + class Test { + /** @param arg0 + */ + void method(int arg0) { + } + } + """, + """ + class Test { + /** + */ + void method(int arg0) { + } + } + """ + ) + ); + } + + @Test + void firstLineReturn() { + rewriteRun( + //language=java + java( + """ + class Test { + /** @return + */ + int method() { + } + } + """, + """ + class Test { + /** + */ + int method() { + } + } + """ + ) + ); + } + + @Test + void firstLineThrows() { + rewriteRun( + //language=java + java( + """ + class Test { + /** @throws + */ + int method() throws IllegalStateException { + } + } + """, + """ + class Test { + /** + */ + int method() throws IllegalStateException { + } + } + """ + ) + ); + } + + @Test + void firstLineParamNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /**@param arg0 + */ + void method(int arg0) { + } + } + """, + """ + class Test { + /** + */ + void method(int arg0) { + } + } + """ + ) + ); + } + + @Test + void firstLineReturnNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /**@return + */ + int method() { + } + } + """, + """ + class Test { + /** + */ + int method() { + } + } + """ + ) + ); + } + + @Test + void firstLineThrowsNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /**@throws + */ + int method() throws IllegalStateException { + } + } + """, + """ + class Test { + /** + */ + int method() throws IllegalStateException { + } + } + """ + ) + ); + } + } + + @Nested + class EmptyJavaDoc { + @Test + void emptyJavaDoc() { + rewriteRun( + //language=java + java( + """ + class Test { + /** + */ + void method(int arg0) { + } + } + """ + ) + ); + } + + @Test + void emptyJavaDocSingleLine() { + rewriteRun( + //language=java + java( + """ + class Test { + /** */ + void method(int arg0) { + } + } + """ + ) + ); + } + + @Test + void emptyJavaDocSingleLineNoSpace() { + rewriteRun( + //language=java + java( + """ + class Test { + /***/ + void method(int arg0) { + } + } + """ + ) + ); + } + } + + @Test - void emptyThrowsOnFirstLine() { + @Issue("https://github.com/openrewrite/rewrite/issues/3078") + void visitingQuarkMustNotFail() { rewriteRun( - //language=java - java( - """ - class Test { - /** @throws*/ - void method() throws IllegalStateException { - } - } - """, + other( """ - class Test { - /***/ - void method() throws IllegalStateException { - } - } + foo """ ) );