diff --git a/.github/workflows/dependency-check.yml b/.github/workflows/dependency-check.yml deleted file mode 100644 index 78b71d298..000000000 --- a/.github/workflows/dependency-check.yml +++ /dev/null @@ -1,20 +0,0 @@ ---- -name: dependency-check - -on: - workflow_dispatch: {} - schedule: - - cron: 0 18 * * * - -concurrency: - group: dependency-check-${{ github.ref }} - cancel-in-progress: true - -jobs: - dependency-check: - uses: openrewrite/gh-automation/.github/workflows/dependency-check-gradle.yml@main - secrets: - slack_alerts_webhook: ${{ secrets.SLACK_ALERTS_WEBHOOK }} - gradle_enterprise_access_key: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }} - gradle_enterprise_cache_username: ${{ secrets.GRADLE_ENTERPRISE_CACHE_USERNAME }} - gradle_enterprise_cache_password: ${{ secrets.GRADLE_ENTERPRISE_CACHE_PASSWORD }} diff --git a/build.gradle.kts b/build.gradle.kts index 18c1c1ddf..f483d4a40 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -15,12 +15,13 @@ dependencies { implementation(platform("org.openrewrite:rewrite-bom:${rewriteVersion}")) implementation("org.openrewrite:rewrite-java") + implementation("org.openrewrite:rewrite-groovy:${rewriteVersion}") implementation("org.openrewrite:rewrite-kotlin:${rewriteVersion}") implementation("org.openrewrite.meta:rewrite-analysis:${rewriteVersion}") implementation("org.apache.commons:commons-text:latest.release") testImplementation("org.openrewrite:rewrite-groovy") - testImplementation("org.junit-pioneer:junit-pioneer:2.0.0") + testImplementation("org.junit-pioneer:junit-pioneer:2.0.1") testImplementation("junit:junit:4.13.2") testRuntimeOnly("org.openrewrite:rewrite-java-17") diff --git a/src/main/java/org/openrewrite/staticanalysis/AddSerialVersionUidToSerializable.java b/src/main/java/org/openrewrite/staticanalysis/AddSerialVersionUidToSerializable.java index 8f513530a..ec5e80ae2 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AddSerialVersionUidToSerializable.java +++ b/src/main/java/org/openrewrite/staticanalysis/AddSerialVersionUidToSerializable.java @@ -101,9 +101,9 @@ private J.VariableDeclarations maybeFixVariableDeclarations(J.VariableDeclaratio || !J.Modifier.hasModifier(modifiers, J.Modifier.Type.Static) || !J.Modifier.hasModifier(modifiers, J.Modifier.Type.Final)) { varDecls = varDecls.withModifiers(Arrays.asList( - new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, J.Modifier.Type.Private, Collections.emptyList()), - new J.Modifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, J.Modifier.Type.Static, Collections.emptyList()), - new J.Modifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, J.Modifier.Type.Final, Collections.emptyList()) + new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, J.Modifier.Type.Private, Collections.emptyList()), + new J.Modifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, null, J.Modifier.Type.Static, Collections.emptyList()), + new J.Modifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, null, J.Modifier.Type.Final, Collections.emptyList()) )); } if (TypeUtils.asPrimitive(varDecls.getType()) != JavaType.Primitive.Long) { diff --git a/src/main/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperator.java b/src/main/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperator.java index 2a7053698..1e95fcca9 100644 --- a/src/main/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperator.java +++ b/src/main/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperator.java @@ -55,7 +55,7 @@ public TreeVisitor getVisitor() { @Override public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) { J.MethodInvocation m = (J.MethodInvocation) super.visitMethodInvocation(method, executionContext); - if (enumEquals.matches(m)) { + if (enumEquals.matches(m) && m.getSelect() != null) { Cursor parent = getCursor().dropParentUntil(is -> is instanceof J.Unary || is instanceof J.Block); boolean isNot = parent.getValue() instanceof J.Unary && ((J.Unary) parent.getValue()).getOperator() == J.Unary.Type.Not; if (isNot) { diff --git a/src/main/java/org/openrewrite/staticanalysis/ExplicitLambdaArgumentTypes.java b/src/main/java/org/openrewrite/staticanalysis/ExplicitLambdaArgumentTypes.java index 3e5896492..a5174555f 100755 --- a/src/main/java/org/openrewrite/staticanalysis/ExplicitLambdaArgumentTypes.java +++ b/src/main/java/org/openrewrite/staticanalysis/ExplicitLambdaArgumentTypes.java @@ -31,6 +31,8 @@ import java.util.List; import java.util.Set; +import static java.util.Collections.emptyList; + public class ExplicitLambdaArgumentTypes extends Recipe { @Override public String getDisplayName() { @@ -138,6 +140,7 @@ private TypeTree buildTypeTree(@Nullable JavaType type, Space space) { J.Identifier identifier = new J.Identifier(Tree.randomId(), space, Markers.EMPTY, + emptyList(), fq.getClassName(), type, null @@ -174,6 +177,7 @@ private TypeTree buildTypeTree(@Nullable JavaType type, Space space) { return new J.Identifier(Tree.randomId(), space, Markers.EMPTY, + emptyList(), genericType.getName(), type, null diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java b/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java index 02737fadf..72a275f82 100755 --- a/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java @@ -22,10 +22,7 @@ import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import java.util.*; import static java.util.Collections.emptyList; import static org.openrewrite.Tree.randomId; @@ -112,7 +109,7 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); if (cd.getType() != null && typesToFinalize.remove(cd.getType().getFullyQualifiedName())) { List modifiers = new ArrayList<>(cd.getModifiers()); - modifiers.add(new J.Modifier(randomId(), Space.EMPTY, Markers.EMPTY, J.Modifier.Type.Final, emptyList())); + modifiers.add(new J.Modifier(randomId(), Space.EMPTY, Markers.EMPTY, null, J.Modifier.Type.Final, emptyList())); modifiers = sortModifiers(modifiers); cd = cd.withModifiers(modifiers); if (cd.getType() instanceof JavaType.Class && !cd.getType().hasFlags(Flag.Final)) { diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java b/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java index 6ba73430c..22017c8eb 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java @@ -74,7 +74,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m })) { mv = autoFormat( mv.withModifiers( - ListUtils.concat(mv.getModifiers(), new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, J.Modifier.Type.Final, Collections.emptyList())) + ListUtils.concat(mv.getModifiers(), new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, J.Modifier.Type.Final, Collections.emptyList())) ), p); } diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java b/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java index 1b825e757..b6070fb51 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java @@ -169,6 +169,7 @@ private static VariableDeclarations updateModifiers(final VariableDeclarations v Modifier finalModifier = new Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, + null, Type.Final, emptyList()); if (leadingAnnotations) { diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalizePrivateFields.java b/src/main/java/org/openrewrite/staticanalysis/FinalizePrivateFields.java index d698f8a21..e5e526344 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalizePrivateFields.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalizePrivateFields.java @@ -98,8 +98,8 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return type != null ? v.withVariableType(type.withFlags( Flag.bitMapToFlags(type.getFlagsBitMap() | Flag.Final.getBitMask()))) : null; })).withModifiers(ListUtils.concat(mv.getModifiers(), - new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, J.Modifier.Type.Final, - emptyList()))), ctx); + new J.Modifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, + J.Modifier.Type.Final, emptyList()))), ctx); } return mv; @@ -121,7 +121,9 @@ private static List collectPrivateFields(J .stream() .filter(statement -> statement instanceof J.VariableDeclarations) .map(J.VariableDeclarations.class::cast) - .filter(mv -> mv.hasModifier(J.Modifier.Type.Private) && !mv.hasModifier(J.Modifier.Type.Final)) + .filter(mv -> mv.hasModifier(J.Modifier.Type.Private) + && !mv.hasModifier(J.Modifier.Type.Final) + && !mv.hasModifier(J.Modifier.Type.Volatile)) .filter(mv -> !anyAnnotationApplied(mv)) .map(J.VariableDeclarations::getVariables) .flatMap(Collection::stream) diff --git a/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java b/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java index 5702ed353..bc4b7ff6e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java @@ -212,6 +212,7 @@ public J.InstanceOf processInstanceOf(J.InstanceOf instanceOf, Cursor cursor) { randomId(), Space.build(" ", emptyList()), Markers.EMPTY, + emptyList(), name, type, null)); @@ -251,6 +252,7 @@ public J processTypeCast(J.TypeCast typeCast, Cursor cursor) { randomId(), typeCast.getPrefix(), Markers.EMPTY, + emptyList(), name, typeCast.getType(), fieldType); diff --git a/src/main/java/org/openrewrite/staticanalysis/JavaElementFactory.java b/src/main/java/org/openrewrite/staticanalysis/JavaElementFactory.java index 48ade289c..718e7ffa5 100644 --- a/src/main/java/org/openrewrite/staticanalysis/JavaElementFactory.java +++ b/src/main/java/org/openrewrite/staticanalysis/JavaElementFactory.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.Scanner; +import static java.util.Collections.emptyList; import static org.openrewrite.Tree.randomId; final class JavaElementFactory { @@ -51,11 +52,12 @@ static Expression className(JavaType type, boolean qualified) { Space.EMPTY, Markers.EMPTY, name, - new JLeftPadded<>(Space.EMPTY, new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, part, typeOfContaining, null), Markers.EMPTY), + new JLeftPadded<>(Space.EMPTY, new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, + emptyList(), part, typeOfContaining, null), Markers.EMPTY), typeOfContaining ); } else { - name = new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, part, type, null); + name = new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), part, type, null); } } assert name != null; @@ -69,7 +71,7 @@ static J.MemberReference newInstanceMethodReference(JavaType.Method method, Expr Markers.EMPTY, new JRightPadded<>(containing, Space.EMPTY, Markers.EMPTY), null, - new JLeftPadded<>(Space.EMPTY, new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, method.getName(), null, null), Markers.EMPTY), + new JLeftPadded<>(Space.EMPTY, new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), method.getName(), null, null), Markers.EMPTY), type, method, null @@ -91,7 +93,7 @@ static J.FieldAccess newClassLiteral(@Nullable JavaType type, boolean qualified) className(type, qualified), new JLeftPadded<>( Space.EMPTY, - new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, "class", parameterized, null), + new J.Identifier(randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), "class", parameterized, null), Markers.EMPTY ), parameterized diff --git a/src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCases.java b/src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCases.java index b0965b5f6..c7d8e67ee 100644 --- a/src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCases.java +++ b/src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCases.java @@ -21,10 +21,12 @@ import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.RecipeRunException; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.ShortenFullyQualifiedTypeReferences; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Marker; @@ -61,7 +63,7 @@ public Duration getEstimatedEffortPerOccurrence() { } @Override - public JavaVisitor getVisitor() { + public TreeVisitor getVisitor() { return new JavaVisitor() { final JavaTemplate ifElseIfPrimitive = JavaTemplate.builder("" + "if(#{any()} == #{any()}) {\n" + @@ -183,6 +185,7 @@ public J visitSwitch(J.Switch switch_, ExecutionContext ctx) { } else { generatedIf = ifElseIfEnum.apply(getCursor(), switch_.getCoordinates().replace(), tree, enumIdentToFieldAccessString(cases[0].getPattern()), tree, enumIdentToFieldAccessString(cases[1].getPattern())); } + doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()); } else { if (cases[1] == null) { if (isDefault(cases[0])) { @@ -249,7 +252,7 @@ private boolean switchesOnEnum(J.Switch switch_) { } private String enumIdentToFieldAccessString(Expression casePattern) { - String caseType = requireNonNull(TypeUtils.asFullyQualified(casePattern.getType())).getClassName(); + String caseType = requireNonNull(TypeUtils.asFullyQualified(casePattern.getType())).getFullyQualifiedName(); if (casePattern instanceof J.FieldAccess) { // may be a field access in Groovy return caseType + "." + ((J.FieldAccess) casePattern).getSimpleName(); diff --git a/src/main/java/org/openrewrite/staticanalysis/ModifierOrder.java b/src/main/java/org/openrewrite/staticanalysis/ModifierOrder.java index 9440790e2..61fce5016 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ModifierOrder.java +++ b/src/main/java/org/openrewrite/staticanalysis/ModifierOrder.java @@ -75,11 +75,19 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m } public static List sortModifiers(List modifiers) { + for (J.Modifier mod : modifiers) { + if (mod.getType() == J.Modifier.Type.LanguageExtension) { + // avoid harmful changes with modifiers not seen in Java + return modifiers; + } + } + List sortedTypes = modifiers.stream() .map(J.Modifier::getType) .sorted(Comparator.comparingInt(J.Modifier.Type::ordinal)) .collect(toList()); + return ListUtils.map(modifiers, (i, mod) -> mod.getType() == sortedTypes.get(i) ? mod : mod.withType(sortedTypes.get(i))); } } diff --git a/src/main/java/org/openrewrite/staticanalysis/NeedBraces.java b/src/main/java/org/openrewrite/staticanalysis/NeedBraces.java index b58f916ef..f6344282b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NeedBraces.java +++ b/src/main/java/org/openrewrite/staticanalysis/NeedBraces.java @@ -152,6 +152,5 @@ public J.ForLoop visitForLoop(J.ForLoop forLoop, ExecutionContext ctx) { } return elem; } - - } + }; } diff --git a/src/main/java/org/openrewrite/staticanalysis/NoPrimitiveWrappersForToStringOrCompareTo.java b/src/main/java/org/openrewrite/staticanalysis/NoPrimitiveWrappersForToStringOrCompareTo.java index 74eebc6f7..c27675cd3 100644 --- a/src/main/java/org/openrewrite/staticanalysis/NoPrimitiveWrappersForToStringOrCompareTo.java +++ b/src/main/java/org/openrewrite/staticanalysis/NoPrimitiveWrappersForToStringOrCompareTo.java @@ -33,6 +33,8 @@ import java.time.Duration; import java.util.*; +import static java.util.Collections.emptyList; + public class NoPrimitiveWrappersForToStringOrCompareTo extends Recipe { private static final MethodMatcher NUMBER_TO_STRING_MATCHER = new MethodMatcher("java.lang.Number toString()", true); private static final MethodMatcher BOOLEAN_TO_STRING_MATCHER = new MethodMatcher("java.lang.Boolean toString()", true); @@ -95,7 +97,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } if (arg != null && !TypeUtils.isString(arg.getType()) && mi.getSelect() != null) { JavaType.FullyQualified fq = mi.getMethodType().getDeclaringType(); - mi = mi.withSelect(new J.Identifier(UUID.randomUUID(), mi.getSelect().getPrefix(), Markers.EMPTY, fq.getClassName(), fq, null)); + mi = mi.withSelect(new J.Identifier(UUID.randomUUID(), mi.getSelect().getPrefix(), Markers.EMPTY, emptyList(), fq.getClassName(), fq, null)); //noinspection ArraysAsListWithZeroOrOneArgument mi = mi.withArguments(Arrays.asList(arg)); } @@ -112,7 +114,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu if (arg != null && !TypeUtils.isString(arg.getType()) && mi.getSelect() != null) { JavaType.FullyQualified fq = mi.getMethodType().getDeclaringType(); - mi = mi.withSelect(new J.Identifier(UUID.randomUUID(), mi.getSelect().getPrefix(), Markers.EMPTY, fq.getClassName(), fq, null)); + mi = mi.withSelect(new J.Identifier(UUID.randomUUID(), mi.getSelect().getPrefix(), Markers.EMPTY, emptyList(), fq.getClassName(), fq, null)); mi = mi.withArguments(ListUtils.concat(arg, mi.getArguments())); mi = maybeAutoFormat(mi, mi.withName(mi.getName().withSimpleName("compare")), executionContext); } diff --git a/src/main/java/org/openrewrite/staticanalysis/ReferentialEqualityToObjectEquals.java b/src/main/java/org/openrewrite/staticanalysis/ReferentialEqualityToObjectEquals.java index 9365a25e8..a850a846d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReferentialEqualityToObjectEquals.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReferentialEqualityToObjectEquals.java @@ -29,6 +29,7 @@ import java.util.Optional; import java.util.Set; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; public class ReferentialEqualityToObjectEquals extends Recipe { @@ -70,7 +71,7 @@ private static J.MethodInvocation asEqualsMethodInvocation(J.Binary binary, @Nul Markers.EMPTY, new JRightPadded<>(binary.getLeft().withPrefix(Space.EMPTY), Space.EMPTY, Markers.EMPTY), null, - new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, "equals", JavaType.Primitive.Boolean, null), + new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), "equals", JavaType.Primitive.Boolean, null), JContainer.build(singletonList(new JRightPadded<>(binary.getRight().withPrefix(Space.EMPTY), Space.EMPTY, Markers.EMPTY))), new JavaType.Method( null, diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java index af1b0cdef..d590d0f30 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java @@ -80,19 +80,19 @@ public TreeVisitor getVisitor() { private Cursor getCursorToParentScope(Cursor cursor) { return cursor.dropParentUntil(is -> is instanceof J.ClassDeclaration || - is instanceof J.Block || - is instanceof J.MethodDeclaration || - is instanceof J.ForLoop || - is instanceof J.ForEachLoop || - is instanceof J.ForLoop.Control || - is instanceof J.ForEachLoop.Control || - is instanceof J.Case || - is instanceof J.Try || - is instanceof J.Try.Resource || - is instanceof J.Try.Catch || - is instanceof J.MultiCatch || - is instanceof J.Lambda || - is instanceof JavaSourceFile + is instanceof J.Block || + is instanceof J.MethodDeclaration || + is instanceof J.ForLoop || + is instanceof J.ForEachLoop || + is instanceof J.ForLoop.Control || + is instanceof J.ForEachLoop.Control || + is instanceof J.Case || + is instanceof J.Try || + is instanceof J.Try.Resource || + is instanceof J.Try.Catch || + is instanceof J.MultiCatch || + is instanceof J.Lambda || + is instanceof JavaSourceFile ); } @@ -106,20 +106,22 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations Cursor parentScope = getCursorToParentScope(getCursor()); J parent = parentScope.getValue(); if (parentScope.getParent() == null || - // skip class instance variables. parentScope.getValue() covers java records. - parentScope.getParent().getValue() instanceof J.ClassDeclaration || parentScope.getValue() instanceof J.ClassDeclaration || - // skip anonymous class instance variables - parentScope.getParent().getValue() instanceof J.NewClass || - // skip if method declaration parameter - parent instanceof J.MethodDeclaration || - // skip if defined in an enhanced or standard for loop, since there isn't much we can do about the semantics at that point - parent instanceof J.ForLoop.Control || parent instanceof J.ForEachLoop.Control || - // skip if defined in a try's catch clause as an Exception variable declaration - parent instanceof J.Try.Resource || parent instanceof J.Try.Catch || parent instanceof J.MultiCatch || - // skip if defined as a parameter to a lambda expression - parent instanceof J.Lambda || - // skip if the initializer may have a side effect - initializerMightSideEffect(variable) + // skip class instance variables. parentScope.getValue() covers java records. + parentScope.getParent().getValue() instanceof J.ClassDeclaration || parentScope.getValue() instanceof J.ClassDeclaration || + // skip anonymous class instance variables + parentScope.getParent().getValue() instanceof J.NewClass || + // skip if method declaration parameter + parent instanceof J.MethodDeclaration || + // skip if defined in an enhanced or standard for loop, since there isn't much we can do about the semantics at that point + parent instanceof J.ForLoop.Control || parent instanceof J.ForEachLoop.Control || + // skip if defined in a switch case + parent instanceof J.Case || + // skip if defined in a try's catch clause as an Exception variable declaration + parent instanceof J.Try.Resource || parent instanceof J.Try.Catch || parent instanceof J.MultiCatch || + // skip if defined as a parameter to a lambda expression + parent instanceof J.Lambda || + // skip if the initializer may have a side effect + initializerMightSideEffect(variable) ) { return variable; } @@ -179,6 +181,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat return methodInvocation; } + @Override + public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean result) { + result.set(true); + return newClass; + } + @Override public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean result) { result.set(true); @@ -219,9 +227,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation m, ExecutionC @EqualsAndHashCode(callSuper = true) private static class AssignmentToLiteral extends JavaVisitor { J.Assignment assignment; + @Override public J visitAssignment(J.Assignment a, ExecutionContext executionContext) { - if(assignment.isScope(a)) { + if (assignment.isScope(a)) { return a.getAssignment().withPrefix(a.getPrefix()); } return a; diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java index 773f11219..0251b1819 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java @@ -24,6 +24,7 @@ import java.time.Duration; import java.util.Collections; +import java.util.List; import java.util.Set; import static org.openrewrite.internal.NameCaseConvention.LOWER_CAMEL; @@ -79,35 +80,64 @@ protected boolean shouldRename(Set hasNameKey, J.VariableDeclarations.Na return !hasNameKey.contains(toName); } - @SuppressWarnings("all") @Override - public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { - Cursor parentScope = getCursorToParentScope(getCursor()); + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx); + // the meaning of a local variable is “is contained in a method declaration body”. + if (!isLocalVariable(mv)) { + return mv; + } - // Does not currently support renaming fields in a J.ClassDeclaration. - if (!(parentScope.getParent() != null && - (parentScope.getParent().getValue() instanceof J.ClassDeclaration || - // Detect java records - parentScope.getValue() instanceof J.ClassDeclaration)) && - // Does not apply for instance variables of anonymous inner classes - !(parentScope.getParent().getValue() instanceof J.NewClass) && - // Does not apply to for loop controls. - !(parentScope.getValue() instanceof J.ForLoop.Control) && - // Does not apply to catches with 1 character. - !((parentScope.getValue() instanceof J.Try.Catch || parentScope.getValue() instanceof J.MultiCatch) && variable.getSimpleName().length() == 1)) { - - if (!LOWER_CAMEL.matches(variable.getSimpleName())) { - String toName = LOWER_CAMEL.format(variable.getSimpleName()); - renameVariable(variable, toName); + List variables = mv.getVariables(); + for (J.VariableDeclarations.NamedVariable v : variables) { + String name = v.getSimpleName(); + if (!LOWER_CAMEL.matches(name) && name.length() > 1) { + renameVariable(v, LOWER_CAMEL.format(name)); } else { - hasNameKey(variable.getSimpleName()); + hasNameKey(name); } } + return mv; + } + + private boolean isLocalVariable(J.VariableDeclarations mv) { + // The recipe will not rename variables declared in for loop controls or catches. + if (!isInMethodDeclarationBody() || isDeclaredInForLoopControl() || isDeclaredInCatch()) { + return false; + } - return variable; + // Skip constant variable + if (mv.hasModifier(J.Modifier.Type.Final)) { + return false; + } + + // Ignore fields (aka "instance variable" or "class variable") + for (J.VariableDeclarations.NamedVariable v : mv.getVariables()) { + if (v.isField(getCursor())) { + return false; + } + } + + return true; + } + + private boolean isInMethodDeclarationBody() { + return getCursor().dropParentUntil(p -> p instanceof J.MethodDeclaration || + p instanceof J.ClassDeclaration || + p instanceof J.NewClass || + p == Cursor.ROOT_VALUE).getValue() instanceof J.MethodDeclaration; + } + + private boolean isDeclaredInForLoopControl() { + return getCursor().getParentTreeCursor() + .getValue() instanceof J.ForLoop.Control; + } + + private boolean isDeclaredInCatch() { + Cursor parentScope = getCursorToParentScope(getCursor()); + return parentScope.getValue() instanceof J.Try.Catch || parentScope.getValue() instanceof J.MultiCatch; } - @SuppressWarnings("all") @Override public J.Identifier visitIdentifier(J.Identifier identifier, ExecutionContext ctx) { hasNameKey(identifier.getSimpleName()); diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java index 7361a4d4a..437a64087 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java @@ -124,9 +124,9 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct // Temporary work around due to an issue in the JavaTemplate related to BlockStatementTemplateGenerator#enumClassDeclaration. Space singleSpace = Space.build(" ", emptyList()); Expression literal = duplicateLiteralsMap.get(valueOfLiteral).toArray(new J.Literal[0])[0].withId(randomId()); - J.Modifier privateModifier = new J.Modifier(randomId(), Space.build("\n", emptyList()), Markers.EMPTY, J.Modifier.Type.Private, emptyList()); - J.Modifier staticModifier = new J.Modifier(randomId(), singleSpace, Markers.EMPTY, J.Modifier.Type.Static, emptyList()); - J.Modifier finalModifier = new J.Modifier(randomId(), singleSpace, Markers.EMPTY, J.Modifier.Type.Final, emptyList()); + J.Modifier privateModifier = new J.Modifier(randomId(), Space.build("\n", emptyList()), Markers.EMPTY, null, J.Modifier.Type.Private, emptyList()); + J.Modifier staticModifier = new J.Modifier(randomId(), singleSpace, Markers.EMPTY, null, J.Modifier.Type.Static, emptyList()); + J.Modifier finalModifier = new J.Modifier(randomId(), singleSpace, Markers.EMPTY, null, J.Modifier.Type.Final, emptyList()); J.VariableDeclarations variableDeclarations = autoFormat(new J.VariableDeclarations( randomId(), Space.EMPTY, @@ -137,6 +137,7 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct randomId(), singleSpace, Markers.EMPTY, + emptyList(), "String", JavaType.ShallowClass.build("java.lang.String"), null), @@ -150,6 +151,7 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct randomId(), Space.EMPTY, Markers.EMPTY, + emptyList(), variableName, JavaType.ShallowClass.build("java.lang.String"), null), @@ -364,6 +366,7 @@ public J visitLiteral(J.Literal literal, ExecutionContext ctx) { Tree.randomId(), literal.getPrefix(), literal.getMarkers(), + emptyList(), variableName, JavaType.Primitive.String, new JavaType.Variable( diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReference.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReference.java index dfd3f0a0c..9637cf582 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReference.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReference.java @@ -15,14 +15,14 @@ */ package org.openrewrite.staticanalysis; -import org.openrewrite.Cursor; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; +import org.openrewrite.*; +import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.ShortenFullyQualifiedTypeReferences; import org.openrewrite.java.tree.*; +import org.openrewrite.kotlin.KotlinVisitor; +import org.openrewrite.kotlin.tree.K; import java.time.Duration; import java.util.*; @@ -54,190 +54,208 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { - return new JavaVisitor() { + return new TreeVisitor() { @Override - public J visitLambda(J.Lambda lambda, ExecutionContext executionContext) { - J.Lambda l = (J.Lambda) super.visitLambda(lambda, executionContext); - updateCursor(l); - - if (TypeUtils.isOfClassType(lambda.getType(), "groovy.lang.Closure")) { - return l; + public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx, Cursor parent) { + if (tree instanceof J.CompilationUnit) { + return new ReplaceLambdaWithMethodReferenceJavaVisitor().visit(tree, ctx); + } else if (tree instanceof K.CompilationUnit) { + return new ReplaceLambdaWithMethodReferenceKotlinVisitor().visit(tree, ctx); } + return tree; + } + }; + } + + private static class ReplaceLambdaWithMethodReferenceKotlinVisitor extends KotlinVisitor { + // Implement Me + } - String code = ""; - J body = l.getBody(); - if (body instanceof J.Block && ((J.Block) body).getStatements().size() == 1) { - Statement statement = ((J.Block) body).getStatements().get(0); - if (statement instanceof J.MethodInvocation) { - body = statement; - } else if (statement instanceof J.Return && - (((J.Return) statement).getExpression()) instanceof MethodCall) { - body = ((J.Return) statement).getExpression(); + private static class ReplaceLambdaWithMethodReferenceJavaVisitor extends JavaVisitor { + @Override + public J visitLambda(J.Lambda lambda, ExecutionContext executionContext) { + J.Lambda l = (J.Lambda) super.visitLambda(lambda, executionContext); + updateCursor(l); + + String code = ""; + J body = l.getBody(); + if (body instanceof J.Block && ((J.Block) body).getStatements().size() == 1) { + Statement statement = ((J.Block) body).getStatements().get(0); + if (statement instanceof J.MethodInvocation) { + body = statement; + } else if (statement instanceof J.Return && + (((J.Return) statement).getExpression()) instanceof MethodCall) { + body = ((J.Return) statement).getExpression(); + } + } else if (body instanceof J.InstanceOf) { + J.InstanceOf instanceOf = (J.InstanceOf) body; + J j = instanceOf.getClazz(); + if ((j instanceof J.Identifier || j instanceof J.FieldAccess) && + instanceOf.getExpression() instanceof J.Identifier) { + J.FieldAccess classLiteral = newClassLiteral(((TypeTree) j).getType(), j instanceof J.FieldAccess); + if (classLiteral != null) { + //noinspection DataFlowIssue + JavaType.FullyQualified rawClassType = ((JavaType.Parameterized) classLiteral.getType()).getType(); + Optional isInstanceMethod = rawClassType.getMethods().stream().filter(m -> m.getName().equals("isInstance")).findFirst(); + if (isInstanceMethod.isPresent()) { + return newInstanceMethodReference(isInstanceMethod.get(), classLiteral, lambda.getType()).withPrefix(lambda.getPrefix()); + } } - } else if (body instanceof J.InstanceOf) { - J.InstanceOf instanceOf = (J.InstanceOf) body; - J j = instanceOf.getClazz(); - if ((j instanceof J.Identifier || j instanceof J.FieldAccess) && - instanceOf.getExpression() instanceof J.Identifier) { - J.FieldAccess classLiteral = newClassLiteral(((TypeTree) j).getType(), j instanceof J.FieldAccess); + } + } else if (body instanceof J.TypeCast) { + if (!(((J.TypeCast) body).getExpression() instanceof J.MethodInvocation)) { + J.ControlParentheses j = ((J.TypeCast) body).getClazz(); + J tree = j.getTree(); + if ((tree instanceof J.Identifier || tree instanceof J.FieldAccess) && + !(j.getType() instanceof JavaType.GenericTypeVariable)) { + J.FieldAccess classLiteral = newClassLiteral(((Expression) tree).getType(), tree instanceof J.FieldAccess); if (classLiteral != null) { //noinspection DataFlowIssue - JavaType.FullyQualified rawClassType = ((JavaType.Parameterized) classLiteral.getType()).getType(); - Optional isInstanceMethod = rawClassType.getMethods().stream().filter(m -> m.getName().equals("isInstance")).findFirst(); - if (isInstanceMethod.isPresent()) { - return newInstanceMethodReference(isInstanceMethod.get(), classLiteral, lambda.getType()).withPrefix(lambda.getPrefix()); + JavaType.FullyQualified classType = ((JavaType.Parameterized) classLiteral.getType()).getType(); + Optional castMethod = classType.getMethods().stream().filter(m -> m.getName().equals("cast")).findFirst(); + if (castMethod.isPresent()) { + return newInstanceMethodReference(castMethod.get(), classLiteral, lambda.getType()).withPrefix(lambda.getPrefix()); } } } - } else if (body instanceof J.TypeCast) { - if (!(((J.TypeCast) body).getExpression() instanceof J.MethodInvocation)) { - J.ControlParentheses j = ((J.TypeCast) body).getClazz(); - J tree = j.getTree(); - if ((tree instanceof J.Identifier || tree instanceof J.FieldAccess) && - !(j.getType() instanceof JavaType.GenericTypeVariable)) { - J.FieldAccess classLiteral = newClassLiteral(((Expression) tree).getType(), tree instanceof J.FieldAccess); - if (classLiteral != null) { - //noinspection DataFlowIssue - JavaType.FullyQualified classType = ((JavaType.Parameterized) classLiteral.getType()).getType(); - Optional castMethod = classType.getMethods().stream().filter(m -> m.getName().equals("cast")).findFirst(); - if (castMethod.isPresent()) { - return newInstanceMethodReference(castMethod.get(), classLiteral, lambda.getType()).withPrefix(lambda.getPrefix()); - } + } + } + + if (body instanceof J.Binary) { + J.Binary binary = (J.Binary) body; + if (isNullCheck(binary.getLeft(), binary.getRight()) || + isNullCheck(binary.getRight(), binary.getLeft())) { + doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()); + code = J.Binary.Type.Equal.equals(binary.getOperator()) ? "java.util.Objects::isNull" : + "java.util.Objects::nonNull"; + return JavaTemplate.builder(code) + .contextSensitive() + .build() + .apply(getCursor(), l.getCoordinates().replace()); + } + } else if (body instanceof MethodCall) { + MethodCall method = (MethodCall) body; + if (method instanceof J.NewClass) { + J.NewClass nc = (J.NewClass) method; + if (nc.getBody() != null) { + return l; + } else { + if (isAMethodInvocationArgument(l, getCursor()) && nc.getType() instanceof JavaType.Class) { + JavaType.Class clazz = (JavaType.Class) nc.getType(); + boolean hasMultipleConstructors = clazz.getMethods().stream().filter(JavaType.Method::isConstructor).count() > 1; + if (hasMultipleConstructors) { + return l; } } } } - if (body instanceof J.Binary) { - J.Binary binary = (J.Binary) body; - if (isNullCheck(binary.getLeft(), binary.getRight()) || - isNullCheck(binary.getRight(), binary.getLeft())) { + if (multipleMethodInvocations(method) || + !methodArgumentsMatchLambdaParameters(method, lambda) || + method instanceof J.MemberReference) { + return l; + } + + Expression select = + method instanceof J.MethodInvocation ? ((J.MethodInvocation) method).getSelect() : null; + JavaType.Method methodType = method.getMethodType(); + if (methodType != null && !isMethodReferenceAmbiguous(methodType)) { + if (methodType.hasFlags(Flag.Static) || + methodSelectMatchesFirstLambdaParameter(method, lambda)) { doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()); - code = J.Binary.Type.Equal.equals(binary.getOperator()) ? "java.util.Objects::isNull" : - "java.util.Objects::nonNull"; - return JavaTemplate.builder(code) + return newStaticMethodReference(methodType, true, lambda.getType()).withPrefix(lambda.getPrefix()); + } else if (method instanceof J.NewClass) { + return JavaTemplate.builder("#{}::new") .contextSensitive() .build() - .apply(getCursor(), l.getCoordinates().replace()); - } - } else if (body instanceof MethodCall) { - MethodCall method = (MethodCall) body; - if (method instanceof J.NewClass) { - J.NewClass nc = (J.NewClass) method; - if (nc.getBody() != null) { - return l; - } else { - if (isAMethodInvocationArgument(l, getCursor()) && nc.getType() instanceof JavaType.Class) { - JavaType.Class clazz = (JavaType.Class) nc.getType(); - boolean hasMultipleConstructors = clazz.getMethods().stream().filter(JavaType.Method::isConstructor).count() > 1; - if (hasMultipleConstructors) { - return l; - } - } - } + .apply(getCursor(), l.getCoordinates().replace(), className((J.NewClass) method)); + } else if (select != null) { + return newInstanceMethodReference(methodType, select, lambda.getType()).withPrefix(lambda.getPrefix()); + } else { + String templ = "#{}::#{}"; + return JavaTemplate.builder(templ) + .contextSensitive() + .build() + .apply(getCursor(), l.getCoordinates().replace(), "this", + method.getMethodType().getName()); } + } + } - if (multipleMethodInvocations(method) || - !methodArgumentsMatchLambdaParameters(method, lambda) || - method instanceof J.MemberReference) { - return l; - } + return l; + } - Expression select = - method instanceof J.MethodInvocation ? ((J.MethodInvocation) method).getSelect() : null; - JavaType.Method methodType = method.getMethodType(); - if (methodType != null) { - if (methodType.hasFlags(Flag.Static) || - methodSelectMatchesFirstLambdaParameter(method, lambda)) { - doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()); - - return newStaticMethodReference(methodType, true, lambda.getType()).withPrefix(lambda.getPrefix()); - } else if (method instanceof J.NewClass) { - return JavaTemplate.builder("#{}::new") - .contextSensitive() - .build() - .apply(getCursor(), l.getCoordinates().replace(), className((J.NewClass) method)); - } else if (select != null) { - return newInstanceMethodReference(methodType, select, lambda.getType()).withPrefix(lambda.getPrefix()); - } else { - String templ = "#{}::#{}"; - return JavaTemplate.builder(templ) - .contextSensitive() - .build() - .apply(getCursor(), l.getCoordinates().replace(), "this", - method.getMethodType().getName()); - } - } - } + // returns the class name as given in the source code (qualified or unqualified) + private String className(J.NewClass method) { + TypeTree clazz = method.getClazz(); + return clazz instanceof J.ParameterizedType ? ((J.ParameterizedType) clazz).getClazz().toString() : + Objects.toString(clazz); + } - return l; - } + private boolean multipleMethodInvocations(MethodCall method) { + return method instanceof J.MethodInvocation && + ((J.MethodInvocation) method).getSelect() instanceof J.MethodInvocation; + } - // returns the class name as given in the source code (qualified or unqualified) - private String className(J.NewClass method) { - TypeTree clazz = method.getClazz(); - return clazz instanceof J.ParameterizedType ? ((J.ParameterizedType) clazz).getClazz().toString() : - Objects.toString(clazz); + private boolean methodArgumentsMatchLambdaParameters(MethodCall method, J.Lambda lambda) { + JavaType.Method methodType = method.getMethodType(); + if (methodType == null) { + return false; } - - private boolean multipleMethodInvocations(MethodCall method) { - return method instanceof J.MethodInvocation && - ((J.MethodInvocation) method).getSelect() instanceof J.MethodInvocation; + boolean static_ = methodType.hasFlags(Flag.Static); + List methodArgs = method.getArguments().stream().filter(a -> !(a instanceof J.Empty)) + .collect(Collectors.toList()); + List lambdaParameters = lambda.getParameters().getParameters() + .stream().filter(J.VariableDeclarations.class::isInstance) + .map(J.VariableDeclarations.class::cast).map(v -> v.getVariables().get(0)) + .collect(Collectors.toList()); + if (methodArgs.isEmpty() && lambdaParameters.isEmpty()) { + return true; } - - private boolean methodArgumentsMatchLambdaParameters(MethodCall method, J.Lambda lambda) { - JavaType.Method methodType = method.getMethodType(); - if (methodType == null) { + if (!static_ && methodSelectMatchesFirstLambdaParameter(method, lambda)) { + methodArgs.add(0, ((J.MethodInvocation) method).getSelect()); + } + if (methodArgs.size() != lambdaParameters.size()) { + return false; + } + for (int i = 0; i < lambdaParameters.size(); i++) { + JavaType lambdaParam = lambdaParameters.get(i).getVariableType(); + if (!(methodArgs.get(i) instanceof J.Identifier)) { return false; } - boolean static_ = methodType.hasFlags(Flag.Static); - List methodArgs = method.getArguments().stream().filter(a -> !(a instanceof J.Empty)) - .collect(Collectors.toList()); - List lambdaParameters = lambda.getParameters().getParameters() - .stream().filter(J.VariableDeclarations.class::isInstance) - .map(J.VariableDeclarations.class::cast).map(v -> v.getVariables().get(0)) - .collect(Collectors.toList()); - if (methodArgs.isEmpty() && lambdaParameters.isEmpty()) { - return true; - } - if (!static_ && methodSelectMatchesFirstLambdaParameter(method, lambda)) { - methodArgs.add(0, ((J.MethodInvocation) method).getSelect()); - } - if (methodArgs.size() != lambdaParameters.size()) { + JavaType methodArgument = ((J.Identifier) methodArgs.get(i)).getFieldType(); + if (lambdaParam != methodArgument) { return false; } - for (int i = 0; i < lambdaParameters.size(); i++) { - JavaType lambdaParam = lambdaParameters.get(i).getVariableType(); - if (!(methodArgs.get(i) instanceof J.Identifier)) { - return false; - } - JavaType methodArgument = ((J.Identifier) methodArgs.get(i)).getFieldType(); - if (lambdaParam != methodArgument) { - return false; - } - } - return true; } + return true; + } - private boolean methodSelectMatchesFirstLambdaParameter(MethodCall method, J.Lambda lambda) { - if (!(method instanceof J.MethodInvocation) || - !(((J.MethodInvocation) method).getSelect() instanceof J.Identifier) || - lambda.getParameters().getParameters().isEmpty() || - !(lambda.getParameters().getParameters().get(0) instanceof J.VariableDeclarations)) { - return false; - } - J.VariableDeclarations firstLambdaParameter = (J.VariableDeclarations) lambda.getParameters() - .getParameters().get(0); - return ((J.Identifier) ((J.MethodInvocation) method).getSelect()).getFieldType() == - firstLambdaParameter.getVariables().get(0).getVariableType(); + private boolean methodSelectMatchesFirstLambdaParameter(MethodCall method, J.Lambda lambda) { + if (!(method instanceof J.MethodInvocation) || + !(((J.MethodInvocation) method).getSelect() instanceof J.Identifier) || + lambda.getParameters().getParameters().isEmpty() || + !(lambda.getParameters().getParameters().get(0) instanceof J.VariableDeclarations)) { + return false; } + J.VariableDeclarations firstLambdaParameter = (J.VariableDeclarations) lambda.getParameters() + .getParameters().get(0); + return ((J.Identifier) ((J.MethodInvocation) method).getSelect()).getFieldType() == + firstLambdaParameter.getVariables().get(0).getVariableType(); + } - private boolean isNullCheck(J j1, J j2) { - return j1 instanceof J.Identifier && j2 instanceof J.Literal && - "null".equals(((J.Literal) j2).getValueSource()); - } - }; + private boolean isNullCheck(J j1, J j2) { + return j1 instanceof J.Identifier && j2 instanceof J.Literal && + "null".equals(((J.Literal) j2).getValueSource()); + } + private boolean isMethodReferenceAmbiguous(JavaType.Method _method) { + return _method.getDeclaringType().getMethods().stream() + .filter(meth -> meth.getName().equals(_method.getName())) + .filter(meth -> !meth.getName().equals("println")) + .filter(meth -> !meth.isConstructor()) + .count() > 1; + } } private static boolean isAMethodInvocationArgument(J.Lambda lambda, Cursor cursor) { diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java index 5e361ecc9..7ce1cf6e0 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceStackWithDeque.java @@ -18,11 +18,10 @@ import org.openrewrite.*; import org.openrewrite.analysis.dataflow.DataFlowNode; import org.openrewrite.analysis.dataflow.FindLocalFlowPaths; -import org.openrewrite.analysis.dataflow.LocalFlowSpec; +import org.openrewrite.analysis.dataflow.DataFlowSpec; import org.openrewrite.java.ChangeType; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; public class ReplaceStackWithDeque extends Recipe { @@ -44,7 +43,7 @@ public TreeVisitor getVisitor() { public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { J.VariableDeclarations.NamedVariable v = super.visitVariable(variable, ctx); - LocalFlowSpec returned = new LocalFlowSpec() { + DataFlowSpec returned = new DataFlowSpec() { @Override public boolean isSource(DataFlowNode srcNode) { return variable.getInitializer() == srcNode.getCursor().getValue(); diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceWeekYearWithYear.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceWeekYearWithYear.java new file mode 100644 index 000000000..6262fc2c9 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceWeekYearWithYear.java @@ -0,0 +1,124 @@ +/* + * Copyright 2023 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.*; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.*; + +import java.util.*; + +public class ReplaceWeekYearWithYear extends Recipe { + private static final MethodMatcher SIMPLE_DATE_FORMAT_CONSTRUCTOR_MATCHER = new MethodMatcher("java.text.SimpleDateFormat (..)"); + private static final MethodMatcher OF_PATTERN_MATCHER = new MethodMatcher("java.time.format.DateTimeFormatter ofPattern(..)"); + + @Override + public String getDisplayName() { + return "Week Year (YYYY) should not be used for date formatting"; + } + + @Override + public String getDescription() { + return "For most dates Week Year (YYYY) and Year (yyyy) yield the same results. However, on the last week of" + + " December and first week of January Week Year could produce unexpected results."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-3986"); + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check( + Preconditions.or( + new UsesType<>("java.util.Date", false), + new UsesType<>("java.time.format.DateTimeFormatter", false), + new UsesType<>("java.text.SimpleDateFormat", false) + ), + new ReplaceWeekYearVisitor() + ); + } + + private static class ReplaceWeekYearVisitor extends JavaIsoVisitor { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) { + if (OF_PATTERN_MATCHER.matches(mi)) { + getCursor().putMessage("KEY", mi); + } + + return super.visitMethodInvocation(mi, ctx); + } + + @Override + public J.NewClass visitNewClass(J.NewClass nc, ExecutionContext ctx) { + if (SIMPLE_DATE_FORMAT_CONSTRUCTOR_MATCHER.matches(nc)) { + getCursor().putMessage("KEY", nc); + } + + return super.visitNewClass(nc, ctx); + } + + @Override + public J.Literal visitLiteral(J.Literal li, ExecutionContext ctx) { + if (li.getValue() instanceof String) { + Cursor c = getCursor().dropParentWhile(is -> is instanceof J.Parentheses || !(is instanceof Tree)); + if (c.getMessage("KEY") != null) { + Object value = li.getValue(); + + if (value == null) { + return li; + } + + String newValue = replaceY(value.toString()); + + if (newValue.equals(value.toString())) { + return li; + } + + return li.withValueSource("\""+newValue+"\"").withValue(newValue); + } + } + + return li; + } + + public static String replaceY(String input) { + StringBuilder output = new StringBuilder(); + boolean insideQuotes = false; + + for (int i = 0; i < input.length(); i++) { + char currentChar = input.charAt(i); + char nextChar = (i < input.length() - 1) ? input.charAt(i + 1) : '\0'; + + if (currentChar == '\'') { + insideQuotes = !insideQuotes; + output.append(currentChar); + } else if (currentChar == 'Y' && !insideQuotes) { + output.append('y'); + } else if (currentChar == 'Y' && nextChar == '\'') { + output.append(currentChar); + } else { + output.append(currentChar); + } + } + + return output.toString(); + } + } +} \ No newline at end of file diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpression.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpression.java new file mode 100644 index 000000000..26b040411 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpression.java @@ -0,0 +1,221 @@ +/* + * 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.*; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.UnwrapParentheses; +import org.openrewrite.java.format.AutoFormatVisitor; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaSourceFile; +import org.openrewrite.java.tree.Space; + +import java.time.Duration; +import java.util.Collections; +import java.util.Set; + +import static java.util.Objects.requireNonNull; + +public class SimplifyBooleanExpression extends Recipe { + + @Override + public String getDisplayName() { + return "Simplify boolean expression"; + } + + @Override + public String getDescription() { + return "Checks for over-complicated boolean expressions. Finds code like `if (b == true)`, `b || true`, `!false`, etc."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-1125"); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(5); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaVisitor() { + private static final String MAYBE_AUTO_FORMAT_ME = "MAYBE_AUTO_FORMAT_ME"; + + @Override + public J visit(@Nullable Tree tree, ExecutionContext ctx) { + if (tree instanceof JavaSourceFile) { + JavaSourceFile cu = (JavaSourceFile) requireNonNull(super.visit(tree, ctx)); + if (tree != cu) { + // recursive simplification + cu = (JavaSourceFile) getVisitor().visitNonNull(cu, ctx); + } + return cu; + } + return super.visit(tree, ctx); + } + + @Override + public J visitBinary(J.Binary binary, ExecutionContext ctx) { + J j = super.visitBinary(binary, ctx); + J.Binary asBinary = (J.Binary) j; + + if (asBinary.getOperator() == J.Binary.Type.And) { + if (isLiteralFalse(asBinary.getLeft())) { + maybeUnwrapParentheses(); + j = asBinary.getLeft(); + } else if (isLiteralFalse(asBinary.getRight())) { + maybeUnwrapParentheses(); + j = asBinary.getRight().withPrefix(asBinary.getRight().getPrefix().withWhitespace("")); + } else if (removeAllSpace(asBinary.getLeft()).printTrimmed(getCursor()) + .equals(removeAllSpace(asBinary.getRight()).printTrimmed(getCursor()))) { + maybeUnwrapParentheses(); + j = asBinary.getLeft(); + } + } else if (asBinary.getOperator() == J.Binary.Type.Or) { + if (isLiteralTrue(asBinary.getLeft())) { + maybeUnwrapParentheses(); + j = asBinary.getLeft(); + } else if (isLiteralTrue(asBinary.getRight())) { + maybeUnwrapParentheses(); + j = asBinary.getRight().withPrefix(asBinary.getRight().getPrefix().withWhitespace("")); + } else if (removeAllSpace(asBinary.getLeft()).printTrimmed(getCursor()) + .equals(removeAllSpace(asBinary.getRight()).printTrimmed(getCursor()))) { + maybeUnwrapParentheses(); + j = asBinary.getLeft(); + } + } else if (asBinary.getOperator() == J.Binary.Type.Equal) { + if (isLiteralTrue(asBinary.getLeft())) { + if (isNotKotlinNullableType(asBinary.getRight())) { + maybeUnwrapParentheses(); + j = asBinary.getRight().withPrefix(asBinary.getRight().getPrefix().withWhitespace("")); + } + } else if (isLiteralTrue(asBinary.getRight())) { + if (isNotKotlinNullableType(asBinary.getLeft())) { + maybeUnwrapParentheses(); + j = asBinary.getLeft().withPrefix(asBinary.getLeft().getPrefix().withWhitespace(" ")); + } + } + } else if (asBinary.getOperator() == J.Binary.Type.NotEqual) { + if (isLiteralFalse(asBinary.getLeft())) { + if (isNotKotlinNullableType(asBinary.getRight())) { + maybeUnwrapParentheses(); + j = asBinary.getRight().withPrefix(asBinary.getRight().getPrefix().withWhitespace("")); + } + } else if (isLiteralFalse(asBinary.getRight())) { + if (isNotKotlinNullableType(asBinary.getLeft())) { + maybeUnwrapParentheses(); + j = asBinary.getLeft().withPrefix(asBinary.getLeft().getPrefix().withWhitespace(" ")); + } + } + } + if (asBinary != j) { + getCursor().getParentTreeCursor().putMessage(MAYBE_AUTO_FORMAT_ME, ""); + } + return j; + } + + @Override + public J postVisit(J tree, ExecutionContext ctx) { + J j = super.postVisit(tree, ctx); + if (getCursor().pollMessage(MAYBE_AUTO_FORMAT_ME) != null) { + j = new AutoFormatVisitor<>().visit(j, ctx, getCursor().getParentOrThrow()); + } + return j; + } + + @Override + public J visitUnary(J.Unary unary, ExecutionContext ctx) { + J j = super.visitUnary(unary, ctx); + J.Unary asUnary = (J.Unary) j; + + if (asUnary.getOperator() == J.Unary.Type.Not) { + if (isLiteralTrue(asUnary.getExpression())) { + maybeUnwrapParentheses(); + j = ((J.Literal) asUnary.getExpression()).withValue(false).withValueSource("false"); + } else if (isLiteralFalse(asUnary.getExpression())) { + maybeUnwrapParentheses(); + j = ((J.Literal) asUnary.getExpression()).withValue(true).withValueSource("true"); + } else if (asUnary.getExpression() instanceof J.Unary && ((J.Unary) asUnary.getExpression()).getOperator() == J.Unary.Type.Not) { + maybeUnwrapParentheses(); + j = ((J.Unary) asUnary.getExpression()).getExpression(); + } + } + if (asUnary != j) { + getCursor().getParentTreeCursor().putMessage(MAYBE_AUTO_FORMAT_ME, ""); + } + return j; + } + + /** + * Specifically for removing immediately-enclosing parentheses on Identifiers and Literals. + * This queues a potential unwrap operation for the next visit. After unwrapping something, it's possible + * there are more Simplifications this recipe can identify and perform, which is why visitCompilationUnit + * checks for any changes to the entire Compilation Unit, and if so, queues up another SimplifyBooleanExpression + * recipe call. This convergence loop eventually reconciles any remaining Boolean Expression Simplifications + * the recipe can perform. + */ + private void maybeUnwrapParentheses() { + Cursor c = getCursor().getParentOrThrow().getParentTreeCursor(); + if (c.getValue() instanceof J.Parentheses) { + doAfterVisit(new UnwrapParentheses<>(c.getValue())); + } + } + + private boolean isLiteralTrue(@Nullable Expression expression) { + return expression instanceof J.Literal && ((J.Literal) expression).getValue() == Boolean.valueOf(true); + } + + private boolean isLiteralFalse(@Nullable Expression expression) { + return expression instanceof J.Literal && ((J.Literal) expression).getValue() == Boolean.valueOf(false); + } + + private J removeAllSpace(J j) { + //noinspection ConstantConditions + return new JavaIsoVisitor() { + @Override + public Space visitSpace(Space space, Space.Location loc, Integer integer) { + return Space.EMPTY; + } + }.visit(j, 0); + } + + // Comparing Kotlin nullable type `?` with tree/false can not be simplified, + // e.g. `X?.fun() == true` is not equivalent to `X?.fun()` + private boolean isNotKotlinNullableType(@Nullable J j) { + if (j == null) { + return true; + } + + if (j instanceof J.MethodInvocation) { + J.MethodInvocation m = (J.MethodInvocation) j; + return m.getSelect() != null && !m.getSelect().getMarkers().findFirst(org.openrewrite.kotlin.marker.IsNullSafe.class).isPresent(); + } + + return true; + } + }; + } + + @Override + public boolean causesAnotherCycle() { + return true; + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java new file mode 100644 index 000000000..e72762c58 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanReturn.java @@ -0,0 +1,196 @@ +/* + * 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.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.DeleteStatement; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Statement; + +import java.time.Duration; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; + +public class SimplifyBooleanReturn extends Recipe { + + @Override + public String getDisplayName() { + return "Simplify boolean return"; + } + + @Override + public String getDescription() { + return "Simplifies Boolean expressions by removing redundancies, e.g.: `a && true` simplifies to `a`."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-1126"); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(2); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaVisitor() { + private final JavaTemplate notIfConditionReturn = JavaTemplate.builder("return !(#{any(boolean)});") + .build(); + + @Override + public J visitIf(J.If iff, ExecutionContext ctx) { + J.If i = visitAndCast(iff, ctx, super::visitIf); + + Cursor parent = getCursor().getParentTreeCursor(); + + if (parent.getValue() instanceof J.Block && + parent.getParentOrThrow().getValue() instanceof J.MethodDeclaration && + thenHasOnlyReturnStatement(iff) && + elseWithOnlyReturn(i)) { + List followingStatements = followingStatements(); + Optional singleFollowingStatement = Optional.ofNullable(followingStatements.isEmpty() ? null : followingStatements.get(0)) + .flatMap(stat -> Optional.ofNullable(stat instanceof J.Return ? (J.Return) stat : null)) + .map(J.Return::getExpression); + + if (followingStatements.isEmpty() || singleFollowingStatement.map(r -> isLiteralFalse(r) || isLiteralTrue(r)).orElse(false)) { + J.Return return_ = getReturnIfOnlyStatementInThen(iff).orElse(null); + assert return_ != null; + + Expression ifCondition = i.getIfCondition().getTree(); + + if (isLiteralTrue(return_.getExpression())) { + if (singleFollowingStatement.map(this::isLiteralFalse).orElse(false) && i.getElsePart() == null) { + doAfterVisit(new DeleteStatement<>(followingStatements().get(0))); + return maybeAutoFormat(return_, return_.withExpression(ifCondition), ctx, parent); + } else if (!singleFollowingStatement.isPresent() && + getReturnExprIfOnlyStatementInElseThen(i).map(this::isLiteralFalse).orElse(false)) { + if (i.getElsePart() != null) { + doAfterVisit(new DeleteStatement<>(i.getElsePart().getBody())); + } + return maybeAutoFormat(return_, return_.withExpression(ifCondition), ctx, parent); + } + } else if (isLiteralFalse(return_.getExpression())) { + boolean returnThenPart = false; + + if (singleFollowingStatement.map(this::isLiteralTrue).orElse(false) && i.getElsePart() == null) { + doAfterVisit(new DeleteStatement<>(followingStatements().get(0))); + returnThenPart = true; + } else if (!singleFollowingStatement.isPresent() && getReturnExprIfOnlyStatementInElseThen(i) + .map(this::isLiteralTrue).orElse(false)) { + if (i.getElsePart() != null) { + doAfterVisit(new DeleteStatement<>(i.getElsePart().getBody())); + } + returnThenPart = true; + } + + if (returnThenPart) { + // we need to NOT the expression inside the if condition + return notIfConditionReturn.apply(updateCursor(i), i.getCoordinates().replace(), ifCondition); + } + } + } + } + + return i; + } + + private boolean elseWithOnlyReturn(J.If i) { + return i.getElsePart() == null || !(i.getElsePart().getBody() instanceof J.If); + } + + private boolean thenHasOnlyReturnStatement(J.If iff) { + return getReturnIfOnlyStatementInThen(iff) + .map(return_ -> isLiteralFalse(return_.getExpression()) || isLiteralTrue(return_.getExpression())) + .orElse(false); + } + + private List followingStatements() { + J.Block block = getCursor().getParentTreeCursor().getValue(); + AtomicBoolean dropWhile = new AtomicBoolean(false); + return block.getStatements().stream() + .filter(s -> { + dropWhile.set(dropWhile.get() || s == getCursor().getValue()); + return dropWhile.get(); + }) + .skip(1) + .collect(Collectors.toList()); + } + + private boolean isLiteralTrue(@Nullable J tree) { + return tree instanceof J.Literal && ((J.Literal) tree).getValue() == Boolean.valueOf(true); + } + + private boolean isLiteralFalse(@Nullable J tree) { + return tree instanceof J.Literal && ((J.Literal) tree).getValue() == Boolean.valueOf(false); + } + + private Optional getReturnIfOnlyStatementInThen(J.If iff) { + if (iff.getThenPart() instanceof J.Return) { + return Optional.of((J.Return) iff.getThenPart()); + } + if (iff.getThenPart() instanceof J.Block) { + J.Block then = (J.Block) iff.getThenPart(); + if (then.getStatements().size() == 1 && then.getStatements().get(0) instanceof J.Return) { + return Optional.of((J.Return) then.getStatements().get(0)); + } + } + return Optional.empty(); + } + + private Optional getReturnExprIfOnlyStatementInElseThen(J.If iff2) { + if (iff2.getElsePart() == null) { + return Optional.empty(); + } + + Statement else_ = iff2.getElsePart().getBody(); + if (else_ instanceof J.Return) { + return Optional.ofNullable(((J.Return) else_).getExpression()); + } + + if (else_ instanceof J.Block) { + List statements = ((J.Block) else_).getStatements(); + if (statements.size() == 1) { + J statement = statements.get(0); + if (statement instanceof J.Return) { + return Optional.ofNullable(((J.Return) statement).getExpression()); + } + } + } + + return Optional.empty(); + } + }; + } + + @Override + public boolean causesAnotherCycle() { + return true; + } +} + diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyCompoundVisitor.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyCompoundVisitor.java index b1ef7e76c..ef512a92b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyCompoundVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyCompoundVisitor.java @@ -19,7 +19,7 @@ import org.openrewrite.Tree; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.cleanup.SimplifyBooleanExpression; +import org.openrewrite.java.cleanup.SimplifyBooleanExpressionVisitor; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JLeftPadded; @@ -70,9 +70,9 @@ public J visitAssignmentOperation(J.AssignmentOperation assignOp, ExecutionConte @SuppressWarnings("unchecked") private E cleanupBooleanExpression(E expression, ExecutionContext ctx) { - E ex1 = (E) new SimplifyBooleanExpression().getVisitor() + E ex1 = (E) new SimplifyBooleanExpressionVisitor() .visitNonNull(expression, ctx, getCursor().getParentOrThrow()); - return (E) new SimplifyBooleanExpression().getVisitor() + return (E) new SimplifyBooleanExpressionVisitor() .visitNonNull(ex1, ctx, getCursor().getParentOrThrow()); } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConsecutiveAssignments.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConsecutiveAssignments.java index c7f7d7c62..d68202f86 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConsecutiveAssignments.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConsecutiveAssignments.java @@ -20,7 +20,7 @@ import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; -import org.openrewrite.java.cleanup.UnnecessaryParentheses; +import org.openrewrite.java.cleanup.UnnecessaryParenthesesVisitor; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; @@ -89,7 +89,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { } while (combined != b); if (b != block) { - b = (J.Block) new UnnecessaryParentheses().getVisitor() + b = (J.Block) new UnnecessaryParenthesesVisitor() .visitNonNull(b, ctx, getCursor().getParentOrThrow()); } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java index ff93a9da0..b3273e3e6 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java @@ -22,8 +22,8 @@ import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.cleanup.SimplifyBooleanExpression; -import org.openrewrite.java.cleanup.UnnecessaryParentheses; +import org.openrewrite.java.cleanup.SimplifyBooleanExpressionVisitor; +import org.openrewrite.java.cleanup.UnnecessaryParenthesesVisitor; import org.openrewrite.java.style.Checkstyle; import org.openrewrite.java.style.EmptyBlockStyle; import org.openrewrite.java.tree.Expression; @@ -75,9 +75,9 @@ private E cleanupBooleanExpression( E expression, ExecutionContext context ) { E ex1 = - (E) new UnnecessaryParentheses().getVisitor() + (E) new UnnecessaryParenthesesVisitor() .visitNonNull(expression, context, getCursor().getParentOrThrow()); - ex1 = (E) new SimplifyBooleanExpression().getVisitor() + ex1 = (E) new SimplifyBooleanExpressionVisitor() .visitNonNull(ex1, context, getCursor().getParentTreeCursor()); if (expression == ex1 || isLiteralFalse(ex1) || isLiteralTrue(ex1)) { return ex1; diff --git a/src/main/java/org/openrewrite/staticanalysis/SortedSetStreamToLinkedHashSet.java b/src/main/java/org/openrewrite/staticanalysis/SortedSetStreamToLinkedHashSet.java new file mode 100644 index 000000000..66ac67b07 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/SortedSetStreamToLinkedHashSet.java @@ -0,0 +1,70 @@ +/* + * Copyright 2023 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 lombok.EqualsAndHashCode; +import lombok.Value; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.tree.J; + +@Value +@EqualsAndHashCode(callSuper = false) +public class SortedSetStreamToLinkedHashSet extends Recipe { + + @Override + public String getDisplayName() { + return "Sorted set stream should be collected to LinkedHashSet"; + } + + @Override + public String getDescription() { + return "Correct 'set.stream().sorted().collect(Collectors.toSet())' to 'set.stream().sorted().collect(LinkedHashSet::new)'."; + } + + private static final MethodMatcher STREAM_COLLECT_METHOD_MATCHER = new MethodMatcher("java.util.stream.Stream collect(java.util.stream.Collector)"); + private static final MethodMatcher STREAM_SORTED_METHOD_MATCHER = new MethodMatcher("java.util.stream.Stream sorted()"); + private static final MethodMatcher COLLECTORS_TO_SET_METHOD_MATCHER = new MethodMatcher("java.util.stream.Collectors toSet()"); + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check(new UsesMethod<>(COLLECTORS_TO_SET_METHOD_MATCHER), new JavaIsoVisitor() { + private JavaTemplate template = JavaTemplate.builder("Collectors.toCollection(LinkedHashSet::new)") + .imports("java.util.stream.Collectors", "java.util.LinkedHashSet") + .build(); + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) { + J.MethodInvocation mi = super.visitMethodInvocation(method, executionContext); + if (STREAM_COLLECT_METHOD_MATCHER.matches(mi) + && STREAM_SORTED_METHOD_MATCHER.matches(mi.getSelect()) + && COLLECTORS_TO_SET_METHOD_MATCHER.matches(mi.getArguments().get(0))) { + maybeRemoveImport("java.util.stream.Collectors.toSet"); + maybeAddImport("java.util.LinkedHashSet"); + maybeAddImport("java.util.stream.Collectors"); + return template.apply(updateCursor(mi), mi.getCoordinates().replaceArguments()); + } + return mi; + } + }); + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java b/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java index 6273afc5c..0c86c7b9b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java +++ b/src/main/java/org/openrewrite/staticanalysis/StringLiteralEquality.java @@ -20,11 +20,14 @@ 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 java.time.Duration; import java.util.Collections; import java.util.Set; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; public class StringLiteralEquality extends Recipe { @@ -53,7 +56,14 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new UsesType<>("java.lang.String", false), new JavaVisitor() { + // 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-6519/ + TreeVisitor preconditions = Preconditions.and( + Preconditions.and( + Preconditions.not(new KotlinFileChecker<>()), + Preconditions.not(new GroovyFileChecker<>())), + 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")); private final JavaType TYPE_OBJECT = JavaType.buildType("java.lang.Object"); @@ -73,7 +83,7 @@ private J.MethodInvocation asEqualsMethodInvocation(J.Binary binary) { Markers.EMPTY, new JRightPadded<>(binary.getLeft().withPrefix(Space.EMPTY), Space.EMPTY, Markers.EMPTY), null, - new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, "equals", JavaType.Primitive.Boolean, null), + new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, emptyList(), "equals", JavaType.Primitive.Boolean, null), JContainer.build(singletonList(new JRightPadded<>(binary.getRight().withPrefix(Space.EMPTY), Space.EMPTY, Markers.EMPTY))), new JavaType.Method( null, diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java index de7e805cc..5c1ffbf3e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArguments.java @@ -15,12 +15,10 @@ */ package org.openrewrite.staticanalysis; -import org.openrewrite.Cursor; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; +import org.openrewrite.*; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.tree.*; +import org.openrewrite.kotlin.tree.K; public class UnnecessaryExplicitTypeArguments extends Recipe { @@ -41,23 +39,27 @@ public TreeVisitor getVisitor() { public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { J.MethodInvocation m = super.visitMethodInvocation(method, ctx); + if (m.getTypeParameters() == null || m.getTypeParameters().isEmpty()) { + return m; + } + if (m.getMethodType() != null) { Object enclosing = getCursor().getParentTreeCursor().getValue(); JavaType enclosingType = null; - if(enclosing instanceof J.MethodInvocation) { + if (enclosing instanceof J.MethodInvocation) { // Cannot remove type parameters if it would introduce ambiguity about which method should be called J.MethodInvocation enclosingMethod = (J.MethodInvocation) enclosing; - if(enclosingMethod.getMethodType() == null) { + if (enclosingMethod.getMethodType() == null) { return m; } - if(!(enclosingMethod.getMethodType().getDeclaringType() instanceof JavaType.Class)) { + if (!(enclosingMethod.getMethodType().getDeclaringType() instanceof JavaType.Class)) { return m; } JavaType.Class declaringClass = (JavaType.Class) enclosingMethod.getMethodType().getDeclaringType(); // If there's another method on the class with the same name, skip removing type parameters // More nuanced detection of ambiguity introduction is possible - if(declaringClass.getMethods().stream() + if (declaringClass.getMethods().stream() .filter(it -> it.getName().equals(enclosingMethod.getSimpleName())) .count() > 1) { return m; @@ -86,6 +88,25 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } if (enclosingType != null && TypeUtils.isOfType(enclosingType, m.getMethodType().getReturnType())) { + boolean isKotlinFile = getCursor().dropParentUntil(it -> it instanceof K.CompilationUnit || + it == Cursor.ROOT_VALUE) + .getValue() instanceof K.CompilationUnit; + + if (isKotlinFile) { + // For Kotlin, avoid omitting explicit type arguments only when the method invocation includes + // arguments, as the compiler cannot perform type inference without the presence of arguments. + boolean hasArguments = false; + for (Expression arg : m.getArguments()) { + if (!(arg instanceof J.Empty)) { + hasArguments = true; + break; + } + } + + if (!hasArguments) { + return m; + } + } m = m.withTypeParameters(null); } } diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryParentheses.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryParentheses.java new file mode 100644 index 000000000..efc5e6215 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryParentheses.java @@ -0,0 +1,227 @@ +/* + * 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.*; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.UnwrapParentheses; +import org.openrewrite.java.style.Checkstyle; +import org.openrewrite.java.style.UnnecessaryParenthesesStyle; +import org.openrewrite.java.tree.*; + +import java.time.Duration; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.Set; + +public class UnnecessaryParentheses extends Recipe { + + @Override + public String getDisplayName() { + return "Remove unnecessary parentheses"; + } + + @Override + public String getDescription() { + return "Removes unnecessary parentheses from code where extra parentheses pairs are redundant."; + } + + @Override + public Set getTags() { + return new LinkedHashSet<>(Arrays.asList("RSPEC-1110", "RSPEC-1611")); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(1); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaVisitor() { + + @Override + public boolean isAcceptable(SourceFile sourceFile, ExecutionContext executionContext) { + // Causes problems on other languages like JavaScript + return sourceFile instanceof J.CompilationUnit; + } + + private static final String UNNECESSARY_PARENTHESES_MESSAGE = "unnecessaryParenthesesUnwrapTarget"; + + transient UnnecessaryParenthesesStyle style; + + private UnnecessaryParenthesesStyle getStyle() { + if (style == null) { + JavaSourceFile cu = getCursor().firstEnclosingOrThrow(JavaSourceFile.class); + style = ((SourceFile) cu).getStyle(UnnecessaryParenthesesStyle.class, Checkstyle.unnecessaryParentheses()); + } + return style; + } + + @Override + public J visitParentheses(J.Parentheses parens, ExecutionContext ctx) { + J par = super.visitParentheses(parens, ctx); + Cursor c = getCursor().pollNearestMessage(UNNECESSARY_PARENTHESES_MESSAGE); + if (c != null && (c.getValue() instanceof J.Literal || c.getValue() instanceof J.Identifier)) { + par = new UnwrapParentheses<>((J.Parentheses) par).visit(par, ctx, getCursor().getParentOrThrow()); + } + + assert par != null; + if (par instanceof J.Parentheses) { + if (getCursor().getParentTreeCursor().getValue() instanceof J.Parentheses) { + return ((J.Parentheses) par).getTree().withPrefix(Space.EMPTY); + } + } + return par; + } + + @Override + public J visitIdentifier(J.Identifier ident, ExecutionContext ctx) { + J.Identifier i = (J.Identifier) super.visitIdentifier(ident, ctx); + if (getStyle().getIdent() && getCursor().getParentTreeCursor().getValue() instanceof J.Parentheses) { + getCursor().putMessageOnFirstEnclosing(J.Parentheses.class, UNNECESSARY_PARENTHESES_MESSAGE, getCursor()); + } + return i; + } + + @Override + public J visitLiteral(J.Literal literal, ExecutionContext ctx) { + J.Literal l = (J.Literal) super.visitLiteral(literal, ctx); + JavaType.Primitive type = l.getType(); + if ((getStyle().getNumInt() && type == JavaType.Primitive.Int) || + (getStyle().getNumDouble() && type == JavaType.Primitive.Double) || + (getStyle().getNumLong() && type == JavaType.Primitive.Long) || + (getStyle().getNumFloat() && type == JavaType.Primitive.Float) || + (getStyle().getStringLiteral() && type == JavaType.Primitive.String) || + (getStyle().getLiteralNull() && type == JavaType.Primitive.Null) || + (getStyle().getLiteralFalse() && type == JavaType.Primitive.Boolean && l.getValue() == Boolean.valueOf(false)) || + (getStyle().getLiteralTrue() && type == JavaType.Primitive.Boolean && l.getValue() == Boolean.valueOf(true))) { + if (getCursor().getParentTreeCursor().getValue() instanceof J.Parentheses) { + getCursor().putMessageOnFirstEnclosing(J.Parentheses.class, UNNECESSARY_PARENTHESES_MESSAGE, getCursor()); + } + } + return l; + } + + @Override + public J visitAssignmentOperation(J.AssignmentOperation assignOp, ExecutionContext ctx) { + J.AssignmentOperation a = (J.AssignmentOperation) super.visitAssignmentOperation(assignOp, ctx); + J.AssignmentOperation.Type op = a.getOperator(); + if (a.getAssignment() instanceof J.Parentheses && ((getStyle().getBitAndAssign() && op == J.AssignmentOperation.Type.BitAnd) || + (getStyle().getBitOrAssign() && op == J.AssignmentOperation.Type.BitOr) || + (getStyle().getBitShiftRightAssign() && op == J.AssignmentOperation.Type.UnsignedRightShift) || + (getStyle().getBitXorAssign() && op == J.AssignmentOperation.Type.BitXor) || + (getStyle().getShiftRightAssign() && op == J.AssignmentOperation.Type.RightShift) || + (getStyle().getShiftLeftAssign() && op == J.AssignmentOperation.Type.LeftShift) || + (getStyle().getMinusAssign() && op == J.AssignmentOperation.Type.Subtraction) || + (getStyle().getDivAssign() && op == J.AssignmentOperation.Type.Division) || + (getStyle().getPlusAssign() && op == J.AssignmentOperation.Type.Addition) || + (getStyle().getStarAssign() && op == J.AssignmentOperation.Type.Multiplication) || + (getStyle().getModAssign() && op == J.AssignmentOperation.Type.Modulo))) { + a = (J.AssignmentOperation) new UnwrapParentheses<>((J.Parentheses) a.getAssignment()).visitNonNull(a, ctx, getCursor().getParentOrThrow()); + } + return a; + } + + @Override + public J visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + J.Assignment a = visitAndCast(assignment, ctx, super::visitAssignment); + if (getStyle().getAssign() && a.getAssignment() instanceof J.Parentheses) { + a = (J.Assignment) new UnwrapParentheses<>((J.Parentheses) a.getAssignment()).visitNonNull(a, ctx, getCursor().getParentOrThrow()); + } + return a; + } + + @Override + public J visitReturn(J.Return return_, ExecutionContext ctx) { + J.Return rtn = (J.Return) super.visitReturn(return_, ctx); + if (getStyle().getExpr() && rtn.getExpression() instanceof J.Parentheses) { + rtn = (J.Return) new UnwrapParentheses<>((J.Parentheses) rtn.getExpression()).visitNonNull(rtn, ctx, getCursor().getParentOrThrow()); + } + return rtn; + } + + @Override + public J visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + J.VariableDeclarations.NamedVariable v = (J.VariableDeclarations.NamedVariable) super.visitVariable(variable, ctx); + if (getStyle().getAssign() && v.getInitializer() != null && v.getInitializer() instanceof J.Parentheses) { + v = (J.VariableDeclarations.NamedVariable) new UnwrapParentheses<>((J.Parentheses) v.getInitializer()).visitNonNull(v, ctx, getCursor().getParentOrThrow()); + } + return v; + } + + @Override + public J visitLambda(J.Lambda lambda, ExecutionContext ctx) { + J.Lambda l = (J.Lambda) super.visitLambda(lambda, ctx); + if (l.getParameters().getParameters().size() == 1 && + l.getParameters().isParenthesized() && + l.getParameters().getParameters().get(0) instanceof J.VariableDeclarations && + ((J.VariableDeclarations) l.getParameters().getParameters().get(0)).getTypeExpression() == null) { + l = l.withParameters(l.getParameters().withParenthesized(false)); + } + return l; + } + + @Override + public J visitIf(J.If iff, ExecutionContext ctx) { + J.If i = (J.If) super.visitIf(iff, ctx); + // Unwrap when if condition is a single parenthesized expression + Expression expression = i.getIfCondition().getTree(); + if (expression instanceof J.Parentheses) { + i = (J.If) new UnwrapParentheses<>((J.Parentheses) expression).visitNonNull(i, ctx, getCursor().getParentOrThrow()); + } + return i; + } + + @Override + public J visitWhileLoop(J.WhileLoop whileLoop, ExecutionContext ctx) { + J.WhileLoop w = (J.WhileLoop) super.visitWhileLoop(whileLoop, ctx); + // Unwrap when while condition is a single parenthesized expression + Expression expression = w.getCondition().getTree(); + if (expression instanceof J.Parentheses) { + w = (J.WhileLoop) new UnwrapParentheses<>((J.Parentheses) expression).visitNonNull(w, ctx, getCursor().getParentOrThrow()); + } + return w; + } + + @Override + public J visitDoWhileLoop(J.DoWhileLoop doWhileLoop, ExecutionContext ctx) { + J.DoWhileLoop dw = (J.DoWhileLoop) super.visitDoWhileLoop(doWhileLoop, ctx); + // Unwrap when while condition is a single parenthesized expression + Expression expression = dw.getWhileCondition().getTree(); + if (expression instanceof J.Parentheses) { + dw = (J.DoWhileLoop) new UnwrapParentheses<>((J.Parentheses) expression).visitNonNull(dw, ctx, getCursor().getParentOrThrow()); + } + return dw; + } + + @Override + public J visitForControl(J.ForLoop.Control control, ExecutionContext ctx) { + J.ForLoop.Control fc = (J.ForLoop.Control) super.visitForControl(control, ctx); + Expression condition = fc.getCondition(); + if (condition instanceof J.Parentheses) { + fc = (J.ForLoop.Control) new UnwrapParentheses<>((J.Parentheses) condition).visitNonNull(fc, ctx, getCursor().getParentOrThrow()); + } + return fc; + } + }; + } + + @Override + public boolean causesAnotherCycle() { + return true; + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/UseCollectionInterfaces.java b/src/main/java/org/openrewrite/staticanalysis/UseCollectionInterfaces.java index aaa9fa533..0b85e90b5 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseCollectionInterfaces.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseCollectionInterfaces.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Set; +import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; import static org.openrewrite.Tree.randomId; @@ -126,6 +127,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex randomId(), m.getReturnTypeExpression().getPrefix(), Markers.EMPTY, + emptyList(), newType.getClassName(), newType, null @@ -136,6 +138,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex randomId(), Space.EMPTY, Markers.EMPTY, + emptyList(), newType.getClassName(), newType, null); @@ -176,6 +179,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m randomId(), mv.getTypeExpression().getPrefix(), Markers.EMPTY, + emptyList(), newType.getClassName(), newType, null @@ -186,6 +190,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m randomId(), Space.EMPTY, Markers.EMPTY, + emptyList(), newType.getClassName(), newType, null diff --git a/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java b/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java index 708d66b56..97a56be6d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java @@ -21,7 +21,7 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.RemoveUnusedImports; -import org.openrewrite.java.cleanup.UnnecessaryParentheses; +import org.openrewrite.java.cleanup.UnnecessaryParenthesesVisitor; import org.openrewrite.java.tree.*; import java.time.Duration; @@ -123,7 +123,7 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { .build() .apply(getCursor(), n.getCoordinates().replace()); lambda = lambda.withType(typedInterface); - lambda = (J.Lambda) new UnnecessaryParentheses().getVisitor() + lambda = (J.Lambda) new UnnecessaryParenthesesVisitor() .visitNonNull(lambda, ctx, getCursor().getParentOrThrow()); J.Block lambdaBody = methodDeclaration.getBody(); diff --git a/src/main/java/org/openrewrite/staticanalysis/groovy/GroovyFileChecker.java b/src/main/java/org/openrewrite/staticanalysis/groovy/GroovyFileChecker.java new file mode 100644 index 000000000..2476b20ac --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/groovy/GroovyFileChecker.java @@ -0,0 +1,35 @@ +/* + * Copyright 2023 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.groovy; + +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.groovy.tree.G; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.marker.SearchResult; + +/** + * Add a search marker if vising a Groovy file + */ +public class GroovyFileChecker

extends TreeVisitor { + @Override + public @Nullable Tree visit(@Nullable Tree tree, P p) { + if (tree instanceof G.CompilationUnit) { + return SearchResult.found(tree); + } + return tree; + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/kotlin/KotlinFileChecker.java b/src/main/java/org/openrewrite/staticanalysis/kotlin/KotlinFileChecker.java new file mode 100644 index 000000000..5ceb261c4 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/kotlin/KotlinFileChecker.java @@ -0,0 +1,35 @@ +/* + * Copyright 2023 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.kotlin; + +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.kotlin.tree.K; +import org.openrewrite.marker.SearchResult; + +/** + * Add a search marker if vising a Kotlin file + */ +public class KotlinFileChecker

extends TreeVisitor { + @Override + public @Nullable Tree visit(@Nullable Tree tree, P p) { + if (tree instanceof K.CompilationUnit) { + return SearchResult.found(tree); + } + return tree; + } +} diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml index 060f083b7..6976f2672 100644 --- a/src/main/resources/META-INF/rewrite/common-static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/common-static-analysis.yml @@ -73,19 +73,20 @@ recipeList: - org.openrewrite.staticanalysis.RenamePrivateFieldsToCamelCase - org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference - org.openrewrite.staticanalysis.ReplaceStringBuilderWithString - - org.openrewrite.java.cleanup.SimplifyBooleanExpression - - org.openrewrite.java.cleanup.SimplifyBooleanReturn + - org.openrewrite.staticanalysis.SimplifyBooleanExpression + - org.openrewrite.staticanalysis.SimplifyBooleanReturn - org.openrewrite.staticanalysis.StaticMethodNotFinal - org.openrewrite.staticanalysis.StringLiteralEquality - org.openrewrite.staticanalysis.UnnecessaryCloseInTryWithResources - org.openrewrite.staticanalysis.UnnecessaryExplicitTypeArguments - - org.openrewrite.java.cleanup.UnnecessaryParentheses + - org.openrewrite.staticanalysis.UnnecessaryParentheses - org.openrewrite.staticanalysis.UnnecessaryPrimitiveAnnotations - org.openrewrite.staticanalysis.UpperCaseLiteralSuffixes # - org.openrewrite.staticanalysis.UnnecessaryThrows # - org.openrewrite.staticanalysis.UseCollectionInterfaces - org.openrewrite.staticanalysis.UseDiamondOperator - org.openrewrite.staticanalysis.UseJavaStyleArrayDeclarations +# https://github.com/openrewrite/rewrite-static-analysis/issues/10 # - org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface # - org.openrewrite.staticanalysis.UseStringReplace - org.openrewrite.staticanalysis.WhileInsteadOfFor @@ -142,16 +143,19 @@ recipeList: - org.openrewrite.staticanalysis.RenameMethodsNamedHashcodeEqualOrTostring - org.openrewrite.staticanalysis.RenamePrivateFieldsToCamelCase - org.openrewrite.staticanalysis.ReplaceStringBuilderWithString - - org.openrewrite.java.cleanup.SimplifyBooleanExpression - - org.openrewrite.java.cleanup.SimplifyBooleanReturn + - org.openrewrite.staticanalysis.SimplifyBooleanExpression + - org.openrewrite.staticanalysis.SimplifyBooleanReturn - org.openrewrite.staticanalysis.StaticMethodNotFinal - - org.openrewrite.staticanalysis.StringLiteralEquality - org.openrewrite.staticanalysis.UnnecessaryCloseInTryWithResources - org.openrewrite.staticanalysis.UnnecessaryParentheses - org.openrewrite.staticanalysis.UnnecessaryPrimitiveAnnotations - org.openrewrite.staticanalysis.UpperCaseLiteralSuffixes - org.openrewrite.staticanalysis.UseDiamondOperator - org.openrewrite.staticanalysis.UseJavaStyleArrayDeclarations - - org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface +# https://github.com/openrewrite/rewrite-static-analysis/issues/10 +# - org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface - org.openrewrite.staticanalysis.WhileInsteadOfFor - org.openrewrite.staticanalysis.WriteOctalValuesAsDecimal + - org.openrewrite.kotlin.cleanup.EqualsMethodUsage + - org.openrewrite.kotlin.cleanup.ImplicitParameterInLambda + - org.openrewrite.kotlin.cleanup.ReplaceCharToIntWithCode diff --git a/src/main/resources/META-INF/rewrite/static-analysis.yml b/src/main/resources/META-INF/rewrite/static-analysis.yml index d965bee56..04b740bd8 100644 --- a/src/main/resources/META-INF/rewrite/static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/static-analysis.yml @@ -35,7 +35,7 @@ recipeList: - org.openrewrite.staticanalysis.HideUtilityClassConstructor - org.openrewrite.staticanalysis.NeedBraces - org.openrewrite.staticanalysis.OperatorWrap - - org.openrewrite.java.cleanup.UnnecessaryParentheses + - org.openrewrite.staticanalysis.UnnecessaryParentheses - org.openrewrite.staticanalysis.ReplaceThreadRunWithThreadStart - org.openrewrite.staticanalysis.ChainStringBuilderAppendCalls - org.openrewrite.staticanalysis.ReplaceStringBuilderWithString diff --git a/src/test/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperatorTest.java b/src/test/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperatorTest.java index 62bd74f93..bb7cdb329 100644 --- a/src/test/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperatorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperatorTest.java @@ -29,6 +29,7 @@ public void defaults(RecipeSpec spec) { spec.recipe(new CompareEnumsWithEqualityOperator()); } + //language=java SourceSpecs enumA = java( """ package a; @@ -198,4 +199,21 @@ void m(Object parameterValue, Type partType) { """ )); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/143") + void noSelect() { + rewriteRun( + //language=java + java(""" + package a; + public enum A { + FOO, BAR, BUZ; + boolean isFoo() { + return equals(FOO); + } + } + """) + ); + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/FinalizePrivateFieldsTest.java b/src/test/java/org/openrewrite/staticanalysis/FinalizePrivateFieldsTest.java index f71a84ef2..1aa1b3355 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FinalizePrivateFieldsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FinalizePrivateFieldsTest.java @@ -836,4 +836,24 @@ class Reproducer { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/121") + void mustNotChangeVolatileFields() { + //language=java + rewriteRun( + java( + """ + public final class Reproducer { + + private Reproducer() { + } + + private static volatile String foo = "this becomes final volatile, which is invalid"; + + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/MinimumSwitchCasesTest.java b/src/test/java/org/openrewrite/staticanalysis/MinimumSwitchCasesTest.java index 860ad8335..71a985064 100644 --- a/src/test/java/org/openrewrite/staticanalysis/MinimumSwitchCasesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/MinimumSwitchCasesTest.java @@ -486,6 +486,52 @@ void someMethod() { ); } + @Test + void importsOnEnumImplied() { + //noinspection EnhancedSwitchMigration + rewriteRun( + //language=java + java( + """ + import java.time.LocalDate; + + class Test { + void test(LocalDate date) { + switch(date.getDayOfWeek()) { + case MONDAY: + someMethod(); + break; + default: + someMethod(); + break; + } + } + + void someMethod() { + } + } + """, + """ + import java.time.DayOfWeek; + import java.time.LocalDate; + + class Test { + void test(LocalDate date) { + if (date.getDayOfWeek() == DayOfWeek.MONDAY) { + someMethod(); + } else { + someMethod(); + } + } + + void someMethod() { + } + } + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/1701") @Test void removeBreaksFromCaseBody() { diff --git a/src/test/java/org/openrewrite/staticanalysis/ModifierOrderTest.java b/src/test/java/org/openrewrite/staticanalysis/ModifierOrderTest.java index 8585fa370..d1f79497a 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ModifierOrderTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ModifierOrderTest.java @@ -15,6 +15,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; @@ -22,6 +23,7 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.kotlin.Assertions.kotlin; @SuppressWarnings("UnnecessaryModifier") @Issue("https://github.com/openrewrite/rewrite/issues/466") @@ -70,4 +72,39 @@ public static void main(String[] args) { ) ); } + + @Nested + class KotlinTest { + @Test + void constModifier() { + rewriteRun( + kotlin( + """ + object Test { + private const val CLIENT = "ABC" + } + """ + ) + ); + } + + @Test + void overrideModifier() { + rewriteRun( + kotlin( + """ + open class Shape { + public open fun draw() { + } + } + + class Circle : Shape() { + public override fun draw() { + } + } + """ + ) + ); + } + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java index 6d759743f..ffcc1c056 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java @@ -15,7 +15,9 @@ */ package org.openrewrite.staticanalysis; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; import org.openrewrite.test.RecipeSpec; @@ -23,6 +25,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.version; +import static org.openrewrite.kotlin.Assertions.kotlin; @SuppressWarnings({ "ConstantConditions", @@ -558,6 +561,7 @@ static void method() { ); } + @SuppressWarnings("All") @Test void ignoreAnonymousClassVariables() { rewriteRun( @@ -988,4 +992,123 @@ public record MyRecord( ), 17) ); } + + @Test + void removeKotlinUnusedLocalVariable() { + rewriteRun( + kotlin( + """ + class A (val b: String) { + fun foo() { + val bar = b; + } + } + """, + """ + class A (val b: String) { + fun foo() { + } + } + """ + ) + ); + } + + @Test + void retainJavaUnusedLocalVariableWithNewClass() { + rewriteRun( + //language=java + java( + """ + class A {} + class B { + void foo() { + A a = new A(); + } + } + """ + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/152") + void retainUnusedInsideCase() { + rewriteRun( + //language=java + java( + """ + class Test { + static void method() { + int x = 10; + char y = 20; + switch (x) { + case 10: + byte unused; + break; + } + } + } + """, + """ + class Test { + static void method() { + int x = 10; + switch (x) { + case 10: + byte unused; + break; + } + } + } + """ + ) + ); + } + + @Nested + class Kotlin { + + @Test + void retainUnusedLocalVariableWithNewClass() { + rewriteRun( + kotlin( + """ + class A {} + class B { + fun foo() { + val a = A(); + } + } + """ + ) + ); + } + + @Test + @ExpectedToFail("Not yet implemented") + void retainUnusedLocalVariableConst() { + rewriteRun( + //language=kotlin + kotlin( + """ + package constants + const val FOO = "bar" + """ + ), + //language=kotlin + kotlin( + """ + package config + import constants.FOO + fun baz() { + val foo = FOO + println(foo) + } + """ + ) + ); + } + + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java b/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java index 80adabb72..8dd0c3ba7 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java @@ -347,4 +347,29 @@ public record MyRecord( ) ); } + + @Test + void noChangeInAnonymousClass() { + rewriteRun( + java( + """ + interface MyInterface { + void doSomething(); + } + + public class A { + void test() { + new MyInterface() { + int SOME_THING_NOT_LOCAL = 1; + + @Override + public void doSomething() { + } + }; + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java index 5cfd9e18d..5dbd5ec89 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceLambdaWithMethodReferenceTest.java @@ -16,7 +16,6 @@ package org.openrewrite.staticanalysis; - import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -78,6 +77,25 @@ List method(List l) { ); } + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/96") + @Test + void ignoreAmbiguousMethodReference() { + rewriteRun( + //language=java + java( + """ + import java.util.stream.Stream; + + class Test { + Stream method() { + return Stream.of(1, 32, 12, 15, 23).map(x -> Integer.toString(x)); + } + } + """ + ) + ); + } + @Test void containsMultipleStatements() { rewriteRun( @@ -426,7 +444,6 @@ void systemOutPrint() { java( """ import java.util.List; - class Test { void method(List input) { input.forEach(x -> System.out.println(x)); @@ -435,7 +452,6 @@ void method(List input) { """, """ import java.util.List; - class Test { void method(List input) { input.forEach(System.out::println); @@ -453,7 +469,6 @@ void systemOutPrintInBlock() { java( """ import java.util.List; - class Test { void method(List input) { input.forEach(x -> { System.out.println(x); }); @@ -462,7 +477,6 @@ void method(List input) { """, """ import java.util.List; - class Test { void method(List input) { input.forEach(System.out::println); @@ -484,8 +498,8 @@ public class CheckType { } """ ), + //language=java java( - //language=java """ import java.util.List; import java.util.stream.Collectors; @@ -917,7 +931,6 @@ void foo() { s = () -> new java.util.ArrayList(); s = () -> new ArrayList(); s = () -> new java.util.HashSet(); - Function f; f = i -> new ArrayList(i); } @@ -938,7 +951,6 @@ void foo() { s = java.util.ArrayList::new; s = ArrayList::new; s = java.util.HashSet::new; - Function f; f = ArrayList::new; } diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceWeekYearWithYearTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceWeekYearWithYearTest.java new file mode 100644 index 000000000..0a7b8bd50 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceWeekYearWithYearTest.java @@ -0,0 +1,261 @@ +/* + * Copyright 2023 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.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.Issue; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("SuspiciousDateFormat") +class ReplaceWeekYearWithYearTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec + .recipe(new ReplaceWeekYearWithYear()); + } + + @Test + @DocumentExample + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/58") + void changeSimpleDateFormat() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31"); + String result = new SimpleDateFormat("YYYY/MM/dd").format(date); + } + } + """, + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31"); + String result = new SimpleDateFormat("yyyy/MM/dd").format(date); + } + } + """ + ) + ); + } + + @Test + void worksWithOfPatternFormatter() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.time.format.DateTimeFormatter; + import java.util.Date; + + class Test { + public void formatDate() { + Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31"); + String result = DateTimeFormatter.ofPattern("YYYY/MM/dd").format(date.toInstant()); + } + } + """, + """ + import java.text.SimpleDateFormat; + import java.time.format.DateTimeFormatter; + import java.util.Date; + + class Test { + public void formatDate() { + Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31"); + String result = DateTimeFormatter.ofPattern("yyyy/MM/dd").format(date.toInstant()); + } + } + """ + ) + ); + } + + @Test + void worksWithYYUses() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.time.format.DateTimeFormatter; + import java.util.Date; + + class Test { + public void formatDate() { + Date date = new SimpleDateFormat("yy/MM/dd").parse("2015/12/31"); + String result = DateTimeFormatter.ofPattern("YY/MM/dd").format(date.toInstant()); + } + } + """, + """ + import java.text.SimpleDateFormat; + import java.time.format.DateTimeFormatter; + import java.util.Date; + + class Test { + public void formatDate() { + Date date = new SimpleDateFormat("yy/MM/dd").parse("2015/12/31"); + String result = DateTimeFormatter.ofPattern("yy/MM/dd").format(date.toInstant()); + } + } + """ + ) + ); + } + + @Test + void onlyRunsWhenFormatAndDateTypesAreUsed() { + //language=java + rewriteRun( + java( + """ + class Test { + public static void main(String[] args) { + String pattern = "YYYY/MM/dd"; + System.out.println(pattern); + } + } + """ + ) + ); + } + + @Test + void standaloneNewClassCall() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("YYYY-MM-dd"); + Date date = format.parse("2015/12/31"); + } + } + """, + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd"); + Date date = format.parse("2015/12/31"); + } + } + """ + ) + ); + } + @Test + void patternUsesSingleQuotes() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("'Your date is:' YYYY-MM-dd"); + Date date = format.parse("2015/12/31"); + } + } + """, + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("'Your date is:' yyyy-MM-dd"); + Date date = format.parse("2015/12/31"); + } + } + """ + ) + ); + } + + @Test + void patternUsesMultipleSingleQuotes() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("'Your date is:' YYYY-MM-dd, 'yy'"); + Date date = format.parse("2015/12/31"); + } + } + """, + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("'Your date is:' yyyy-MM-dd, 'yy'"); + Date date = format.parse("2015/12/31"); + } + } + """ + ) + ); + } + + @Test + void doesNotChangeWhyInSingleQuotes() { + //language=java + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + import java.util.Date; + + class Test { + public void formatDate() { + SimpleDateFormat format = new SimpleDateFormat("'Y' dd-MM"); + Date date = format.parse("2015/12/31"); + } + } + """ + ) + ); + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionTest.java new file mode 100644 index 000000000..b91ba01c6 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionTest.java @@ -0,0 +1,348 @@ +/* + * 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.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("ALL") +class SimplifyBooleanExpressionTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SimplifyBooleanExpression()); + } + + @DocumentExample + @Test + void simplifyEqualsLiteralTrueIf() { + rewriteRun( + java( + """ + public class A { + boolean a; + { + if(true == a) { + } + } + } + """, + """ + public class A { + boolean a; + { + if(a) { + } + } + } + """ + ) + ); + } + + @Test + void simplifyBooleanExpressionComprehensive() { + rewriteRun( + java( + """ + public class A { + { + boolean a = !false; + boolean b = (a == true); + boolean c = b || true; + boolean d = c || c; + boolean e = d && d; + boolean f = (e == true) || e; + boolean g = f && false; + boolean h = g; + boolean i = (a != false); + } + } + """, + """ + public class A { + { + boolean a = true; + boolean b = a; + boolean c = true; + boolean d = c; + boolean e = d; + boolean f = e; + boolean g = false; + boolean h = g; + boolean i = a; + } + } + """ + ) + ); + } + + @Test + void simplifyInvertedBooleanLiteral() { + rewriteRun( + java( + """ + public class A { + { + boolean a = !false; + boolean b = !true; + } + } + """, + """ + public class A { + { + boolean a = true; + boolean b = false; + } + } + """ + ) + ); + } + + @Test + void simplifyEqualsLiteralTrue() { + rewriteRun( + java( + """ + public class A { + { + boolean a = true; + boolean b = (a == true); + } + } + """, + """ + public class A { + { + boolean a = true; + boolean b = a; + } + } + """ + ) + ); + } + + @Test + void simplifyOrLiteralTrue() { + rewriteRun( + java( + """ + public class A { + { + boolean b = true; + boolean c = b || true; + } + } + """, + """ + public class A { + { + boolean b = true; + boolean c = true; + } + } + """ + ) + ); + } + + @Test + void simplifyOrAlwaysTrue() { + rewriteRun( + java( + """ + public class A { + { + boolean c = true; + boolean d = c || c; + } + } + """, + """ + public class A { + { + boolean c = true; + boolean d = c; + } + } + """ + ) + ); + } + + @Test + void simplifyAndAlwaysTrue() { + rewriteRun( + java( + """ + public class A { + { + boolean d = true; + boolean e = d && d; + } + } + """, + """ + public class A { + { + boolean d = true; + boolean e = d; + } + } + """ + ) + ); + } + + @Test + void simplifyEqualsLiteralTrueAlwaysTrue() { + rewriteRun( + java( + """ + public class A { + { + boolean e = true; + boolean f = (e == true) || e; + } + } + """, + """ + public class A { + { + boolean e = true; + boolean f = e; + } + } + """ + ) + ); + } + + @Test + void simplifyLiteralFalseAlwaysFalse() { + rewriteRun( + java( + """ + public class A { + { + boolean f = true; + boolean g = f && false; + } + } + """, + """ + public class A { + { + boolean f = true; + boolean g = false; + } + } + """ + ) + ); + } + + @Test + void simplifyDoubleNegation() { + rewriteRun( + java( + """ + public class A { + public void doubleNegation(boolean g) { + boolean h = g; + } + } + """ + ) + ); + } + + @Test + void simplifyNotEqualsFalse() { + rewriteRun( + java( + """ + public class A { + { + boolean a = true; + boolean i = (a != false); + } + } + """, + """ + public class A { + { + boolean a = true; + boolean i = a; + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/502") + @Test + void autoFormatIsConditionallyApplied() { + rewriteRun( + java( + """ + public class A { + { + boolean a=true; + boolean i=(a!=true); + } + } + """ + ) + ); + } + + @Test + void binaryOrBothFalse() { + rewriteRun( + java( + """ + public class A { + { + if (!true || !true) { + System.out.println(""); + } + } + } + """, + """ + public class A { + { + if (false) { + System.out.println(""); + } + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanReturnTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanReturnTest.java new file mode 100644 index 000000000..02072ecdc --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanReturnTest.java @@ -0,0 +1,214 @@ +/* + * 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.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("ALL") +class SimplifyBooleanReturnTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SimplifyBooleanReturn()); + } + + @DocumentExample + @Test + void simplifyBooleanReturn() { + rewriteRun( + java( + """ + public class A { + boolean ifNoElse() { + if (isOddMillis()) { + return true; + } + return false; + } + + static boolean isOddMillis() { + boolean even = System.currentTimeMillis() % 2 == 0; + if (even == true) { + return false; + } + else { + return true; + } + } + } + """, + """ + public class A { + boolean ifNoElse() { + return isOddMillis(); + } + + static boolean isOddMillis() { + boolean even = System.currentTimeMillis() % 2 == 0; + return !(even == true); + } + } + """ + ) + ); + } + + @Test + void dontSimplifyToReturnUnlessLastStatement() { + rewriteRun( + java( + """ + public class A { + public boolean absurdEquals(Object o) { + if(this == o) { + return true; + } + if(this == o) { + return true; + } + return false; + } + } + """, + """ + public class A { + public boolean absurdEquals(Object o) { + if(this == o) { + return true; + } + return this == o; + } + } + """ + ) + ); + } + + @Test + void nestedIfsWithNoBlock() { + rewriteRun( + java( + """ + public class A { + public boolean absurdEquals(Object o) { + if(this == o) + if(this == null) + return true; + return false; + } + } + """ + ) + ); + } + + @Test + void dontAlterWhenElseIfPresent() { + rewriteRun( + java( + """ + public class A { + public boolean foo(int n) { + if (n == 1) { + return false; + } + else if (n == 2) { + return true; + } + else { + return false; + } + } + } + """ + ) + ); + } + + @Test + void dontAlterWhenElseContainsSomethingOtherThanReturn() { + rewriteRun( + java( + """ + public class A { + public boolean foo(int n) { + if (n == 1) { + return true; + } + else { + System.out.println("side effect"); + return false; + } + } + } + """ + ) + ); + } + + @Test + void onlySimplifyToReturnWhenLastStatement() { + rewriteRun( + java( + """ + import java.util.*; + public class A { + public static boolean deepEquals(List l, List r) { + for (int i = 0; i < l.size(); ++i) { + if (!Arrays.equals(l.get(i), r.get(i))) { + return false; + } + } + return true; + } + } + """ + ) + ); + } + + @Test + void wrapNotReturnsOfTernaryIfConditionsInParentheses() { + rewriteRun( + java( + """ + public class A { + Object failure; + public boolean equals(Object o) { + if (failure != null ? !failure.equals(this.failure) : this.failure != null) { + return false; + } + return true; + } + } + """, + """ + public class A { + Object failure; + public boolean equals(Object o) { + return !(failure != null ? !failure.equals(this.failure) : this.failure != null); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/SortedSetStreamToLinkedHashSetTest.java b/src/test/java/org/openrewrite/staticanalysis/SortedSetStreamToLinkedHashSetTest.java new file mode 100644 index 000000000..52859ac2c --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/SortedSetStreamToLinkedHashSetTest.java @@ -0,0 +1,116 @@ +/* + * Copyright 2023 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.junit.jupiter.api.Test; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class SortedSetStreamToLinkedHashSetTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SortedSetStreamToLinkedHashSet()); + } + + @Test + void changeSortedSetStreamToLinkedHashSet() { + rewriteRun( + //language=java + java(""" + import java.util.Set; + import java.util.stream.Collectors; + + class A { + void method(Set set) { + Set sorted = set.stream().sorted().collect(Collectors.toSet()); + } + } + """, """ + import java.util.LinkedHashSet; + import java.util.Set; + import java.util.stream.Collectors; + + class A { + void method(Set set) { + Set sorted = set.stream().sorted().collect(Collectors.toCollection(LinkedHashSet::new)); + } + } + """)); + } + + @Test + void changeSortedSetStreamToLinkedHashSetStaticImport() { + rewriteRun( + //language=java + java(""" + import java.util.Set; + import static java.util.stream.Collectors.toSet; + + class A { + void method(Set set) { + Set sorted = set.stream().sorted().collect(toSet()); + } + } + """, """ + import java.util.LinkedHashSet; + import java.util.Set; + import java.util.stream.Collectors; + + class A { + void method(Set set) { + Set sorted = set.stream().sorted().collect(Collectors.toCollection(LinkedHashSet::new)); + } + } + """)); + } + + @Test + void ignoreCollectToLinkedHashSet() { + rewriteRun( + //language=java + java(""" + import java.util.Set; + import java.util.LinkedHashSet; + import java.util.stream.Collectors; + + class A { + void method(Set set) { + Set sorted = set.stream().sorted().collect(Collectors.toCollection(LinkedHashSet::new)); + } + } + """)); + } + + @Test + void ignoreCollectToList() { + rewriteRun( + //language=java + java(""" + import java.util.List; + import java.util.stream.Collectors; + + class A { + void method(Set set) { + List sorted = set.stream().sorted().collect(Collectors.toList()); + } + } + """)); + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArgumentsTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArgumentsTest.java index f753d5728..6ba1d3388 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArgumentsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryExplicitTypeArgumentsTest.java @@ -16,6 +16,7 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -23,6 +24,7 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.kotlin.Assertions.kotlin; @SuppressWarnings({"RedundantTypeArguments", "InfiniteRecursion", "CodeBlock2Expr"}) class UnnecessaryExplicitTypeArgumentsTest implements RewriteTest { @@ -135,6 +137,7 @@ void test() { ); } + @SuppressWarnings("UnnecessaryLocalVariable") @Issue("https://github.com/openrewrite/rewrite/issues/2818") @Test void assignedToVar() { @@ -155,4 +158,63 @@ List test() { ) ); } + + @Test + void containerInitialization() { + rewriteRun( + java( + """ + import java.util.List; + + public class Test { + List test() { + List l = List. of("x"); + return l; + } + } + """, + """ + import java.util.List; + + public class Test { + List test() { + List l = List. of("x"); + return l; + } + } + """ + ) + ); + } + + @Nested + class kotlinTest { + @Test + void doNotChangeIfHasNotTypeInference() { + rewriteRun( + kotlin( + """ + val foo = listOf() + var bar = mutableMapOf() + """ + ) + ); + } + + @Test + void changeIfHasTypeInference() { + rewriteRun( + kotlin( + """ + val foo = listOf("a", "b") + var bar = mutableMapOf("a" to 1) + """, + """ + val foo = listOf("a", "b") + var bar = mutableMapOf("a" to 1) + """ + ) + ); + } + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryParenthesesTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryParenthesesTest.java new file mode 100644 index 000000000..071b5c97a --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryParenthesesTest.java @@ -0,0 +1,913 @@ +/* + * Copyright 2023 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. + */ + +/* + * 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.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; +import org.openrewrite.Tree; +import org.openrewrite.java.JavaParser; +import org.openrewrite.java.style.UnnecessaryParenthesesStyle; +import org.openrewrite.style.NamedStyles; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import java.util.function.Consumer; +import java.util.function.UnaryOperator; + +import static java.util.Collections.emptySet; +import static java.util.Collections.singletonList; +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings({"ConstantConditions"}) +class UnnecessaryParenthesesTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new UnnecessaryParentheses()); + } + + private static Consumer unnecessaryParentheses(UnaryOperator with) { + return spec -> spec.parser(JavaParser.fromJavaVersion().styles( + singletonList( + new NamedStyles( + Tree.randomId(), "test", "test", "test", emptySet(), + singletonList(with.apply(new UnnecessaryParenthesesStyle(false, false, false, false, false, + false, false, false, false, false, false, false, false, false, + false, false, false, false, false, false, false, false, false)))))) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2170") + @Test + void minimumSpace() { + rewriteRun( + java( + """ + class Test { + int test() { + return (1); + } + } + """, + """ + class Test { + int test() { + return 1; + } + } + """ + ) + ); + } + + @DocumentExample + @Test + void fullUnwrappingDefault() { + rewriteRun( + java( + """ + import java.util.*; + + class Test { + int square(int a, int b) { + int square = (a * b); + + int sumOfSquares = 0; + for (int i = (0); i < 10; i++) { + sumOfSquares += (square(i * i, i)); + } + double num = (10.0); + + List list = Arrays.asList("a1", "b1", "c1"); + list.stream() + .filter((s) -> s.startsWith("c")) + .forEach(System.out::println); + + return (square); + } + } + """, + """ + import java.util.*; + + class Test { + int square(int a, int b) { + int square = a * b; + + int sumOfSquares = 0; + for (int i = 0; i < 10; i++) { + sumOfSquares += square(i * i, i); + } + double num = 10.0; + + List list = Arrays.asList("a1", "b1", "c1"); + list.stream() + .filter(s -> s.startsWith("c")) + .forEach(System.out::println); + + return square; + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/798") + @Disabled + @Test + void unwrapExpr() { + rewriteRun( + unnecessaryParentheses(style -> style.withExpr(true)), + java( + """ + class Test { + void method(int x, int y, boolean a) { + if (a && ((x + y > 0))) { + int q = ((1 + 2) + 3); + int z = (q + q) * q; + } + } + } + """, + """ + class Test { + void method(int x, int y, boolean a) { + if (a && (x + y > 0)) { + int q = (1 + 2) + 3; + int z = (q + q) * q; + } + } + } + """ + ) + ); + } + + @Test + void unwrapIdent() { + rewriteRun( + unnecessaryParentheses(style -> style.withIdent(true)), + java( + """ + class Test { + double doNothing() { + double num = (10.0); + return (num); + } + } + """, + """ + class Test { + double doNothing() { + double num = (10.0); + return num; + } + } + """ + ) + ); + } + + @Test + void unwrapNum() { + rewriteRun( + unnecessaryParentheses(style -> style.withNumDouble(true).withNumFloat(true) + .withNumInt(true).withNumLong(true)), + java( + """ + class Test { + void doNothing() { + double a = (1000.0); + if ((1000.0) == a) { + a = (1000.0); + } + float b = (1000.0f); + int c = (1000); + long d = (1000L); + } + } + """, + """ + class Test { + void doNothing() { + double a = 1000.0; + if (1000.0 == a) { + a = 1000.0; + } + float b = 1000.0f; + int c = 1000; + long d = 1000L; + } + } + """ + ) + ); + } + + @Test + void unwrapLiteral() { + rewriteRun( + unnecessaryParentheses(style -> style.withLiteralFalse(true).withLiteralTrue(true) + .withLiteralNull(true).withStringLiteral(true)), + java( + """ + class Test { + void doNothing() { + boolean a = (true); + boolean b = (false); + if (a == (true)) { + b = (false); + } else if (b == (false)) { + a = (true); + } + + String s = ("literallyString"); + String t = ("literallyString" + "stringLiteral"); + if (s == (null)) { + s = (null); + } else if ((("someLiteral").toLowerCase()).equals(s)) { + s = null; + } + } + } + """, + """ + class Test { + void doNothing() { + boolean a = true; + boolean b = false; + if (a == true) { + b = false; + } else if (b == false) { + a = true; + } + + String s = "literallyString"; + String t = ("literallyString" + "stringLiteral"); + if (s == null) { + s = null; + } else if (("someLiteral".toLowerCase()).equals(s)) { + s = null; + } + } + } + """ + ) + ); + } + + @Test + void unwrapAssignment() { + rewriteRun( + unnecessaryParentheses(style -> style.withAssign(true)), + java( + """ + class Test { + void doNothing() { + double a = (10.0); + a = (10.0); + double b = (a); + b = b; // identity assignment + b += (b); + double c = (a + (b)); + c = (a + b); + c = a + b; // binary operation + c *= (c); + + String d = ("example") + ("assignment"); + d = ("example" + "assignment"); + d += ("example") + ("assignment"); + d = (("example") + ("assignment")); + } + } + """, + """ + class Test { + void doNothing() { + double a = 10.0; + a = 10.0; + double b = a; + b = b; // identity assignment + b += (b); + double c = a + (b); + c = a + b; + c = a + b; // binary operation + c *= (c); + + String d = ("example") + ("assignment"); + d = "example" + "assignment"; + d += ("example") + ("assignment"); + d = ("example") + ("assignment"); + } + } + """ + ) + ); + } + + @Test + void unwrapBandAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withBitAndAssign(true)), + java( + """ + class Test { + int a = 5; + int b = 7; + + void bitwiseAnd() { + int c = (a & b); + c &= (c); + } + } + """, + """ + class Test { + int a = 5; + int b = 7; + + void bitwiseAnd() { + int c = (a & b); + c &= c; + } + } + """ + ) + ); + } + + @Test + void unwrapBorAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withBitOrAssign(true)), + java( + """ + class Test { + int a = 5; + int b = 7; + + void bitwiseOr() { + int c = (a | b); + c |= (c); + } + } + """, + """ + class Test { + int a = 5; + int b = 7; + + void bitwiseOr() { + int c = (a | b); + c |= c; + } + } + """ + ) + ); + } + + @Test + void unwrapBsrAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withBitShiftRightAssign(true)), + java( + """ + class Test { + int a = -1; + + void unsignedRightShiftAssignment() { + int b = a >>> 1; + b >>>= (b); + } + } + """, + """ + class Test { + int a = -1; + + void unsignedRightShiftAssignment() { + int b = a >>> 1; + b >>>= b; + } + } + """ + ) + ); + } + + @Test + void unwrapBxorAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withBitXorAssign(true)), + java( + """ + class Test { + boolean a = true; + boolean b = false; + + void bitwiseExclusiveOr() { + boolean c = (a ^ b); + c ^= (c); + } + } + """, + """ + class Test { + boolean a = true; + boolean b = false; + + void bitwiseExclusiveOr() { + boolean c = (a ^ b); + c ^= c; + } + } + """ + ) + ); + } + + @Test + void unwrapDivAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withDivAssign(true)), + java( + """ + class Test { + int a = 10; + int b = 5; + + void divisionAssignmentOperator() { + int c = (a / b); + c /= (c); + } + } + """, + """ + class Test { + int a = 10; + int b = 5; + + void divisionAssignmentOperator() { + int c = (a / b); + c /= c; + } + } + """ + ) + ); + } + + @Test + void unwrapMinusAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withMinusAssign(true)), + java( + """ + class Test { + int a = 10; + int b = 5; + + void minusAssignment() { + int c = (a - b); + c -= (c); + } + } + """, + """ + class Test { + int a = 10; + int b = 5; + + void minusAssignment() { + int c = (a - b); + c -= c; + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/1486") + @Test + void unwrapMinusReturnExpression() { + rewriteRun( + unnecessaryParentheses(style -> style.withExpr(true)), + java( + """ + class T { + int getInt() { + return (4 - 5); + } + } + """, + """ + class T { + int getInt() { + return 4 - 5; + } + } + """ + ) + ); + } + + @Test + void unwrapModAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withModAssign(true)), + java( + """ + class Test { + int a = 5; + int b = 3; + + void remainderAssignment() { + int c = a % b; + c %= (c); + } + } + """, + """ + class Test { + int a = 5; + int b = 3; + + void remainderAssignment() { + int c = a % b; + c %= c; + } + } + """ + ) + ); + } + + @Test + void unwrapPlusAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withPlusAssign(true)), + java( + """ + class Test { + int a = 1; + int b = 1; + + void plusAssignment() { + int c = a + b; + c += (c); + } + } + """, + """ + class Test { + int a = 1; + int b = 1; + + void plusAssignment() { + int c = a + b; + c += c; + } + } + """ + ) + ); + } + + @Test + void unwrapSlAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withShiftLeftAssign(true)), + java( + """ + class Test { + int a = 1; + int b = 1; + + void leftShiftAssignment() { + int c = a << b; + c <<= (c); + } + } + """, + """ + class Test { + int a = 1; + int b = 1; + + void leftShiftAssignment() { + int c = a << b; + c <<= c; + } + } + """ + ) + ); + } + + @Test + void unwrapSrAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withShiftRightAssign(true)), + java( + """ + class Test { + int a = 1; + int b = 1; + + void signedRightShiftAssignment() { + int c = a >> b; + c >>= (c); + } + } + """, + """ + class Test { + int a = 1; + int b = 1; + + void signedRightShiftAssignment() { + int c = a >> b; + c >>= c; + } + } + """ + ) + ); + } + + @Test + void unwrapStarAssign() { + rewriteRun( + unnecessaryParentheses(style -> style.withStarAssign(true)), + java( + """ + class Test { + int a = 1; + int b = 1; + + void multiplicationAssignmentOperator() { + int c = a * b; + c *= (c); + } + } + """, + """ + class Test { + int a = 1; + int b = 1; + + void multiplicationAssignmentOperator() { + int c = a * b; + c *= c; + } + } + """ + ) + ); + } + + @Test + void unwrapLambda() { + rewriteRun( + unnecessaryParentheses(style -> style.withLambda(true)), + java( + """ + import java.util.*; + + class Test { + void doNothing() { + List list = Arrays.asList("a1", "b1", "c1"); + list.stream() + .filter((s) -> s.startsWith("c")) + .forEach(System.out::println); + } + } + """, + """ + import java.util.*; + + class Test { + void doNothing() { + List list = Arrays.asList("a1", "b1", "c1"); + list.stream() + .filter(s -> s.startsWith("c")) + .forEach(System.out::println); + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/798") + @Test + void unwrapDoubleParens() { + rewriteRun( + java( + """ + class Test { + void test() { + int sum = 1 + ((2 + 3)); + } + } + """, + """ + class Test { + void test() { + int sum = 1 + (2 + 3); + } + } + """ + ) + ); + } + + @Test + void unwrapIfParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + if ((s == null || s.isEmpty())) { + System.out.println("empty"); + } + } + } + """, + """ + class Test { + void test(String s) { + if (s == null || s.isEmpty()) { + System.out.println("empty"); + } + } + } + """ + ) + ); + } + + @Test + void doNotUnwrapIfNoParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + if (s == null || s.isEmpty()) { + System.out.println("empty"); + } + } + } + """ + ) + ); + } + + @Test + void doNotUnwrapNegatedIfParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + if (!(s == null || s.isEmpty())) { + System.out.println("empty"); + } + } + } + """ + ) + ); + } + + @Test + void doNotUnwrapIfParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + if ((s == null || s.isEmpty()) || false) { + System.out.println("empty"); + } + } + } + """ + ) + ); + } + + @Test + void unwrapWhileParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + while ((s == null || s.isEmpty())) { + s = "not empty"; + } + } + } + """, + """ + class Test { + void test(String s) { + while (s == null || s.isEmpty()) { + s = "not empty"; + } + } + } + """ + ) + ); + } + + @Test + void unwrapDoWhileParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + do { + s = "not empty"; + } while ((s == null || s.isEmpty())); + } + } + """, + """ + class Test { + void test(String s) { + do { + s = "not empty"; + } while (s == null || s.isEmpty()); + } + } + """ + ) + ); + } + + @Test + void unwrapForControlParens() { + rewriteRun( + java( + """ + class Test { + void test(String s) { + for (int i = 0; (i < 10); i++) { + System.out.println(i); + } + } + } + """, + """ + class Test { + void test(String s) { + for (int i = 0; i < 10; i++) { + System.out.println(i); + } + } + } + """ + ) + ); + } +} + diff --git a/src/test/java/org/openrewrite/staticanalysis/UseDiamondOperatorTest.java b/src/test/java/org/openrewrite/staticanalysis/UseDiamondOperatorTest.java index ec31e975d..34fb4342e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UseDiamondOperatorTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UseDiamondOperatorTest.java @@ -15,6 +15,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; @@ -23,6 +24,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.javaVersion; +import static org.openrewrite.kotlin.Assertions.kotlin; @SuppressWarnings({"Convert2Diamond", "unchecked", "rawtypes"}) class UseDiamondOperatorTest implements RewriteTest { @@ -401,4 +403,25 @@ class Foo { ) ); } + + @Nested + class kotlinTest { + @Test + void doNotChange() { + rewriteRun( + kotlin( + """ + class test { + fun method() { + val foo = listOf() + var schemaPaths = mutableListOf("a") + var typeMapping = mutableMapOf() + } + } + """ + ) + ); + } + } + } diff --git a/src/test/java/org/openrewrite/staticanalysis/groovy/StringLiteralEqualityTest.java b/src/test/java/org/openrewrite/staticanalysis/groovy/StringLiteralEqualityTest.java new file mode 100644 index 000000000..97f0a7e8c --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/groovy/StringLiteralEqualityTest.java @@ -0,0 +1,46 @@ +/* + * Copyright 2023 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.groovy; + +import org.junit.jupiter.api.Test; +import org.openrewrite.staticanalysis.StringLiteralEquality; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.groovy.Assertions.groovy; + +class StringLiteralEqualityTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new StringLiteralEquality()); + } + + // Don't change for Groovy because in Groovy, `==` means structural equality and it's redundant to call equals(). + @Test + void doNotChangeForGroovy() { + rewriteRun( + groovy( + """ + def foo(String token) { + if (token == "Foo" ) { + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/kotlin/NeedBracesTest.java b/src/test/java/org/openrewrite/staticanalysis/kotlin/NeedBracesTest.java new file mode 100644 index 000000000..0e2fa5357 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/kotlin/NeedBracesTest.java @@ -0,0 +1,96 @@ +/* + * Copyright 2023 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.kotlin; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.openrewrite.staticanalysis.NeedBraces; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.kotlin.Assertions.kotlin; + +class NeedBracesTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new NeedBraces()); + } + + @Disabled("AutoFormat needs to be updated to support Kotlin") + @Test + void addBracesForIfBranch() { + rewriteRun( + kotlin( + """ + fun getSymbol(num : Int) : String { + if (num > 0) return "+" + + return "-" + } + """, + """ + fun getSymbol(num : Int) : String { + if (num > 0) { + return "+" + } + + return "-" + } + """ + ) + ); + } + + @Disabled("AutoFormat needs to be updated to support Kotlin") + @Test + void addBracesForElseBranch() { + rewriteRun( + kotlin( + """ + fun getSymbol(num : Int) : String { + return if (num > 0) + "+" + else "-" + } + """, + """ + fun getSymbol(num : Int) : String { + return if (num > 0) { + "+" + } else { + "-" + } + } + """ + ) + ); + } + + @Disabled("AutoFormat needs to be updated to support Kotlin") + @Test + void doNotChangeForArguments() { + rewriteRun( + kotlin( + """ + fun run(foo: String, bar: String) {} + var x = run( if (true) "" else "", + if (true) "" else "") + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/kotlin/RenameLocalVariablesToCamelCaseTest.java b/src/test/java/org/openrewrite/staticanalysis/kotlin/RenameLocalVariablesToCamelCaseTest.java new file mode 100644 index 000000000..a4e90b4b2 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/kotlin/RenameLocalVariablesToCamelCaseTest.java @@ -0,0 +1,92 @@ +/* + * Copyright 2023 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.kotlin; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.openrewrite.staticanalysis.RenameLocalVariablesToCamelCase; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.kotlin.Assertions.kotlin; + +class RenameLocalVariablesToCamelCaseTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new RenameLocalVariablesToCamelCase()); + } + + @Test + void regular() { + rewriteRun( + kotlin( + """ + fun foo() { + var EMPTY_METAS = HashMap() + EMPTY_METAS.isEmpty() + } + """, + """ + fun foo() { + var emptyMetas = HashMap() + emptyMetas.isEmpty() + } + """ + ) + ); + } + + @Disabled("A bug to be fixed") + @Test + void renameBothVariableAndUsage() { + rewriteRun( + kotlin( + """ + class MqttRegex (val topic : String) + """ + ), + kotlin( + """ + fun foo() { + val MQTT = MqttRegex("topic1") + val x = listOf("", MQTT.topic) + } + """, + """ + fun foo() { + val mqtt = MqttRegex("topic1") + val x = listOf("", mqtt.topic) + } + """ + ) + ); + } + + // `internal` modifier means package-private in Kotlin, so it's not a local variable + @Test + void doNotChangeIfHasInternalModifier() { + rewriteRun( + kotlin( + """ + internal val EMPTY_METAS = HashMap() + """ + ) + ); + } + + +} diff --git a/src/test/java/org/openrewrite/staticanalysis/kotlin/ReplaceLambdaWithMethodReferenceTest.java b/src/test/java/org/openrewrite/staticanalysis/kotlin/ReplaceLambdaWithMethodReferenceTest.java new file mode 100644 index 000000000..05ed6eac2 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/kotlin/ReplaceLambdaWithMethodReferenceTest.java @@ -0,0 +1,99 @@ +/* + * Copyright 2023 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.kotlin; + +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; +import org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + + +import static org.openrewrite.kotlin.Assertions.kotlin; + +class ReplaceLambdaWithMethodReferenceTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceLambdaWithMethodReference()); + } + + @ExpectedToFail("Kotlin visitor to be implemented") + @Test + void toQualifiedMethodReference() { + rewriteRun( + kotlin( + """ + interface Pet { + fun move() { + } + } + + class Cat : Pet { + override fun move() { + println("Cat is moving") + } + } + + class Dog : Pet { + override fun move() { + println("Dog is moving") + } + } + """ + ), + kotlin( + """ + fun main() { + val pets = listOf(Cat(), Dog()) + pets.forEach { it.move() } + } + """, + """ + fun main() { + val pets = listOf(Cat(), Dog()) + pets.forEach ( Pet::move ) + } + """ + ) + ); + } + + @ExpectedToFail("Kotlin visitor to be implemented") + @Test + void toUnqualifiedMethodReference() { + rewriteRun( + kotlin( + """ + fun isEven(number: Int): Boolean { + return number % 2 == 0 + } + """ + ), + kotlin( + """ + val numbers = listOf(1, 2, 3, 4, 5) + val evenNumbers = numbers.filter{isEven(it)} + """ + , + """ + val numbers = listOf(1, 2, 3, 4, 5) + val evenNumbers = numbers.filter(::isEven) + """ + ) + ); + } + +} diff --git a/src/test/java/org/openrewrite/staticanalysis/kotlin/SimplifyBooleanExpressionTest.java b/src/test/java/org/openrewrite/staticanalysis/kotlin/SimplifyBooleanExpressionTest.java new file mode 100644 index 000000000..6d18706eb --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/kotlin/SimplifyBooleanExpressionTest.java @@ -0,0 +1,112 @@ +/* + * Copyright 2023 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.kotlin; + +import org.junit.jupiter.api.Test; +import org.openrewrite.staticanalysis.SimplifyBooleanExpression; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.kotlin.Assertions.kotlin; + +class SimplifyBooleanExpressionTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SimplifyBooleanExpression()); + } + + @Test + void regular() { + rewriteRun( + kotlin( + """ + fun getSymbol() : String? { + return null + } + """ + ), + kotlin( + """ + val isPositive = getSymbol().equals("+") == true + """, + """ + val isPositive = getSymbol().equals("+") + """ + ) + ); + } + + @Test + void doNotChangeWithNullable() { + rewriteRun( + kotlin( + """ + fun getSymbol() : String? { + return null + } + + val isPositive = getSymbol()?.equals("+") == true + """ + ) + ); + } + + @Test + void nullableChain() { + rewriteRun( + kotlin( + """ + fun getSymbol() : String? { + return null + } + """ + ), + kotlin( + """ + val isPositive1 = getSymbol()?.plus("").equals("+") == true + val isPositive2 = getSymbol()?.plus("")?.equals("+") == true + """, + """ + val isPositive1 = getSymbol()?.plus("").equals("+") + val isPositive2 = getSymbol()?.plus("")?.equals("+") == true + """ + ) + ); + } + + @Test + void nullableVariable() { + rewriteRun( + kotlin( + """ + fun main() { + val name : String? = null + val isPositive1 = name?.equals("+") == true + val isPositive2 = name.equals("+") == true + } + """, + """ + fun main() { + val name : String? = null + val isPositive1 = name?.equals("+") == true + val isPositive2 = name.equals("+") + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/kotlin/StringLiteralEqualityTest.java b/src/test/java/org/openrewrite/staticanalysis/kotlin/StringLiteralEqualityTest.java new file mode 100644 index 000000000..dc51298fd --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/kotlin/StringLiteralEqualityTest.java @@ -0,0 +1,49 @@ +/* + * Copyright 2023 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.kotlin; + +import org.junit.jupiter.api.Test; +import org.openrewrite.staticanalysis.StringLiteralEquality; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.kotlin.Assertions.kotlin; + +class StringLiteralEqualityTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new StringLiteralEquality()); + } + + // Don't change for Kotlin because In Kotlin, `==` means structural equality and it's redundant to call equals(). + @Test + void doNotChangeForKotlin() { + rewriteRun( + kotlin( + """ + class MqttRegex() { + val str: java.lang.String = java.lang.String("123") + fun processToken(token: String?) { + if (token == null || "" == token.trim { it <= ' ' }) { + } + } + } + """ + ) + ); + } +}