Skip to content

Make subpackage recursion explicit in the Jackson JAX-RS JSON rename - #1185

Merged
timtebeek merged 1 commit into
mainfrom
tim/changepackage-recursive-subpackages
Aug 5, 2026
Merged

Make subpackage recursion explicit in the Jackson JAX-RS JSON rename#1185
timtebeek merged 1 commit into
mainfrom
tim/changepackage-recursive-subpackages

Conversation

@timtebeek

@timtebeek timtebeek commented Aug 5, 2026

Copy link
Copy Markdown
Member

com.fasterxml.jackson.jaxrs.jsoncom.fasterxml.jackson.jakarta.rs.json in jakarta-ee-9.yml omits recursive while depending on it, so types in the .annotation subpackage were never migrated.

Verified against the published artifacts — jackson-jaxrs-json-provider 2.14.3 → jackson-jakarta-rs-json-provider 2.15.4:

old new
com.fasterxml.jackson.jaxrs.json.annotation com.fasterxml.jackson.jakarta.rs.json.annotation

The subpackage maps exactly under prefix substitution, so recursion is both safe and required here.

I audited the other 18 recursive-omitting rules in this repo the same way (the DataNucleus set, org.apache.shiro.codec, javax.annotation.security / .sql, javax.security.cert, org.apache.commons.fileupload2.jakarta.servlet5, …) and they are all leaf packages, so this is the only one that needs changing. Two are deliberately left non-recursive because recursion would be wrong: com.alibaba.fastjsoncom.alibaba.fastjson2 (only .annotation has a counterpart; fastjson2 has no .parser / .serializer / .support.*), and com.sun.net.ssljavax.net.ssl (no .internal counterpart).

Why the tests didn't catch this

ChangePackage.recursive is @Nullable with required = false and no documented default, and a null was read two different ways inside the same recipe: non-recursive by its preconditions, recursive by its visitor. The upshot is that a subpackage type gets renamed only if the file also references a type sitting directly in oldPackageName — which every existing fixture happens to do. A real source file importing only the subpackage type was never migrated.

Verification

  • Full ./gradlew test green. Each new test fails without the recursive: true line and passes with it. Correct on its own merits and safe to merge now, independently of rewrite#8382.

`com.fasterxml.jackson.jaxrs.json` -> `com.fasterxml.jackson.jakarta.rs.json`
omits `recursive` while depending on it, so types in the `.annotation`
subpackage were never migrated.

Verified against the published artifacts:

    jackson-jaxrs-json-provider 2.14.3       -> jackson-jakarta-rs-json-provider 2.15.4
      com.fasterxml.jackson.jaxrs.json.annotation -> com.fasterxml.jackson.jakarta.rs.json.annotation

The subpackage maps exactly under prefix substitution, so recursion is both
safe and required here.

The existing tests could not catch this. `ChangePackage.recursive` is
`@Nullable` with no documented default, and a null was read as non-recursive
by the recipe's preconditions but as recursive by its visitor, so a subpackage
type was renamed only for files that also referenced a type sitting directly
in `oldPackageName`. The new test imports only `...jaxrs.json.annotation.JSONP`
and fails without this change.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Aug 5, 2026
@timtebeek
timtebeek merged commit 55f2e93 into main Aug 5, 2026
1 check passed
@timtebeek
timtebeek deleted the tim/changepackage-recursive-subpackages branch August 5, 2026 09:11
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Aug 5, 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.

1 participant