Recipe to remove `.toString()` called on parameterized logging statement arguments #224

pdelagrave · 2025-06-17T16:16:55Z

What's changed?

New recipe.

What's your motivation?

Fixes Remove .toString() called on parameterized logging statement arguments #223

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

I've left a few comments and questions in the PR already.
Let me know if the test coverage is sufficient.

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files

src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java

src/main/java/org/openrewrite/java/logging/slf4j/StripToStringFromArguments.java

src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java

src/main/java/org/openrewrite/java/logging/slf4j/StripToStringFromArguments.java

src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java

src/main/java/org/openrewrite/java/logging/slf4j/StripToStringFromArguments.java

src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java

timtebeek · 2025-06-18T08:36:30Z

I noticed you're working off of a forked project; know that you can push branches directly to our repositories as well. :)

timtebeek · 2025-06-18T13:16:22Z

For other recipes we support not just SLF4J, but other logging frameworks as well; can that be replicated here?
https://github.com/openrewrite/rewrite-logging-frameworks/blob/main/src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java

…ameters

…eing changed isn't going to change which log() method overload is called.

pdelagrave · 2025-06-18T19:42:14Z

For other recipes we support not just SLF4J, but other logging frameworks as well; can that be replicated here? https://github.com/openrewrite/rewrite-logging-frameworks/blob/main/src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java

So potentially do like ParametrizedLogging is doing and have the StripToStringFromArguments recipe accept parameters? The solution will depend on how different or not are the log() methods parameters between slf4j and the other framework to support. Also, I guess it'd be better to do that work in a separate PR?

timtebeek · 2025-06-18T21:44:08Z

Sure we can do that on a follow up PR. could you ask someone on the team to help you run this against all Apache LSTs locally before a merge? That could help uncover some interesting cases still.

pdelagrave · 2025-06-20T18:21:25Z

Successfully tested the recipe locally on all Apache repos. Took longer than expected because I had crashes caused by openrewrite/rewrite#5614 which I didn't know was fixed already, rebuilt the CLI from the latest main and it's all good.

…rom 3.7.0 to 3.13.0 [skip ci] Bumps [org.openrewrite.recipe:rewrite-logging-frameworks](https://github.com/openrewrite/rewrite-logging-frameworks) from 3.7.0 to 3.13.0. Release notes *Sourced from [org.openrewrite.recipe:rewrite-logging-frameworks's releases](https://github.com/openrewrite/rewrite-logging-frameworks/releases).* > 3.13.0 > ------ > > What's Changed > -------------- > > * Recipe to migrate deprecated JBoss Logging calls by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#245](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/245) > * Add `ArgumentArrayToVarargs` for logger methods that take a var args argument Object array by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-logging-frameworks#246](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/246) > * Avoid unsafe changes in `JulParameterizedArguments` when `JavaType.Array` argument is not a `J.NewArray` by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#244](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/244) > * LoggerLevelArgumentToMethod for JBoss Logging by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#243](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/243) > * Add a recipe to migrate from JBoss Logging to SLF4J by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#241](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/241) > * rename settings.local.json to settings.json by [`@zieka`](https://github.com/zieka) in [openrewrite/rewrite-logging-frameworks#247](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/247) > > **Full Changelog**: <openrewrite/rewrite-logging-frameworks@v3.12.0...v3.13.0> > > 3.12.1 > ------ > > What's Changed > -------------- > > * OpenRewrite v8.59.1: <https://github.com/openrewrite/rewrite> > > **Full Changelog**: <https://github.com/openrewrite/rewrite-logging-frameworks> > > 3.12.0 > ------ > > What's Changed > -------------- > > * Adds filePattern parameter to ConfigureLoggerLevel by [`@simonzn`](https://github.com/simonzn) in [openrewrite/rewrite-logging-frameworks#235](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/235) > * Handle existing arguments alongside concatenation in `ParameterizedLogging` by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-logging-frameworks#237](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/237) > * Fix Slf4jLogShouldBeConstant to preserve format specifiers with width, alignment, and precision. by [`@motlin`](https://github.com/motlin) in [openrewrite/rewrite-logging-frameworks#239](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/239) > > New Contributors > ---------------- > > * [`@simonzn`](https://github.com/simonzn) made their first contribution in [openrewrite/rewrite-logging-frameworks#235](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/235) > * [`@motlin`](https://github.com/motlin) made their first contribution in [openrewrite/rewrite-logging-frameworks#239](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/239) > > **Full Changelog**: <openrewrite/rewrite-logging-frameworks@v3.11.0...v3.12.0> > > 3.11.0 > ------ > > What's Changed > -------------- > > * fix: any last argument matching `Throwable.toString()` is left as is by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#229](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/229) > * refactor: org.openrewrite.mavencentral by [`@Laurens-W`](https://github.com/Laurens-W) in [openrewrite/rewrite-logging-frameworks#230](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/230) > * Don't if-wrap logs when arguments are getters by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#233](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/233) > > **Full Changelog**: <openrewrite/rewrite-logging-frameworks@v3.10.0...v3.11.0> > > 3.10.0 > ------ > > What's Changed > -------------- > > * Add recipe to change logger fields to private by [`@jhl221123`](https://github.com/jhl221123) in [openrewrite/rewrite-logging-frameworks#221](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/221) > * Add `MatchIsLogLevelEnabledWithLogStatements` to SLF4J best practices by [`@timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-logging-frameworks#222](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/222) > * Exclude spotbugs dependency by [`@greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-logging-frameworks#225](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/225) > * Recipe to remove `.toString()` called on parameterized logging statement arguments by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#224](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/224) > * fix: match `.toString()` on any type, not just java.lang.Object by [`@pdelagrave`](https://github.com/pdelagrave) in [openrewrite/rewrite-logging-frameworks#226](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/226) > > New Contributors > ---------------- > > * [`@jhl221123`](https://github.com/jhl221123) made their first contribution in [openrewrite/rewrite-logging-frameworks#221](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/221) > * [`@pdelagrave`](https://github.com/pdelagrave) made their first contribution in [openrewrite/rewrite-logging-frameworks#224](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/pull/224) ... (truncated) Commits * [`946d0ce`](openrewrite/rewrite-logging-frameworks@946d0ce) Update documentation examples * [`210eff9`](openrewrite/rewrite-logging-frameworks@210eff9) rename settings.local.json to settings.json ([#247](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/issues/247)) * [`2e67627`](openrewrite/rewrite-logging-frameworks@2e67627) Add a recipe to migrate from JBoss Logging to SLF4J ([#241](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/issues/241)) * [`c59c405`](openrewrite/rewrite-logging-frameworks@c59c405) LoggerLevelArgumentToMethod for JBoss Logging ([#243](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/issues/243)) * [`36a5200`](openrewrite/rewrite-logging-frameworks@36a5200) OpenRewrite recipe best practices * [`aa3f14f`](openrewrite/rewrite-logging-frameworks@aa3f14f) Avoid unsafe changes in `JulParameterizedArguments` when `JavaType.Array` arg... * [`249944e`](openrewrite/rewrite-logging-frameworks@249944e) Add `ArgumentArrayToVarargs` for logger methods that take a var args argument... * [`bb34ecf`](openrewrite/rewrite-logging-frameworks@bb34ecf) Create Claude settings file * [`ce8bc2d`](openrewrite/rewrite-logging-frameworks@ce8bc2d) refactor: Static imports for Collections and Collectors * [`af44c67`](openrewrite/rewrite-logging-frameworks@af44c67) Recipe to migrate deprecated JBoss Logging calls ([#245](https://redirect.github.com/openrewrite/rewrite-logging-frameworks/issues/245)) * Additional commits viewable in [compare view](openrewrite/rewrite-logging-frameworks@v3.7.0...v3.13.0) [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=org.openrewrite.recipe:rewrite-logging-frameworks&package-manager=maven&previous-version=3.7.0&new-version=3.13.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)

moderne-meeseeks bot added this to OpenRewrite Jun 17, 2025

github-project-automation bot moved this to In Progress in OpenRewrite Jun 17, 2025

pdelagrave commented Jun 17, 2025

View reviewed changes

src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java Outdated Show resolved Hide resolved

pdelagrave force-pushed the issue-223 branch 2 times, most recently from 4468316 to 6ab3a4a Compare June 17, 2025 20:53

New StripToStringFromArguments SLF4j recipe

84b51c2

pdelagrave force-pushed the issue-223 branch from 6ab3a4a to 84b51c2 Compare June 17, 2025 20:57

pdelagrave commented Jun 17, 2025

View reviewed changes

src/main/java/org/openrewrite/java/logging/slf4j/StripToStringFromArguments.java Outdated Show resolved Hide resolved

pdelagrave commented Jun 17, 2025

View reviewed changes

src/main/java/org/openrewrite/java/logging/slf4j/StripToStringFromArguments.java Show resolved Hide resolved

pdelagrave commented Jun 17, 2025

View reviewed changes

src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java Show resolved Hide resolved

pdelagrave changed the title ~~Tests for the new recipe~~ Recipe to remove .toString() called on parameterized logging statement arguments Jun 17, 2025

pdelagrave marked this pull request as ready for review June 17, 2025 21:02

timtebeek assigned pdelagrave Jun 17, 2025

timtebeek self-requested a review June 17, 2025 21:24

timtebeek added the recipe Recipe Requested label Jun 17, 2025

timtebeek requested changes Jun 17, 2025

View reviewed changes

Merge branch 'main' into issue-223

f2f7ec0

github-actions bot reviewed Jun 18, 2025

View reviewed changes

pdelagrave added 10 commits June 18, 2025 10:31

Remove useless null check

f887c8a

Replace bad string comparison, using TypeUtils now

a55edd8

Don't use final for local variables

b32934a

using a matcher to match toString invocations

23b295f

curly braces

40fa083

Licenses

adb6356

new test, needs a fix

2ed724e

refactor tests, add test with no additional message format string par…

e79fc64

…ameters

Using ListUtils.map instead of manually creating a list

c4682a4

Returns super.visitMethodInvocation() if the the method doesn't match

65a1049

pdelagrave added 2 commits June 18, 2025 14:19

rename mi to m

6d10848

Do strip toString() on Throwable.toString() when that argument type b…

0201efa

…eing changed isn't going to change which log() method overload is called.

pdelagrave merged commit 85518ce into openrewrite:main Jun 20, 2025
2 checks passed

github-project-automation bot moved this from In Progress to Done in OpenRewrite Jun 20, 2025

pdelagrave deleted the issue-223 branch June 20, 2025 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Recipe to remove `.toString()` called on parameterized logging statement arguments #224

Recipe to remove `.toString()` called on parameterized logging statement arguments #224

Uh oh!

pdelagrave commented Jun 17, 2025 •

edited by timtebeek

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timtebeek commented Jun 18, 2025

Uh oh!

timtebeek commented Jun 18, 2025

Uh oh!

pdelagrave commented Jun 18, 2025

Uh oh!

timtebeek commented Jun 18, 2025

Uh oh!

pdelagrave commented Jun 20, 2025

Uh oh!

Uh oh!

Uh oh!

Recipe to remove .toString() called on parameterized logging statement arguments #224

Recipe to remove .toString() called on parameterized logging statement arguments #224

Uh oh!

Conversation

pdelagrave commented Jun 17, 2025 • edited by timtebeek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's changed?

What's your motivation?

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

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timtebeek commented Jun 18, 2025

Uh oh!

timtebeek commented Jun 18, 2025

Uh oh!

pdelagrave commented Jun 18, 2025

Uh oh!

timtebeek commented Jun 18, 2025

Uh oh!

pdelagrave commented Jun 20, 2025

Uh oh!

Uh oh!

Uh oh!

Recipe to remove `.toString()` called on parameterized logging statement arguments #224

Recipe to remove `.toString()` called on parameterized logging statement arguments #224

pdelagrave commented Jun 17, 2025 •

edited by timtebeek

Loading