org.openrewrite.java.ChangePackage precise match

[selfyguy]

https://docs.openrewrite.org/recipes/java/changepackage

When I use the above recipe to move/rename/change the types in package com.test.model.staff to com.test.model.data.staff, it picks up the types in package com.test.model.staffdetails too.

How can I restrict it to pick up only files from package .staff and not from .staffdetails ?

[timtebeek]

Hi! Thanks for the report; I think this might be a bug we hadn't encountered before. Looking at the implementation quickly it might just be this line that causes trouble:

rewrite/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java

Line 94 in 2122964

if (original.startsWith(oldPackageName)) {

Slightly further down we don't use startsWith but equals for package comparison:

rewrite/rewrite-java/src/main/java/org/openrewrite/java/ChangePackage.java

Line 102 in 2122964

if (importedPackage.equals(oldPackageName) || recursive && importedPackage.startsWith(recursivePackageNamePrefix)) {

The best way to rule this out would be to add a unit test to ChangePackageTest similar to this one

rewrite/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java

Lines 114 to 147 in 2122964

	void renameImport() {
	rewriteRun(
	java(
	"""
	package org.openrewrite;
	public class Test {
	}
	""",
	"""
	package org.openrewrite.test;
	public class Test {
	}
	"""
	),
	java(
	"""
	import org.openrewrite.Test;
	class A {
	}
	""",
	"""
	import org.openrewrite.test.Test;
	class A {
	}
	""",
	spec -> spec.afterRecipe(cu -> {
	J.Import imported = cu.getImports().get(0);
	assertThat(imported.getPackageName()).isEqualTo("org.openrewrite.test");
	})
	)
	);
	}

That unit test should then contain two java() source specifications; one using package com.test.model.staff and one using com.test.model.staffdetails. Only the source spec using com.test.model.staff` should have a second text block to indicate the desired change. The other source spec should only have a single text block.

Is this something you'd want to help resolve?

[shanman190]

@timtebeek, I think it's a little more complicated than just swapping to equals as it looks like nested packages should be moved as well.

rewrite/rewrite-java-test/src/test/java/org/openrewrite/java/ChangePackageTest.java

Lines 364 to 400 in 2122964

	void renamePackageRecursiveImported() {
	rewriteRun(
	spec -> spec.recipe(new ChangePackage(
	"org.openrewrite",
	"org.openrewrite.test",
	true
	)),
	java(
	"""
	package org.openrewrite.other;
	public class Test {}
	""",
	"""
	package org.openrewrite.test.other;
	public class Test {}
	"""
	),
	java(
	"""
	import org.openrewrite.other.Test;
	class A {
	Test test = null;
	}
	""",
	"""
	import org.openrewrite.test.other.Test;
	class A {
	Test test = null;
	}
	""",
	spec -> spec.afterRecipe(cu -> {
	assertThat(cu.findType("org.openrewrite.other.Test")).isEmpty();
	assertThat(cu.findType("org.openrewrite.test.other.Test")).isNotEmpty();
	})
	)
	);
	}

I suspect that these all should be equals exact and then starts with the package name followed by a dot. That should handle the case of exact package and all subpackages.

[Dinozavvvr]

Hi everyone! I'm a user of your project and I'm interested in contributing to it. Can I try to help you in this and maybe some other issues if you don't mind?

[timtebeek]

Thanks for your sharp eye as always @shanman190 , and thanks for the offer to help @Dinozavvvr .

I'm a user of your project and I'm interested in contributing to it. Can I try to help you in this and maybe some other issues if you don't mind?

All help is welcome! Best to start with a draft pull request that just adds tests to replicate the issue at hand here, and then from there we can see what changes would make the test pass to fix the issue.

[Dinozavvvr]

Is something need from me now? I wrote tests and fixed issue @timtebeek

[timtebeek]

Thanks a lot! Hadn't seen your PR pop up just yet as drafts don't trigger a notification for me yet. I see it now though:

• org.openrewrite.java.ChangePackage precise package match #4189

I'm traveling right now, but will be back Thursday for a closer look; great to see you picked this up and worked out how to make the tests pass.

[timtebeek]

Thanks a lot @Dinozavvvr ; Change should make it into our next release, and can already be tried out from the latest snapshot version.