Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.util.Collections;
import java.util.Set;

public class BooleanChecksNotInverted extends Recipe {
public class BooleanChecksNotInverted extends Recipe {

@Override
public String getDisplayName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
TabsAndIndentsStyle style = ((SourceFile) cu).getStyle(TabsAndIndentsStyle.class);
TabsAndIndentsStyle style = cu.getStyle(TabsAndIndentsStyle.class);
if (style == null) {
style = IntelliJ.tabsAndIndents();
}
Expand Down
110 changes: 104 additions & 6 deletions src/main/java/org/openrewrite/staticanalysis/CovariantEquals.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,20 @@
*/
package org.openrewrite.staticanalysis;

import org.openrewrite.ExecutionContext;
import org.openrewrite.Incubating;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.*;
import org.openrewrite.java.AnnotationMatcher;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.service.AnnotationService;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;

import java.time.Duration;
import java.util.Collections;
import java.util.Comparator;
import java.util.Set;
import java.util.stream.Stream;

@Incubating(since = "7.0.0")
public class CovariantEquals extends Recipe {
Expand All @@ -35,7 +41,7 @@ public String getDisplayName() {
@Override
public String getDescription() {
return "Checks that classes and records which define a covariant `equals()` method also override method `equals(Object)`. " +
"Covariant `equals()` means a method that is similar to `equals(Object)`, but with a covariant parameter type (any subtype of `Object`).";
"Covariant `equals()` means a method that is similar to `equals(Object)`, but with a covariant parameter type (any subtype of `Object`).";
}

@Override
Expand All @@ -50,6 +56,98 @@ public Duration getEstimatedEffortPerOccurrence() {

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new CovariantEqualsVisitor<>();
MethodMatcher objectEquals = new MethodMatcher("* equals(java.lang.Object)");
return new JavaIsoVisitor<ExecutionContext>() {

@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) {
J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx);
Stream<J.MethodDeclaration> mds = cd.getBody().getStatements().stream()
.filter(J.MethodDeclaration.class::isInstance)
.map(J.MethodDeclaration.class::cast);
if (cd.getKind() != J.ClassDeclaration.Kind.Type.Interface && mds.noneMatch(m -> objectEquals.matches(m, classDecl))) {
cd = (J.ClassDeclaration) new ChangeCovariantEqualsMethodVisitor(cd).visit(cd, ctx, getCursor().getParentOrThrow());
assert cd != null;
}
return cd;
}

class ChangeCovariantEqualsMethodVisitor extends JavaIsoVisitor<ExecutionContext> {
private final AnnotationMatcher OVERRIDE_ANNOTATION = new AnnotationMatcher("@java.lang.Override");

private final J.ClassDeclaration enclosingClass;

public ChangeCovariantEqualsMethodVisitor(J.ClassDeclaration enclosingClass) {
this.enclosingClass = enclosingClass;
}

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext p) {
J.MethodDeclaration m = super.visitMethodDeclaration(method, p);
updateCursor(m);

/*
* Looking for "public boolean equals(EnclosingClassType)" as the method signature match.
* We'll replace it with "public boolean equals(Object)"
*/
JavaType.FullyQualified type = enclosingClass.getType();
if (type == null || type instanceof JavaType.Unknown) {
return m;
}

String ecfqn = type.getFullyQualifiedName();
if (m.hasModifier(J.Modifier.Type.Public) &&
m.getReturnTypeExpression() != null &&
JavaType.Primitive.Boolean.equals(m.getReturnTypeExpression().getType()) &&
new MethodMatcher(ecfqn + " equals(" + ecfqn + ")").matches(m, enclosingClass)) {

if (!service(AnnotationService.class).matches(getCursor(), OVERRIDE_ANNOTATION)) {
m = JavaTemplate.builder("@Override").build()
.apply(updateCursor(m),
m.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName)));
}

/*
* Change parameter type to Object, and maybe change input parameter name representing the other object.
* This is because we prepend these type-checking replacement statements to the existing "equals(..)" body.
* Therefore we don't want to collide with any existing variable names.
*/
J.VariableDeclarations.NamedVariable oldParamName = ((J.VariableDeclarations) m.getParameters().get(0)).getVariables().get(0);
String paramName = "obj".equals(oldParamName.getSimpleName()) ? "other" : "obj";
m = JavaTemplate.builder("Object #{}").build()
.apply(updateCursor(m),
m.getCoordinates().replaceParameters(),
paramName);

/*
* We'll prepend this type-check and type-cast to the beginning of the existing
* equals(..) method body statements, and let the existing equals(..) method definition continue
* with the logic doing what it was doing.
*/
String equalsBodyPrefixTemplate = "if (#{} == this) return true;\n" +
"if (#{} == null || getClass() != #{}.getClass()) return false;\n" +
"#{} #{} = (#{}) #{};\n";
JavaTemplate equalsBodySnippet = JavaTemplate.builder(equalsBodyPrefixTemplate).contextSensitive().build();

assert m.getBody() != null;
Object[] params = new Object[]{
paramName,
paramName,
paramName,
enclosingClass.getSimpleName(),
oldParamName.getSimpleName(),
enclosingClass.getSimpleName(),
paramName
};

m = equalsBodySnippet.apply(new Cursor(getCursor().getParent(), m),
m.getBody().getStatements().get(0).getCoordinates().before(),
params);
}

return m;
}
}
};
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static class DefaultComesLastFromCompilationUnitStyle extends JavaIsoVis
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
DefaultComesLastStyle style = ((SourceFile) cu).getStyle(DefaultComesLastStyle.class);
DefaultComesLastStyle style = cu.getStyle(DefaultComesLastStyle.class);
if (style == null) {
style = Checkstyle.defaultComesLast();
}
Expand All @@ -70,5 +70,4 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
return (J) tree;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static class EmptyBlockFromCompilationUnitStyle extends JavaIsoVisitor<E
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
EmptyBlockStyle style = ((SourceFile) cu).getStyle(EmptyBlockStyle.class);
EmptyBlockStyle style = cu.getStyle(EmptyBlockStyle.class);
if (style == null) {
style = Checkstyle.emptyBlock();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static class EqualsAvoidsNullFromCompilationUnitStyle extends JavaIsoVis
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
EqualsAvoidsNullStyle style = ((SourceFile) cu).getStyle(EqualsAvoidsNullStyle.class);
EqualsAvoidsNullStyle style = cu.getStyle(EqualsAvoidsNullStyle.class);
if (style == null) {
style = Checkstyle.equalsAvoidsNull();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
ExplicitInitializationStyle style = ((SourceFile) cu).getStyle(ExplicitInitializationStyle.class);
ExplicitInitializationStyle style = cu.getStyle(ExplicitInitializationStyle.class);
if (style == null) {
style = Checkstyle.explicitInitialization();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private static class FallThroughFromCompilationUnitStyle extends JavaIsoVisitor<
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
FallThroughStyle style = ((SourceFile) cu).getStyle(FallThroughStyle.class);
FallThroughStyle style = cu.getStyle(FallThroughStyle.class);
if (style == null) {
style = Checkstyle.fallThrough();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public J.Case visitCase(J.Case case_, P p) {
J.Switch switch_ = getCursor().dropParentUntil(J.Switch.class::isInstance).getValue();
if (Boolean.TRUE.equals(style.getCheckLastCaseGroup()) || !isLastCase(case_, switch_)) {
if (FindLastLineBreaksOrFallsThroughComments.find(switch_, c).isEmpty()) {
c = (J.Case) new AddBreak<>(c).visit(c, p, getCursor().getParent());
c = (J.Case) new AddBreak<>(c).visitNonNull(c, p, getCursor().getParentOrThrow());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static class HiddenFieldFromCompilationUnitStyle extends JavaIsoVisitor<
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
HiddenFieldStyle style = ((SourceFile) cu).getStyle(HiddenFieldStyle.class);
HiddenFieldStyle style = cu.getStyle(HiddenFieldStyle.class);
if (style == null) {
style = Checkstyle.hiddenFieldStyle();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ private static class HideUtilityClassConstructorFromCompilationUnitStyle extends
public J visit(@Nullable Tree tree, ExecutionContext ctx) {
if (tree instanceof JavaSourceFile) {
JavaSourceFile cu = (JavaSourceFile) requireNonNull(tree);
HideUtilityClassConstructorStyle style = ((SourceFile) cu).getStyle(HideUtilityClassConstructorStyle.class);
HideUtilityClassConstructorStyle style = cu.getStyle(HideUtilityClassConstructorStyle.class);
if (style == null) {
style = Checkstyle.hideUtilityClassConstructorStyle();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ public J.Lambda visitLambda(J.Lambda lambda, ExecutionContext ctx) {
Space prefix = statement.getPrefix();
if (statement instanceof J.Return) {
Expression expression = ((J.Return) statement).getExpression();
if (prefix.getComments().isEmpty()) {
return l.withBody(expression);
} else {
return l.withBody(expression.withPrefix(prefix));
}
if (prefix.getComments().isEmpty()) {
//noinspection DataFlowIssue
return l.withBody(expression);
} else if (expression != null) {
return l.withBody(expression.withPrefix(prefix));
}
} else if (statement instanceof J.MethodInvocation) {
if (prefix.getComments().isEmpty()) {
return l.withBody(statement);
Expand Down
Loading