-
Notifications
You must be signed in to change notification settings - Fork 465
Inline annotated methods through generated recipes to support multiple versions #6059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
On rewrite-migrate-java this generated 47 recipes: type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.InlineMethodCallsGenerated
displayName: Inline methods annotated with `@InlineMe`
description: >
Automatically generated recipes to inline method calls based on `@InlineMe` annotations
discovered in the type table.
recipeList:
# From com.google.guava:guava:33.4.8-jre
- org.openrewrite.java.InlineMethodCalls:
methodPattern: 'com.google.common.primitives.Booleans compare(boolean, boolean)'
replacement: 'Boolean.compare(a, b)'
classpathFromResources:
- 'guava-33.4.8-jre'
- org.openrewrite.java.InlineMethodCalls:
methodPattern: 'com.google.common.primitives.Longs compare(long, long)'
replacement: 'Long.compare(a, b)'
classpathFromResources:
- 'guava-33.4.8-jre'
... |
Here's what usage downstream looks like as a branch; to be opened and merged if this is approved and merged: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume these recipes will then be part of a corresponding framework upgrade recipe, which will then make sure that we don't run one of these recipes, when they aren't yet applicable.
I think it is a bit of a shame that we need to do it this way, but I see why we need to do something like this, as we never recompile the source code against new dependencies.
methodPattern: 'com.example.TestClass annotatedMethod()' | ||
replacement: 'replacement()' | ||
classpathFromResources: | ||
- 'test-lib-1.0.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be really nice if we could change the JavaParser.Builder#classpathFromResources()
API a bit to work with GAVs. My proposal would be that we in a first step leave the signature as is, but change it so that if the value contains a :
, then it is interpreted as a GA or GAV (depending on if there is one or two colons). For the version part we could then also allow version ranges like 1.0.+
or ~1.0.0
- 'test-lib-1.0.0' | |
- 'com.example:test-lib:1.0.0' |
I realize this isn't directly related to this PR.
… (#853) * Generate Guava InlineMethodCalls recipes using openrewrite/rewrite#6059 * Add license header * Regenerate file * Read all resources now that Guava is split across two files
…17.1 to 3.18.0 [skip ci] [//]: # (dependabot-start)⚠️ \*\*Dependabot is rebasing this PR\*\*⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Bumps [org.openrewrite.recipe:rewrite-migrate-java](https://github.com/openrewrite/rewrite-migrate-java) from 3.17.1 to 3.18.0. Release notes *Sourced from [org.openrewrite.recipe:rewrite-migrate-java's releases](https://github.com/openrewrite/rewrite-migrate-java/releases).* > 3.18.0 > ------ > > What's Changed > -------------- > > * Add OpenRewrite recipe for JEP 512 instance main methods by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-migrate-java#852](https://redirect.github.com/openrewrite/rewrite-migrate-java/pull/852) > * Generate Guava InlineMethodCalls recipes using [openrewrite/rewrite#6059](https://redirect.github.com/openrewrite/rewrite/issues/6059) by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-migrate-java#853](https://redirect.github.com/openrewrite/rewrite-migrate-java/pull/853) > * Run CI on Java 25 by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-migrate-java#851](https://redirect.github.com/openrewrite/rewrite-migrate-java/pull/851) > * Generate recipes for Guava 33.5.0-jre by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-migrate-java#854](https://redirect.github.com/openrewrite/rewrite-migrate-java/pull/854) > * Use type tables for tests by [`@knutwannheden`](https://github.com/knutwannheden) in [openrewrite/rewrite-migrate-java#856](https://redirect.github.com/openrewrite/rewrite-migrate-java/pull/856) > > **Full Changelog**: <openrewrite/rewrite-migrate-java@v3.17.1...v3.18.0> Commits * [`5a66684`](openrewrite/rewrite-migrate-java@5a66684) Use type tables for tests ([#856](https://redirect.github.com/openrewrite/rewrite-migrate-java/issues/856)) * [`1111d4a`](openrewrite/rewrite-migrate-java@1111d4a) Migrate away from micrometer nullable annotations * [`257d4bc`](openrewrite/rewrite-migrate-java@257d4bc) Generate recipes for Guava 33.5.0-jre ([#854](https://redirect.github.com/openrewrite/rewrite-migrate-java/issues/854)) * [`041816a`](openrewrite/rewrite-migrate-java@041816a) Run CI on Java 25 ([#851](https://redirect.github.com/openrewrite/rewrite-migrate-java/issues/851)) * [`bb024d5`](openrewrite/rewrite-migrate-java@bb024d5) Generate Guava InlineMethodCalls recipes using [openrewrite/rewrite#6059](https://redirect.github.com/openrewrite/rewrite/issues/6059) ([#853](https://redirect.github.com/openrewrite/rewrite-migrate-java/issues/853)) * [`2a1a482`](openrewrite/rewrite-migrate-java@2a1a482) [Auto] SDKMAN! Java candidates as of 2025-09-22T1016 * [`72818ad`](openrewrite/rewrite-migrate-java@72818ad) Add OpenRewrite recipe for JEP 512 instance main methods ([#852](https://redirect.github.com/openrewrite/rewrite-migrate-java/issues/852)) * See full diff in [compare view](openrewrite/rewrite-migrate-java@v3.17.1...v3.18.0) [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
What's changed?
Change the relatively new
InlineMethodCalls
to take arguments, and provide a generator to generate declarative recipes based on annotations found in TypeTables.What's your motivation?
We want to support multiple (major) versions of Guava and other libraries that might use the annotations only in specific versions, before methods are dropped in subsequent versions. That means we would only be able to support the version that a project was compiled against, but not a chain of versions that might follow for those same methods. Additionally we were not able to use the
classpathFromResources
for such replacements, leaving us.classpath
which does not work as well in all environments, as types might not be on the classpath there.Anything in particular you'd like reviewers to focus on?
public
. I didn't want to expose those types publicly to avoid locking ourselves in too much.Have you considered any alternatives or workarounds?
We could move the generator to
rewrite-build-gradle-plugin
instead as a separate task.Any additional context
@InlineMe
andInlineMethodCalls
recipe to replace annotated methods #5953