diff --git a/src/main/java/org/openrewrite/staticanalysis/ControlFlowIndentation.java b/src/main/java/org/openrewrite/staticanalysis/ControlFlowIndentation.java index 4d784dc9fe..9ea4c749d2 100755 --- a/src/main/java/org/openrewrite/staticanalysis/ControlFlowIndentation.java +++ b/src/main/java/org/openrewrite/staticanalysis/ControlFlowIndentation.java @@ -66,7 +66,7 @@ public TreeVisitor 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(); } diff --git a/src/main/java/org/openrewrite/staticanalysis/CovariantEquals.java b/src/main/java/org/openrewrite/staticanalysis/CovariantEquals.java index 9ed4e4f6a0..3c31d1f394 100644 --- a/src/main/java/org/openrewrite/staticanalysis/CovariantEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/CovariantEquals.java @@ -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 { @@ -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 @@ -50,6 +56,98 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { - return new CovariantEqualsVisitor<>(); + MethodMatcher objectEquals = new MethodMatcher("* equals(java.lang.Object)"); + return new JavaIsoVisitor() { + + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); + Stream 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 { + 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 ctx) { + J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); + 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; + } + } + }; } } diff --git a/src/main/java/org/openrewrite/staticanalysis/CovariantEqualsVisitor.java b/src/main/java/org/openrewrite/staticanalysis/CovariantEqualsVisitor.java deleted file mode 100644 index 6ac7c5b0b4..0000000000 --- a/src/main/java/org/openrewrite/staticanalysis/CovariantEqualsVisitor.java +++ /dev/null @@ -1,125 +0,0 @@ -/* - * Copyright 2020 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.openrewrite.staticanalysis; - -import org.openrewrite.Cursor; -import org.openrewrite.Incubating; -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.util.Comparator; -import java.util.stream.Stream; - -@Incubating(since = "7.0.0") -public class CovariantEqualsVisitor

extends JavaIsoVisitor

{ - private static final MethodMatcher OBJECT_EQUALS = new MethodMatcher("* equals(java.lang.Object)"); - - @Override - public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P p) { - J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, p); - Stream 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 -> OBJECT_EQUALS.matches(m, classDecl))) { - cd = (J.ClassDeclaration) new ChangeCovariantEqualsMethodVisitor<>(cd).visit(cd, p, getCursor().getParentOrThrow()); - assert cd != null; - } - return cd; - } - - private static class ChangeCovariantEqualsMethodVisitor

extends JavaIsoVisitor

{ - private static final AnnotationMatcher OVERRIDE_ANNOTATION = new AnnotationMatcher("@java.lang.Override"); - private static final String EQUALS_BODY_PREFIX_TEMPLATE = - "if (#{} == this) return true;\n" + - "if (#{} == null || getClass() != #{}.getClass()) return false;\n" + - "#{} #{} = (#{}) #{};\n"; - - private final J.ClassDeclaration enclosingClass; - - public ChangeCovariantEqualsMethodVisitor(J.ClassDeclaration enclosingClass) { - this.enclosingClass = enclosingClass; - } - - @Override - public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, P 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. - */ - JavaTemplate equalsBodySnippet = JavaTemplate.builder(EQUALS_BODY_PREFIX_TEMPLATE).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; - } - } -} diff --git a/src/main/java/org/openrewrite/staticanalysis/DefaultComesLast.java b/src/main/java/org/openrewrite/staticanalysis/DefaultComesLast.java index a8f85e1dcb..bf04bcf8d5 100644 --- a/src/main/java/org/openrewrite/staticanalysis/DefaultComesLast.java +++ b/src/main/java/org/openrewrite/staticanalysis/DefaultComesLast.java @@ -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(); } @@ -70,5 +70,4 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) { return (J) tree; } } - } diff --git a/src/main/java/org/openrewrite/staticanalysis/EmptyBlock.java b/src/main/java/org/openrewrite/staticanalysis/EmptyBlock.java index 0f636271a1..1604096c81 100644 --- a/src/main/java/org/openrewrite/staticanalysis/EmptyBlock.java +++ b/src/main/java/org/openrewrite/staticanalysis/EmptyBlock.java @@ -61,7 +61,7 @@ private static class EmptyBlockFromCompilationUnitStyle extends JavaIsoVisitor 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(); } diff --git a/src/main/java/org/openrewrite/staticanalysis/FallThrough.java b/src/main/java/org/openrewrite/staticanalysis/FallThrough.java index ba23f852f7..130feb7344 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FallThrough.java +++ b/src/main/java/org/openrewrite/staticanalysis/FallThrough.java @@ -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(); } diff --git a/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java b/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java index 00b6996462..fb29e0dd17 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java @@ -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()); } } } diff --git a/src/main/java/org/openrewrite/staticanalysis/HiddenField.java b/src/main/java/org/openrewrite/staticanalysis/HiddenField.java index 0a2b132865..49873b7657 100644 --- a/src/main/java/org/openrewrite/staticanalysis/HiddenField.java +++ b/src/main/java/org/openrewrite/staticanalysis/HiddenField.java @@ -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(); } diff --git a/src/main/java/org/openrewrite/staticanalysis/HideUtilityClassConstructor.java b/src/main/java/org/openrewrite/staticanalysis/HideUtilityClassConstructor.java index 43026d3994..9f04f9b8ce 100644 --- a/src/main/java/org/openrewrite/staticanalysis/HideUtilityClassConstructor.java +++ b/src/main/java/org/openrewrite/staticanalysis/HideUtilityClassConstructor.java @@ -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(); } diff --git a/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java b/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java index 936f1b2a3d..a68f3f98a6 100644 --- a/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java +++ b/src/main/java/org/openrewrite/staticanalysis/LambdaBlockToExpression.java @@ -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); diff --git a/src/main/java/org/openrewrite/staticanalysis/MethodNameCasing.java b/src/main/java/org/openrewrite/staticanalysis/MethodNameCasing.java index 5c7156572a..cca1935db9 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MethodNameCasing.java +++ b/src/main/java/org/openrewrite/staticanalysis/MethodNameCasing.java @@ -187,7 +187,7 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) { } @Value - static class MethodNameChange { + public static class MethodNameChange { UUID scope; boolean privateMethod; ChangeMethodName recipe; diff --git a/src/main/java/org/openrewrite/staticanalysis/NestedEnumsAreNotStatic.java b/src/main/java/org/openrewrite/staticanalysis/NestedEnumsAreNotStatic.java index 180e9c84f4..941cb8e5de 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NestedEnumsAreNotStatic.java +++ b/src/main/java/org/openrewrite/staticanalysis/NestedEnumsAreNotStatic.java @@ -67,6 +67,7 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex if (cd.getKind() == J.ClassDeclaration.Kind.Type.Enum && cd.getType() != null && cd.getType().getOwningClass() != null) { if (J.Modifier.hasModifier(cd.getModifiers(), J.Modifier.Type.Static)) { J.Block enumBody = cd.getBody(); + //noinspection DataFlowIssue cd = cd.withBody(null); cd = maybeAutoFormat(cd, cd.withModifiers(ListUtils.map(cd.getModifiers(), mod -> diff --git a/src/main/java/org/openrewrite/staticanalysis/NoDoubleBraceInitialization.java b/src/main/java/org/openrewrite/staticanalysis/NoDoubleBraceInitialization.java index c5199ef78e..171b504d87 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NoDoubleBraceInitialization.java +++ b/src/main/java/org/openrewrite/staticanalysis/NoDoubleBraceInitialization.java @@ -229,7 +229,7 @@ public J.NewClass visitNewClass(J.NewClass newClass, String methodName) { private static class FindMethodInvocationInDoubleBrace extends JavaIsoVisitor { /** - * Find whether any collection content initialization method(e.g add() or put()) is invoked in the double brace. + * Find whether any collection content initialization method(e.g. add() or put()) is invoked in the double brace. * * @param j The subtree to search, supposed to be the 2nd brace (J.Block) * @return true if any method invocation found in the double brace, otherwise false. diff --git a/src/main/java/org/openrewrite/staticanalysis/NoEmptyCollectionWithRawType.java b/src/main/java/org/openrewrite/staticanalysis/NoEmptyCollectionWithRawType.java index 0c058712fc..70c6a420b1 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NoEmptyCollectionWithRawType.java +++ b/src/main/java/org/openrewrite/staticanalysis/NoEmptyCollectionWithRawType.java @@ -42,7 +42,7 @@ public String getDisplayName() { @Override public String getDescription() { - return "Replaces `Collections#EMPTY_..` with methods that return generic types."; + return "Replaces `Collections#EMPTY_...` with methods that return generic types."; } @Override diff --git a/src/main/java/org/openrewrite/staticanalysis/NoToStringOnStringType.java b/src/main/java/org/openrewrite/staticanalysis/NoToStringOnStringType.java index cd45f50a8d..b012fee96b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NoToStringOnStringType.java +++ b/src/main/java/org/openrewrite/staticanalysis/NoToStringOnStringType.java @@ -33,12 +33,12 @@ public class NoToStringOnStringType extends Recipe { @Override public String getDisplayName() { - return "Unnecessary String#toString()"; + return "Unnecessary `String#toString`"; } @Override public String getDescription() { - return "Remove unnecessary `String#toString()` invocations on objects which are already a string."; + return "Remove unnecessary `String#toString` invocations on objects which are already a string."; } @Override diff --git a/src/main/java/org/openrewrite/staticanalysis/NoValueOfOnStringType.java b/src/main/java/org/openrewrite/staticanalysis/NoValueOfOnStringType.java index 123801262f..f50fbe1456 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NoValueOfOnStringType.java +++ b/src/main/java/org/openrewrite/staticanalysis/NoValueOfOnStringType.java @@ -37,7 +37,7 @@ public class NoValueOfOnStringType extends Recipe { @Override public String getDisplayName() { - return "Unnecessary String#valueOf(..)"; + return "Unnecessary `String#valueOf(..)`"; } @Override diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java index 23bb0f734f..ece5adb945 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameMethodsNamedHashcodeEqualOrToString.java @@ -26,6 +26,7 @@ import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; +import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; import java.util.Collections; @@ -58,7 +59,7 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(Preconditions.or(new DeclaresMethod<>(NO_ARGS), new DeclaresMethod<>(OBJECT_ARG)), new JavaIsoVisitor() { + return Preconditions.check(Preconditions.and(new JavaFileChecker<>(), Preconditions.or(new DeclaresMethod<>(NO_ARGS), new DeclaresMethod<>(OBJECT_ARG))), new JavaIsoVisitor() { @Override public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { if (method.getMethodType() != null && method.getReturnTypeExpression() != null) { diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBuilderWithString.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBuilderWithString.java index 2879b22f87..92b61a3310 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBuilderWithString.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBuilderWithString.java @@ -37,7 +37,7 @@ public class ReplaceStringBuilderWithString extends Recipe { @Override public String getDisplayName() { - return "Replace StringBuilder.append() with String"; + return "Replace `StringBuilder#append` with `String`"; } @Override @@ -88,7 +88,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) return m; } - // Check if a method call is a select of another method call + // Check if a method call is a "select" of another method call private boolean isAMethodSelect(J.MethodInvocation method) { Cursor parent = getCursor().getParent(2); // 2 means skip right padded cursor if (parent == null || !(parent.getValue() instanceof J.MethodInvocation)) { diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java index daa2b29fe3..12d34eafa8 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java @@ -192,7 +192,7 @@ private Optional getReturnExprIfOnlyStatementInElseThen(J.If iff2) { return Optional.empty(); } - private boolean hasElseWithComment(J.If.Else else_) { + private boolean hasElseWithComment(@Nullable J.If.Else else_) { if (else_ == null || else_.getBody() == null) { return false; } @@ -202,11 +202,8 @@ private boolean hasElseWithComment(J.If.Else else_) { if (!else_.getBody().getComments().isEmpty()) { return true; } - if (else_.getBody() instanceof J.Block - && !((J.Block) else_.getBody()).getStatements().get(0).getComments().isEmpty()) { - return true; - } - return false; + return else_.getBody() instanceof J.Block + && !((J.Block) else_.getBody()).getStatements().get(0).getComments().isEmpty(); } }; } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java index d57b984876..960c98c1e0 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java @@ -57,7 +57,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { if (bl != block) { bl = (J.Block) new RemoveUnneededBlock.RemoveUnneededBlockStatementVisitor() .visitNonNull(bl, ctx, getCursor().getParentOrThrow()); - EmptyBlockStyle style = ((SourceFile) getCursor().firstEnclosingOrThrow(JavaSourceFile.class)) + EmptyBlockStyle style = getCursor().firstEnclosingOrThrow(JavaSourceFile.class) .getStyle(EmptyBlockStyle.class); if (style == null) { style = Checkstyle.emptyBlock(); diff --git a/src/main/java/org/openrewrite/staticanalysis/StaticMethodNotFinal.java b/src/main/java/org/openrewrite/staticanalysis/StaticMethodNotFinal.java index 8d9e613a66..c2bd31b030 100644 --- a/src/main/java/org/openrewrite/staticanalysis/StaticMethodNotFinal.java +++ b/src/main/java/org/openrewrite/staticanalysis/StaticMethodNotFinal.java @@ -29,7 +29,7 @@ public class StaticMethodNotFinal extends Recipe { @Override public String getDisplayName() { - return "Static methods not final"; + return "Static methods need not be final"; } @Override diff --git a/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java b/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java index 87540c7f13..a748478460 100644 --- a/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java +++ b/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java @@ -20,8 +20,7 @@ import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; -import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; -import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; +import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; import java.util.Collections; @@ -33,7 +32,7 @@ public class StringLiteralEquality extends Recipe { @Override public String getDisplayName() { - return "Use `String.equals()` on String literals"; + return "Use `String.equals()` on `String` literals"; } @Override @@ -59,9 +58,7 @@ public TreeVisitor getVisitor() { // Don't change for Kotlin because In Kotlin, `==` means structural equality, so it's redundant to call equals(). // see https://rules.sonarsource.com/kotlin/RSPEC-S6519/ TreeVisitor preconditions = Preconditions.and( - Preconditions.and( - Preconditions.not(new KotlinFileChecker<>()), - Preconditions.not(new GroovyFileChecker<>())), + new JavaFileChecker<>(), new UsesType<>("java.lang.String", false)); return Preconditions.check(preconditions, new JavaVisitor() { private final JavaType.FullyQualified TYPE_STRING = TypeUtils.asFullyQualified(JavaType.buildType("java.lang.String")); diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCloseInTryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCloseInTryWithResources.java index 2f35fdb0f2..7bfd0fb7b6 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCloseInTryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCloseInTryWithResources.java @@ -16,12 +16,16 @@ package org.openrewrite.staticanalysis; import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.tree.J; +import org.openrewrite.staticanalysis.groovy.GroovyFileChecker; +import org.openrewrite.staticanalysis.java.JavaFileChecker; +import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; import java.time.Duration; import java.util.Collections; @@ -50,7 +54,14 @@ public Set getTags() { @Override public TreeVisitor getVisitor() { - return new UnnecessaryAutoCloseableVisitor(); + return Preconditions.check( + Preconditions.or( + new JavaFileChecker<>(), + new KotlinFileChecker<>(), + new GroovyFileChecker<>() + ), + new UnnecessaryAutoCloseableVisitor() + ); } private static class UnnecessaryAutoCloseableVisitor extends JavaIsoVisitor { diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java index 9c7003e442..f728f2fde8 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java @@ -18,6 +18,7 @@ import org.openrewrite.*; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.tree.*; +import org.openrewrite.staticanalysis.java.JavaFileChecker; import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker; public class UnnecessaryExplicitTypeArguments extends Recipe { @@ -34,7 +35,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(Preconditions.not(new KotlinFileChecker<>()), new JavaIsoVisitor() { + return Preconditions.check(new JavaFileChecker<>(), new JavaIsoVisitor() { @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { J.MethodInvocation m = super.visitMethodInvocation(method, ctx); diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryPrimitiveAnnotations.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryPrimitiveAnnotations.java index 83cd834f98..adb5d1063d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryPrimitiveAnnotations.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryPrimitiveAnnotations.java @@ -37,12 +37,12 @@ public class UnnecessaryPrimitiveAnnotations extends Recipe { @Override public String getDisplayName() { - return "Remove Nullable and CheckForNull annotations from primitives"; + return "Remove `@Nullable` and `@CheckForNull` annotations from primitives"; } @Override public String getDescription() { - return "Remove `@Nullable` and `@CheckForNull` annotations from primitives since they can't be null."; + return "Primitives can't be null anyway, so these annotations are not useful in this context."; } @Override diff --git a/src/main/java/org/openrewrite/staticanalysis/UpperCaseLiteralSuffixes.java b/src/main/java/org/openrewrite/staticanalysis/UpperCaseLiteralSuffixes.java index d43837de7f..92d4ffa019 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UpperCaseLiteralSuffixes.java +++ b/src/main/java/org/openrewrite/staticanalysis/UpperCaseLiteralSuffixes.java @@ -24,6 +24,7 @@ import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; import java.util.Collections; @@ -52,37 +53,41 @@ public Set getTags() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(Preconditions.or( - new UsesType<>("long", false), - new UsesType<>("java.lang.Long", false), - new UsesType<>("double", false), - new UsesType<>("java.lang.Double", false), - new UsesType<>("float", false), - new UsesType<>("java.lang.Float", false) - ), new JavaIsoVisitor() { - @Override - public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { - J.VariableDeclarations.NamedVariable nv = super.visitVariable(variable, ctx); - if (nv.getInitializer() instanceof J.Literal && nv.getInitializer().getType() != null) { - J.Literal initializer = (J.Literal) nv.getInitializer(); - if (initializer.getType() == JavaType.Primitive.Double - || initializer.getType() == JavaType.Primitive.Float - || initializer.getType() == JavaType.Primitive.Long) { - String upperValueSource = upperCaseSuffix(initializer.getValueSource()); - if (upperValueSource != null && !upperValueSource.equals(initializer.getValueSource())) { - nv = nv.withInitializer(initializer.withValueSource(upperValueSource)); + return Preconditions.check( + Preconditions.and( + new JavaFileChecker<>(), + Preconditions.or( + new UsesType<>("long", false), + new UsesType<>("java.lang.Long", false), + new UsesType<>("double", false), + new UsesType<>("java.lang.Double", false), + new UsesType<>("float", false), + new UsesType<>("java.lang.Float", false) + ) + ), new JavaIsoVisitor() { + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + J.VariableDeclarations.NamedVariable nv = super.visitVariable(variable, ctx); + if (nv.getInitializer() instanceof J.Literal && nv.getInitializer().getType() != null) { + J.Literal initializer = (J.Literal) nv.getInitializer(); + if (initializer.getType() == JavaType.Primitive.Double + || initializer.getType() == JavaType.Primitive.Float + || initializer.getType() == JavaType.Primitive.Long) { + String upperValueSource = upperCaseSuffix(initializer.getValueSource()); + if (upperValueSource != null && !upperValueSource.equals(initializer.getValueSource())) { + nv = nv.withInitializer(initializer.withValueSource(upperValueSource)); + } + } } + return nv; } - } - return nv; - } - private @Nullable String upperCaseSuffix(@Nullable String valueSource) { - if (valueSource == null || valueSource.length() < 2) { - return valueSource; - } - return valueSource.substring(0, valueSource.length() - 1) + valueSource.substring(valueSource.length() - 1).toUpperCase(); - } - }); + private @Nullable String upperCaseSuffix(@Nullable String valueSource) { + if (valueSource == null || valueSource.length() < 2) { + return valueSource; + } + return valueSource.substring(0, valueSource.length() - 1) + valueSource.substring(valueSource.length() - 1).toUpperCase(); + } + }); } } diff --git a/src/main/java/org/openrewrite/staticanalysis/UseDiamondOperator.java b/src/main/java/org/openrewrite/staticanalysis/UseDiamondOperator.java index 5212481c7f..eb5c6a093b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseDiamondOperator.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseDiamondOperator.java @@ -35,12 +35,15 @@ public class UseDiamondOperator extends Recipe { @Override public String getDisplayName() { - return "Use diamond operator"; + return "Use the diamond operator"; } @Override public String getDescription() { - return "The diamond operator (`<>`) should be used. Java 7 introduced the diamond operator (<>) to reduce the verbosity of generics code. For instance, instead of having to declare a List's type in both its declaration and its constructor, you can now simplify the constructor declaration with `<>`, and the compiler will infer the type."; + return "The diamond operator (`<>`) should be used. Java 7 introduced the diamond operator (<>) to " + + "reduce the verbosity of generics code. For instance, instead of having to declare a `List`'s " + + "type in both its declaration and its constructor, you can now simplify the constructor declaration " + + "with `<>`, and the compiler will infer the type."; } @Override diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java index 0403d02d6c..90f945e7dd 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java @@ -29,7 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; -@SuppressWarnings({"unchecked", "RedundantCast", "SimplifyStreamApiCallChains", "Convert2MethodRef", "CodeBlock2Expr", "RedundantOperationOnEmptyContainer", "ResultOfMethodCallIgnored", "rawtypes", "UnusedAssignment"}) +@SuppressWarnings({"unchecked", "RedundantCast", "SimplifyStreamApiCallChains", "Convert2MethodRef", "CodeBlock2Expr", "RedundantOperationOnEmptyContainer", "ResultOfMethodCallIgnored", "rawtypes", "UnusedAssignment", "OptionalGetWithoutIsPresent"}) class ReplaceLambdaWithMethodReferenceTest implements RewriteTest { @Override @@ -87,7 +87,7 @@ void ignoreAmbiguousMethodReference() { java( """ import java.util.stream.Stream; - + class Test { Stream method() { return Stream.of(1, 32, 12, 15, 23).map(x -> Integer.toString(x)); @@ -242,7 +242,7 @@ List method(List input) { //language=java """ import org.test.CheckType; - + import java.util.List; import java.util.stream.Collectors; @@ -525,16 +525,16 @@ void method(List input) { @Test void castType() { rewriteRun( - //language=java java( + //language=java """ package org.test; public class CheckType { } """ ), - //language=java java( + //language=java """ import java.util.List; import java.util.stream.Collectors; @@ -550,6 +550,7 @@ List filter(List l) { } } """, + //language=java """ import java.util.List; import java.util.stream.Collectors; @@ -606,7 +607,7 @@ List filter(List l) { //language=java """ import org.test.CheckType; - + import java.util.List; import java.util.stream.Collectors; @@ -1335,21 +1336,21 @@ void groupingByGetClass() { //language=java java( """ - import java.util.*; - import java.util.stream.*; - - class Animal {} - class Cat extends Animal {} - class Dog extends Animal {} + import java.util.*; + import java.util.stream.*; + + class Animal {} + class Cat extends Animal {} + class Dog extends Animal {} - class Test { - public void groupOnGetClass() { - List animals = List.of(new Cat(), new Dog()); - Map, List> collect; - collect = animals.stream().collect(Collectors.groupingBy(a -> a.getClass())); + class Test { + public void groupOnGetClass() { + List animals = List.of(new Cat(), new Dog()); + Map, List> collect; + collect = animals.stream().collect(Collectors.groupingBy(a -> a.getClass())); + } } - } - """ + """ ) ); }