Skip to content

Make a null recursive consistently mean non-recursive in ChangePackage #8382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timtebeek merged 3 commits into from
Aug 5, 2026
Merged

Make a null recursive consistently mean non-recursive in ChangePackage #8382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ac0430
Make a null `recursive` consistently mean non-recursive in `ChangePac…
timtebeek Aug 4, 2026
240bb74
Collapse the duplicated type-vs-subpackage convention into one helper
timtebeek Aug 4, 2026
5c3bdcb
Address review: flatten the field-access guards, condense the tests
timtebeek Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 132 additions & 1 deletion rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Comment thread
timtebeek marked this conversation as resolved.

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() {
Expand Down Expand Up @@ -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"))
)
);
}
Expand Down
70 changes: 48 additions & 22 deletions rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -84,16 +89,16 @@ public TreeVisitor<?, ExecutionContext> 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)) {
Expand All @@ -115,10 +120,9 @@ public TreeVisitor<?, ExecutionContext> 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);
}
}
Expand All @@ -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<Tree, List<Reference>> matches = new HashMap<>();
for (Reference ref : references.findMatches(matcher)) {
matches.computeIfAbsent(ref.getTree(), k -> new java.util.ArrayList<>()).add(ref);
Expand All @@ -165,18 +168,41 @@ private class JavaChangePackageVisitor extends JavaVisitor<ExecutionContext> {
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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
Expand Down
30 changes: 25 additions & 5 deletions rewrite-java/src/main/java/org/openrewrite/java/PackageMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
}
}
Expand Down
Loading
Loading