Skip to content

Fix ReplaceConstantWithAnotherConstant leaving stale import when replacing enum constants#7014

Merged
MBoegers merged 1 commit intomainfrom
fix/replace-constant-enum-import-removal
Mar 18, 2026
Merged

Fix ReplaceConstantWithAnotherConstant leaving stale import when replacing enum constants#7014
MBoegers merged 1 commit intomainfrom
fix/replace-constant-enum-import-removal

Conversation

@MBoegers
Copy link
Contributor

Summary

ReplaceConstantWithAnotherConstant correctly rewrote the constant reference (e.g. OldValue.ANewValue.B) but failed to remove the old import com.constant.OldValue when the replaced constant was an enum constant.

Root Cause

After the visitor modifies the tree, RemoveImport re-scans it via TypesInUse to determine whether the old type is still referenced. For regular static constants (e.g. File.pathSeparator), the J.FieldAccess expression type and the name identifier type are the field's value type (e.g. String) — unrelated to the owning class — so no stale reference remains after the target identifier is rewritten.

For enum constants (e.g. OldValue.A), the expression type is the enum type itself. Two LST nodes were left carrying com.constant.OldValue after replacement:

  1. J.Identifier (the constant name A/B) — its .type is the enum type
  2. J.FieldAccess — its expression .type is also the enum type

TypesInUse picked these up and RemoveImport concluded the old type was still in use, keeping the stale import.

Fix

After computing the new target, update name.type and fieldAccess.type when they reference the old owning type. Both updates are guarded by TypeUtils.isOfClassType so they only fire for the enum-constant case; for regular constants (where those types are unrelated to the owner), the checks are no-ops.

The duplicate name assignment that existed in both if/else branches was also hoisted out, and the redundant instanceof JavaType.FullyQualified guard before isOfClassType (which performs that check internally) was removed.

Test plan

  • New replaceEnumConstant test covers the broken scenario end-to-end
  • All existing ReplaceConstantWithAnotherConstantTest tests continue to pass

…um constants

For enum constants (e.g. OldValue.A), the J.FieldAccess expression type and
the name identifier type both carry the owning enum type, unlike regular
static constants where the expression type is the field's value type (e.g.
String). This caused TypesInUse to still find a reference to the old owning
type after replacement, preventing RemoveImport from cleaning up the stale
import.

Fix by updating name.type and fieldAccess.type when they reference the old
owning type during field access replacement.
Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, thanks!

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Mar 17, 2026
@MBoegers MBoegers merged commit 28deafc into main Mar 18, 2026
1 check passed
@MBoegers MBoegers deleted the fix/replace-constant-enum-import-removal branch March 18, 2026 09:11
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants