diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java index 803de188e35..ec5cd0474a9 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java @@ -16,6 +16,7 @@ package org.openrewrite.java; import org.intellij.lang.annotations.Language; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; @@ -340,6 +341,7 @@ class Test { @Test void renamePackageRecursive() { rewriteRun( + spec -> spec.recipe(new ChangePackage("org.openrewrite", "org.openrewrite.test", true)), java( """ package org.openrewrite.internal; @@ -573,6 +575,126 @@ class A { ); } + /** + * A null {@code recursive} means non-recursive, and it means that for every source kind and + * regardless of what else the source happens to reference. Before this was pinned down, a null + * {@code recursive} was read as non-recursive by the precondition but as recursive by the + * visitor, so a subpackage type was renamed only when the same file also referenced a type + * sitting directly in {@code oldPackageName}. + */ + @Nested + class NullRecursiveDefaultsToNonRecursive { + + private static final JavaParser.Builder cucumber = JavaParser.fromJavaVersion().dependsOn( + """ + package cucumber.api.java; + public @interface Before {} + """, + """ + package cucumber.api.java.en; + public @interface Given {} + """ + ); + + @Test + void nullRecursiveIsNonRecursiveForEverySourceKind() { + rewriteRun( + spec -> spec.recipe(new ChangePackage("cucumber.api.java", "io.cucumber.java", null)) + .parser(cucumber), + // Only a subpackage type: untouched. + java( + """ + import cucumber.api.java.en.Given; + + class A { + @Given + void given() {} + } + """ + ), + // Both: the type directly in the old package moves, the subpackage one stays put. + java( + """ + import cucumber.api.java.Before; + import cucumber.api.java.en.Given; + + class B { + @Before + void before() {} + + @Given + void given() {} + } + """, + """ + import io.cucumber.java.Before; + import cucumber.api.java.en.Given; + + class B { + @Before + void before() {} + + @Given + void given() {} + } + """ + ), + properties( + """ + given=cucumber.api.java.en.Given + """, + spec -> spec.path("application.properties") + ), + properties( + """ + before=cucumber.api.java.Before + given=cucumber.api.java.en.Given + """, + """ + before=io.cucumber.java.Before + given=cucumber.api.java.en.Given + """, + spec -> spec.path("application-extra.properties") + ) + ); + } + + @Test + void recursiveOptsIntoSubpackagesForEverySourceKind() { + rewriteRun( + spec -> spec.recipe(new ChangePackage("cucumber.api.java", "io.cucumber.java", true)) + .parser(cucumber), + java( + """ + import cucumber.api.java.en.Given; + + class A { + @Given + void given() {} + } + """, + """ + import io.cucumber.java.en.Given; + + class A { + @Given + void given() {} + } + """ + ), + properties( + """ + given=cucumber.api.java.en.Given + """, + """ + given=io.cucumber.java.en.Given + """, + spec -> spec.path("application.properties") + ) + ); + } + } + @Issue("https://github.com/openrewrite/rewrite/issues/1997") @Test void typeParameter() { @@ -2182,8 +2304,17 @@ void changePackageInServiceProviderFileNonRecursive() { rewriteRun( spec -> spec.recipe(new ChangePackage("org.foo", "org.bar", false)), text( - "org.foo.MyImpl\n", + """ + org.foo.MyImplA + org.foo.sub.MyImplB + """, + """ + org.bar.MyImplA + org.foo.sub.MyImplB + """, spec -> spec.path("META-INF/services/org.foo.MyInterface") + .afterRecipe(pt -> assertThat(pt.getSourcePath().toString().replace('\\', '/')) + .isEqualTo("META-INF/services/org.bar.MyInterface")) ) ); } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java b/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java index d824e672986..112f532a056 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java @@ -57,11 +57,16 @@ public class ChangePackage extends Recipe { @With @Option(displayName = "Recursive", - description = "Recursively change subpackage names", + description = "Recursively change subpackage names. Defaults to `false`, renaming only types " + + "directly in `oldPackageName`; set to `true` to also rename types in its subpackages.", required = false) @Nullable Boolean recursive; + private boolean isRecursive() { + return Boolean.TRUE.equals(recursive); + } + @Override public String getInstanceNameSuffix() { return String.format("`%s` to `%s`", oldPackageName, newPackageName); @@ -84,16 +89,16 @@ public TreeVisitor getVisitor() { @Override public @Nullable Tree preVisit(@Nullable Tree tree, ExecutionContext ctx) { stopAfterPreVisit(); + boolean recursive = isRecursive(); + String recursivePackageNamePrefix = oldPackageName + "."; if (tree instanceof JavaSourceFile) { JavaSourceFile cu = (JavaSourceFile) tree; if (cu.getPackageDeclaration() != null) { String original = PackageNameUtils.getPackageName(cu.getPackageDeclaration()); - if (original.startsWith(oldPackageName)) { + if (original.equals(oldPackageName) || recursive && original.startsWith(recursivePackageNamePrefix)) { return SearchResult.found(cu); } } - boolean recursive = Boolean.TRUE.equals(ChangePackage.this.recursive); - String recursivePackageNamePrefix = oldPackageName + "."; for (J.Import anImport : cu.getImports()) { String importedPackage = anImport.getPackageName(); if (importedPackage.equals(oldPackageName) || recursive && importedPackage.startsWith(recursivePackageNamePrefix)) { @@ -115,10 +120,9 @@ public TreeVisitor getVisitor() { } } else if (tree instanceof SourceFileWithReferences) { SourceFileWithReferences cu = (SourceFileWithReferences) tree; - boolean recursive = Boolean.TRUE.equals(ChangePackage.this.recursive); - String recursivePackageNamePrefix = oldPackageName + "."; + PackageMatcher matcher = new PackageMatcher(oldPackageName, recursive); for (Reference ref : cu.getReferences().getReferences()) { - if (ref.getValue().equals(oldPackageName) || recursive && ref.getValue().startsWith(recursivePackageNamePrefix)) { + if (matcher.matchesReference(ref)) { return SearchResult.found(cu); } } @@ -141,8 +145,7 @@ public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { } else if (tree instanceof SourceFileWithReferences) { SourceFileWithReferences sourceFile = (SourceFileWithReferences) tree; SourceFileWithReferences.References references = sourceFile.getReferences(); - boolean recursive = Boolean.TRUE.equals(ChangePackage.this.recursive); - PackageMatcher matcher = new PackageMatcher(oldPackageName, recursive); + PackageMatcher matcher = new PackageMatcher(oldPackageName, isRecursive()); Map> matches = new HashMap<>(); for (Reference ref : references.findMatches(matcher)) { matches.computeIfAbsent(ref.getTree(), k -> new java.util.ArrayList<>()).add(ref); @@ -165,18 +168,41 @@ private class JavaChangePackageVisitor extends JavaVisitor { public J visitFieldAccess(J.FieldAccess fieldAccess, ExecutionContext ctx) { J f = super.visitFieldAccess(fieldAccess, ctx); - if (((J.FieldAccess) f).isFullyQualifiedClassReference(oldPackageName)) { - Cursor parent = getCursor().getParent(); - if (parent != null && - // Ensure the parent isn't a J.FieldAccess OR the parent doesn't match the target package name. - (!(parent.getValue() instanceof J.FieldAccess) || - (!(((J.FieldAccess) parent.getValue()).isFullyQualifiedClassReference(newPackageName))))) { - - f = TypeTree.build(((JavaType.FullyQualified) newPackageType).getFullyQualifiedName()) - .withPrefix(f.getPrefix()); + if (!((J.FieldAccess) f).isFullyQualifiedClassReference(oldPackageName)) { + return f; + } + Cursor parent = getCursor().getParent(); + if (parent == null) { + return f; + } + if (parent.getValue() instanceof J.FieldAccess) { + J.FieldAccess enclosing = (J.FieldAccess) parent.getValue(); + if (enclosing.isFullyQualifiedClassReference(newPackageName)) { + // Already rewritten to the new package. + return f; } + if (!isRecursive() && !namesTypeDirectlyInOldPackage(enclosing)) { + // Leading segments of a subpackage-qualified name such as oldPackageName.sub.Type. + return f; + } + } + return TypeTree.build(((JavaType.FullyQualified) newPackageType).getFullyQualifiedName()) + .withPrefix(f.getPrefix()); + } + + /** + * Whether the name enclosing this occurrence of {@code oldPackageName} is a type declared + * directly in it, rather than a subpackage. Uses the same leading-capital convention as + * {@link PackageMatcher} where type attribution is unavailable. + */ + private boolean namesTypeDirectlyInOldPackage(J.FieldAccess enclosing) { + JavaType.FullyQualified fq = TypeUtils.asFullyQualified(enclosing.getType()); + if (fq != null) { + return oldPackageName.equals(fq.getPackageName()); } - return f; + String nextSegment = enclosing.getSimpleName(); + return "*".equals(nextSegment) || + !nextSegment.isEmpty() && Character.isUpperCase(nextSegment.charAt(0)); } @Override @@ -282,7 +308,7 @@ public J postVisit(J tree, ExecutionContext ctx) { String oldSubPkg = oldPackageName + changingTo.substring(newPackageName.length()); sf = maybeExpandStarImport(sf, changingTo, oldSubPkg, ctx); } - if (Boolean.TRUE.equals(recursive)) { + if (isRecursive()) { for (J.Import anImport : sf.getImports()) { if (!anImport.isStatic() && "*".equals(anImport.getQualid().getSimpleName())) { String pkg = anImport.getPackageName(); @@ -518,7 +544,7 @@ private JavaType.FullyQualified findType(String fqn, JavaSourceFile cu) { } private String getNewPackageName(String packageName) { - return (recursive == null || recursive) && !newPackageName.endsWith(packageName.substring(oldPackageName.length())) ? + return isRecursive() && !newPackageName.endsWith(packageName.substring(oldPackageName.length())) ? newPackageName + packageName.substring(oldPackageName.length()) : newPackageName; } @@ -529,7 +555,7 @@ private boolean isTargetFullyQualifiedType(JavaType.@Nullable FullyQualified fq) } private boolean isTargetRecursivePackageName(String packageName) { - return (recursive == null || recursive) && + return isRecursive() && packageName.startsWith(oldPackageName + ".") && !packageName.startsWith(newPackageName); } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java b/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java index 92536dc8785..cb4fd7ecf5b 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java @@ -39,14 +39,34 @@ public PackageMatcher(@Nullable String targetPackage, boolean recursive) { @Override public boolean matchesReference(Reference reference) { if (reference.getKind() == Reference.Kind.TYPE || reference.getKind() == Reference.Kind.PACKAGE) { - String recursivePackageNamePrefix = targetPackage + "."; - if (reference.getValue().equals(targetPackage) || recursive && reference.getValue().startsWith(recursivePackageNamePrefix)) { - return true; - } + return matchesValue(reference.getValue()); } return false; } + /** + * Matches the target package itself and, when non-recursive, types declared directly in it. + * Subpackages and the types in them match only when recursive. + */ + boolean matchesValue(String value) { + if (targetPackage == null) { + return false; + } + return value.equals(targetPackage) || + value.startsWith(targetPackage + ".") && (recursive || namesTypeDirectlyIn(value, targetPackage)); + } + + /** + * Whether the segment following {@code pkg} names a type rather than a subpackage, inferred from + * a leading capital. A dotted string alone cannot say where the package ends, so this convention + * is what the reference model already uses to tell {@link Reference.Kind#TYPE} from + * {@link Reference.Kind#PACKAGE} when the providers in rewrite-properties, rewrite-yaml, + * rewrite-xml and the service-provider reader build these references in the first place. + */ + private boolean namesTypeDirectlyIn(String value, String pkg) { + return value.length() > pkg.length() + 1 && Character.isUpperCase(value.charAt(pkg.length() + 1)); + } + @Override public Reference.Renamer createRenamer(String newName) { return reference -> getReplacement(reference.getValue(), targetPackage, newName); @@ -57,7 +77,7 @@ String getReplacement(String value, @Nullable String oldValue, String newValue) if (value.equals(oldValue)) { return newValue; } else if (value.startsWith(oldValue)) { - if (recursive || value.length() > oldValue.length() + 1 && Character.isUpperCase(value.charAt(oldValue.length() + 1))) { + if (recursive || namesTypeDirectlyIn(value, oldValue)) { return newValue + value.substring(oldValue.length()); } } diff --git a/rewrite-java/src/main/resources/META-INF/rewrite/recipes.csv b/rewrite-java/src/main/resources/META-INF/rewrite/recipes.csv index db51e96a643..4fcb69ad50a 100644 --- a/rewrite-java/src/main/resources/META-INF/rewrite/recipes.csv +++ b/rewrite-java/src/main/resources/META-INF/rewrite/recipes.csv @@ -14,7 +14,7 @@ maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangeMethodInvocationRe maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangeMethodName,Change method name,Rename a method.,1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""methodPattern"",""type"":""String"",""displayName"":""Method pattern"",""description"":""A [method pattern](https://docs.openrewrite.org/reference/method-patterns) is used to find matching method invocations. For example, to find all method invocations in the Guava library, use the pattern: `com.google.common..*#*(..)`.

The pattern format is `#()`.

`..*` includes all subpackages of `com.google.common`.
`*(..)` matches any method name with any number of arguments.

For more specific queries, like Guava's `ImmutableMap`, use `com.google.common.collect.ImmutableMap#*(..)` to narrow down the results."",""example"":""org.mockito.Matchers anyVararg()"",""required"":true},{""name"":""newMethodName"",""type"":""String"",""displayName"":""New method name"",""description"":""The method name that will replace the existing name."",""example"":""any"",""required"":true},{""name"":""matchOverrides"",""type"":""Boolean"",""displayName"":""Match on overrides"",""description"":""When enabled, find methods that are overrides of the method pattern.""},{""name"":""ignoreDefinition"",""type"":""Boolean"",""displayName"":""Ignore type definition"",""description"":""When set to `true` the definition of the old type will be left untouched. This is useful when you're replacing usage of a class but don't want to rename it.""}]", maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangeMethodTargetToStatic,Change method target to static,Change method invocations to static method calls.,1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""methodPattern"",""type"":""String"",""displayName"":""Method pattern"",""description"":""The original method call may or may not be a static method invocation. A [method pattern](https://docs.openrewrite.org/reference/method-patterns) is used to find matching method invocations. For example, to find all method invocations in the Guava library, use the pattern: `com.google.common..*#*(..)`.

The pattern format is `#()`.

`..*` includes all subpackages of `com.google.common`.
`*(..)` matches any method name with any number of arguments.

For more specific queries, like Guava's `ImmutableMap`, use `com.google.common.collect.ImmutableMap#*(..)` to narrow down the results."",""example"":""com.google.common.collect.ImmutableSet of(..)"",""required"":true},{""name"":""fullyQualifiedTargetTypeName"",""type"":""String"",""displayName"":""Fully-qualified target type name"",""description"":""A fully-qualified class name of the type upon which the static method is defined."",""example"":""java.util.Set"",""required"":true},{""name"":""returnType"",""type"":""String"",""displayName"":""Return type after change"",""description"":""Sometimes changing the target type also changes the return type. In the Guava example, changing from `ImmutableSet#of(..)` to `Set#of(..)` widens the return type from Guava's `ImmutableSet` to just `java.util.Set`."",""example"":""java.util.Set""},{""name"":""matchOverrides"",""type"":""Boolean"",""displayName"":""Match on overrides"",""description"":""When enabled, find methods that are overrides of the method pattern.""},{""name"":""matchUnknownTypes"",""type"":""Boolean"",""displayName"":""Match unknown types"",""description"":""When enabled, include method invocations which appear to match if full type information is missing. Using matchUnknownTypes can improve recipe resiliency for an AST with missing type information, but also increases the risk of false-positive matches on unrelated method invocations.""}]", maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangeMethodTargetToVariable,Change method target to variable,Change method invocations to method calls on a variable.,1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""methodPattern"",""type"":""String"",""displayName"":""Method pattern"",""description"":""A [method pattern](https://docs.openrewrite.org/reference/method-patterns) is used to find matching method invocations. For example, to find all method invocations in the Guava library, use the pattern: `com.google.common..*#*(..)`.

The pattern format is `#()`.

`..*` includes all subpackages of `com.google.common`.
`*(..)` matches any method name with any number of arguments.

For more specific queries, like Guava's `ImmutableMap`, use `com.google.common.collect.ImmutableMap#*(..)` to narrow down the results."",""example"":""org.mycorp.A method(..)"",""required"":true},{""name"":""variableName"",""type"":""String"",""displayName"":""Variable name"",""description"":""Name of variable to use as target for the modified method invocation."",""example"":""foo"",""required"":true},{""name"":""variableType"",""type"":""String"",""displayName"":""Variable type"",""description"":""Type attribution to use for the return type of the modified method invocation."",""example"":""java.lang.String"",""required"":true},{""name"":""matchOverrides"",""type"":""Boolean"",""displayName"":""Match on overrides"",""description"":""When enabled, find methods that are overrides of the method pattern.""}]", -maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangePackage,Rename package name,"A recipe that will rename a package name in package statements, imports, and fully-qualified types.",1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""oldPackageName"",""type"":""String"",""displayName"":""Old package name"",""description"":""The package name to replace."",""example"":""com.yourorg.foo"",""required"":true},{""name"":""newPackageName"",""type"":""String"",""displayName"":""New package name"",""description"":""New package name to replace the old package name with."",""example"":""com.yourorg.bar"",""required"":true},{""name"":""recursive"",""type"":""Boolean"",""displayName"":""Recursive"",""description"":""Recursively change subpackage names""}]", +maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangePackage,Rename package name,"A recipe that will rename a package name in package statements, imports, and fully-qualified types.",1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""oldPackageName"",""type"":""String"",""displayName"":""Old package name"",""description"":""The package name to replace."",""example"":""com.yourorg.foo"",""required"":true},{""name"":""newPackageName"",""type"":""String"",""displayName"":""New package name"",""description"":""New package name to replace the old package name with."",""example"":""com.yourorg.bar"",""required"":true},{""name"":""recursive"",""type"":""Boolean"",""displayName"":""Recursive"",""description"":""Recursively change subpackage names. Defaults to `false`, renaming only types directly in `oldPackageName`; set to `true` to also rename types in its subpackages.""}]", maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangePackageInStringLiteral,Rename package name in String literals,A recipe that will rename a package name in String literals.,1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""oldPackageName"",""type"":""String"",""displayName"":""Old package name"",""description"":""The package name to replace."",""example"":""com.yourorg.foo"",""required"":true},{""name"":""newPackageName"",""type"":""String"",""displayName"":""New package name"",""description"":""New package name to replace the old package name with."",""example"":""com.yourorg.bar"",""required"":true}]", maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangeStaticFieldToMethod,Change static field access to static method access,Migrate accesses to a static field to invocations of a static method.,1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""oldClassName"",""type"":""String"",""displayName"":""Old class name"",""description"":""The fully qualified name of the class containing the field to replace."",""example"":""java.util.Collections"",""required"":true},{""name"":""oldFieldName"",""type"":""String"",""displayName"":""Old field name"",""description"":""The simple name of the static field to replace."",""example"":""EMPTY_LIST"",""required"":true},{""name"":""newClassName"",""type"":""String"",""displayName"":""New class name"",""description"":""The fully qualified name of the class containing the method to use. Leave empty to keep the same class."",""example"":""java.util.List""},{""name"":""newTarget"",""type"":""String"",""displayName"":""New field target"",""description"":""An optional method target that can be used to specify a static field within the new class."",""example"":""OK_RESPONSE""},{""name"":""newMethodName"",""type"":""String"",""displayName"":""New method name"",""description"":""The simple name of the method to use. The method must be static and have no arguments."",""example"":""of"",""required"":true}]", maven,org.openrewrite:rewrite-java,org.openrewrite.java.ChangeType,Change type,Change a given type to another.,1,,Java,,Basic building blocks for transforming Java code.,"[{""name"":""oldFullyQualifiedTypeName"",""type"":""String"",""displayName"":""Old fully-qualified type name"",""description"":""Fully-qualified class name of the original type."",""example"":""org.junit.Assume"",""required"":true},{""name"":""newFullyQualifiedTypeName"",""type"":""String"",""displayName"":""New fully-qualified type name"",""description"":""Fully-qualified class name of the replacement type, or the name of a primitive such as \""int\"". The `OuterClassName$NestedClassName` naming convention should be used for nested classes."",""example"":""org.junit.jupiter.api.Assumptions"",""required"":true},{""name"":""ignoreDefinition"",""type"":""Boolean"",""displayName"":""Ignore type definition"",""description"":""When set to `true` the definition of the old type will be left untouched. This is useful when you're replacing usage of a class but don't want to rename it.""}]",