diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index 18dd0e6d78..b445d51aed 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -216,6 +216,12 @@ testng provided + + org.projectlombok + lombok + ${version.lombok} + import + @@ -241,6 +247,11 @@ refaster-support ${project.version} + + org.projectlombok + lombok + ${version.lombok} + -Xplugin:RefasterRuleCompiler diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java index 212d0b1245..77c7d373d9 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/DirectReturn.java @@ -25,6 +25,7 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.BlockTree; +import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionStatementTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; @@ -76,6 +77,11 @@ public Description matchBlock(BlockTree tree, VisitorState state) { return Description.NO_MATCH; } + //TODO: Fix is => Maybe exclude Lombok @Data classes? +// if (isClassAnnotatedWithLombokData(state)) { +// return Description.NO_MATCH; +// } + Symbol variableSymbol = ASTHelpers.getSymbol(((ReturnTree) finalStatement).getExpression()); StatementTree precedingStatement = statements.get(statements.size() - 2); @@ -91,12 +97,26 @@ public Description matchBlock(BlockTree tree, VisitorState state) { SuggestedFix.builder() .replace( precedingStatement, - String.format("return %s;", SourceCode.treeToString(resultExpr, state))) - .delete(finalStatement) +// String.format("return %s;", SourceCode.treeToString(resultExpr, state))) + String.format("return %s;", SourceCode.treeToString(resultExpr))) + //Same case for trying to delete wrongly assumed "source code". + //Note 2: Seems we can only use ONE fix because of the way the errorprone replaces "source code" +// .delete(finalStatement) +// .replace(finalStatement, "") .build())) .orElse(Description.NO_MATCH); } + private boolean isClassAnnotatedWithLombokData(VisitorState state) { + ClassTree enclosingNode = ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); + + return ASTHelpers.getAnnotations(enclosingNode) + .stream() + .anyMatch(annotation -> + annotation.toString().contains("Data") + ); + } + private static Optional tryMatchAssignment(Symbol targetSymbol, Tree tree) { if (tree instanceof ExpressionStatementTree) { return tryMatchAssignment(targetSymbol, ((ExpressionStatementTree) tree).getExpression()); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java index 1fd8a17f70..3b2bbba01f 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java @@ -38,10 +38,42 @@ private SourceCode() {} * @return A non-{@code null} string. */ public static String treeToString(Tree tree, VisitorState state) { + /** The issue starts inside `state.getSourceForNode(tree)` statement because line: + * CharSequence source = getSourceCode() + * + * returns => + * """ + *import lombok.Data; + * + * @Data + * public class AnyClass { + * private String anything; + * } + * """ + * Which is not the correct start & end position of the intended replacement statement. + + if (end == -1) { + return null; + } + checkArgument(start >= 0, "invalid start position (%s) for: %s", start, tree); + checkArgument(start < end, "invalid source positions (%s, %s) for: %s", start, end, tree); + checkArgument(end <= source.length(), "invalid end position (%s) for: %s", end, tree); + return source.subSequence(start, end).toString(); + */ String src = state.getSourceForNode(tree); return src != null ? src : tree.toString(); } + + + /** + * TODO: Call this conditionally from DirectReturn ?? + */ + public static String treeToString(Tree tree) { + String src = tree.toString(); + return src != null ? src : tree.toString(); + } + /** * Creates a {@link SuggestedFix} for the deletion of the given {@link Tree}, including any * whitespace that follows it. diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java index 42e1889414..ea3bbda768 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/DirectReturnTest.java @@ -14,8 +14,10 @@ void identification() { "import static org.mockito.Mockito.mock;", "import static org.mockito.Mockito.spy;", "", + "import lombok.Data;", "import java.util.function.Supplier;", "", + "@Data", "class A {", " private String field;", "", @@ -223,4 +225,26 @@ void replacement() { "}") .doTest(TestMode.TEXT_MATCH); } + + @Test + void lombokDataReplacement() { + BugCheckerRefactoringTestHelper.newInstance(DirectReturn.class, getClass()) + .addInputLines( + "AnyClass.java", + "import lombok.Data;", + "", + "@Data", + "public class AnyClass {", + "private String anything;", + "}") + .addOutputLines( + "AnyClass.java", + "import lombok.Data;", + "", + "@Data", + "public class AnyClass {", + "private String anything;", + "}") + .doTest(TestMode.TEXT_MATCH); + } } diff --git a/pom.xml b/pom.xml index a2c2bc35bc..0dd2557b28 100644 --- a/pom.xml +++ b/pom.xml @@ -205,6 +205,7 @@ 0.1.18 1.0 11 + 1.18.22 3.8.7 5.4.0 1.0.1