Skip to content

Comments

Fix ChangeType recipe's path resolution logic for substring and directory matches#6743

Open
KaushikV1017 wants to merge 2 commits intoopenrewrite:mainfrom
KaushikV1017:fix/change-type-source-path
Open

Fix ChangeType recipe's path resolution logic for substring and directory matches#6743
KaushikV1017 wants to merge 2 commits intoopenrewrite:mainfrom
KaushikV1017:fix/change-type-source-path

Conversation

@KaushikV1017
Copy link

@KaushikV1017 KaushikV1017 commented Feb 14, 2026

What's changed?

  • ChangeType path logic (rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java): Source path is updated only when it ends with oldFqn + file extension (.java, .groovy, etc). That suffix is replaced with newFqn + file extension. In all other cases the path is left unchanged.
  • Two new tests in ChangeTypeTest.java:
    • doNotRenameFileWhenPathAlreadyHasNewTypeName() – path already has the new type name in the filename; path must not be altered again.
    • doNotCorruptPathWhenDirectoryMatchesOldFqn() – path contains a directory that matches oldFqn; only the filename suffix is considered for renaming, so the path is not corrupted.

What's your motivation?

The ChangeType recipe's logic for updating the source path has faults due to the use of the replaceFirst() string method. Examples of where this comes up are highlighted below:

  1. Substring in filename: Changing type AANew with file path a/ANew.java (the file still has class A inside) was turning the path into a/ANewNew.java because replaceFirst("A", "ANew") matched the "A" in "ANew". See the doNotRenameFileWhenPathAlreadyHasNewTypeName() test for an example.
  2. Directory name matches old FQN: Path src/main/java/com/example/Original/Original.java and change com.example.Originalcom.example.NewName was producing a path like .../NewName/Original.java by replacing the directory segment; the type was correct but the file path was wrong. The path should stay as-is when the match is not the filename suffix. See the doNotCorruptPathWhenDirectoryMatchesOldFqn() test for an example.

Anything in particular you'd like reviewers to focus on?

  • The path-update condition: only when sourcePath ends with oldFqn + extension, replace that suffix with newFqn + "extension.
  • The two new tests and whether they should carry an @Issue (I saw this being used in other tests, so wanted to see if it was needed for these new ones)

Any additional context

  • No changes to recipe behavior when the path already ends with oldFqn + extension (e.g. a/b/Original.javax/y/Target.java); that case is unchanged and still covered by existing tests (e.g. renameClassAndFilePath).
  • Original tests still pass

@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Feb 14, 2026
@KaushikV1017 KaushikV1017 changed the title Fix ChangeType recipe's path resolution logic for substring and direc… Fix ChangeType recipe's path resolution logic for substring and directory matches Feb 14, 2026
@timtebeek timtebeek self-requested a review February 14, 2026 14:12
@timtebeek
Copy link
Member

Thanks for pointing out a limitation; not sure if I agree with making this recipe Java specific though, and we see a related failure:

ChangeTypeAdaptabilityTest > changeDefinition() FAILED
    org.opentest4j.AssertionFailedError: 
    expected: "file.groovy"
     but was: "newFile.groovy"
        at app//org.openrewrite.groovy.ChangeTypeAdaptabilityTest.lambda$changeDefinition$1(ChangeTypeAdaptabilityTest.java:56)
        at app//org.openrewrite.groovy.ChangeTypeAdaptabilityTest$$Lambda/0x00007f5ff050ae28.accept0(Unknown Source)
        at app//org.openrewrite.internal.ThrowingConsumer.accept(ThrowingConsumer.java:26)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:587)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:134)
        at app//org.openrewrite.groovy.ChangeTypeAdaptabilityTest.changeDefinition(ChangeTypeAdaptabilityTest.java:44)
```

Keep in mind that we reuse this recipe across languages; perhaps we can adjust the matching not to expect a specific file extension, but to only look past the last slash?

@timtebeek timtebeek marked this pull request as draft February 17, 2026 09:32
@KaushikV1017
Copy link
Author

@timtebeek Thanks for the comment! I've revised the logic to make this apply for all file types, not just .java files. We do this by finding the index of the last ".", and extract the extension from there.

@KaushikV1017 KaushikV1017 marked this pull request as ready for review February 18, 2026 21:36
@KaushikV1017
Copy link
Author

@timtebeek Let me know if there's any other changes you'd like me to make here. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants