`ChangeType` leaves the outer class import when replacing a doubly nested type

Open
  • Is something wrong?
#9,788 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
Half a day
Newbie friendliness
62/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
java
Domain
compilers

Research direction

Start in ChangeType.visitFieldAccess, where maybeRemoveImport is called on oldType.getOwningClass() after the field access is replaced. The bug is that for a doubly nested type like a.Outer$Middle$Inner the import of the leftmost class a.Outer is never considered. Reproduce with the test in the issue, which expects the a.Outer import to be removed alongside the rewrite to b.Inner. Done when that test passes and the existing ChangeType tests still pass.

Written by the indexing model from the issue text.

Description

bug recipe

ChangeType leaves an unused import behind when the old type is nested two or more levels deep and the code reaches it through the top-level class, as in Outer.Middle.Inner. We ran into this migrating OpenAiAudioApi.SpeechRequest.Voice to OpenAiAudioSpeechOptions.Voice for Spring AI 2.0, which left import org.springframework.ai.openai.api.OpenAiAudioApi; behind. That class no longer exists in Spring AI 2.0, so the result doesn't compile.

What version of OpenRewrite are you using?

rewrite-java 8.96.0-SNAPSHOT; the same code is on main.

What is the smallest, simplest way to reproduce the problem?

@Test
void removeImportOfOutermostClass() {
    rewriteRun(
      spec -> spec.recipe(new ChangeType("a.Outer$Middle$Inner", "b.Inner", true)),
      java("package a; public class Outer { public static class Middle { public enum Inner { ONE } } }"),
      java("package b; public enum Inner { ONE }"),
      java(
        """
          import a.Outer;

          class Test {
              Object o = Outer.Middle.Inner.ONE;
          }
          """,
        """
          import b.Inner;

          class Test {
              Object o = Inner.ONE;
          }
          """
      )
    );
}

What did you see instead?

import a.Outer;
import b.Inner;

class Test {
    Object o = Inner.ONE;
}

Cause

ChangeType.visitFieldAccess calls maybeRemoveImport(oldType.getOwningClass()) after replacing the field access. For a.Outer$Middle$Inner, that owning class is a.Outer$Middle, which nothing imports. The a.Outer import that the field access actually used is never considered. With a type nested only one level deep, the owning class is the top-level class and the import is removed as expected.

A fix could remove the import of the class that the leftmost identifier of the replaced field access refers to, or walk up all the owning classes.

Dominant language
Java
Stars
3.9k
Forks
599
Avg merge
9h 38m
Merged PRs (30d)
557

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from openrewrite/rewrite

All issues in openrewrite/rewrite

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.