openrewrite / rewrite-logging-frameworks

OpenRewrite recipes for assisting with Java logging migration tasks.
Apache License 2.0
25 stars 20 forks source link

File removed after running `SystemPrintToLogging` with logging framework `JUL` #125

Closed gdupontf closed 10 months ago

gdupontf commented 10 months ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

See https://github.com/gdupontf/sandbox/blob/4dd3d64d118e7a8fc2a40071e6dcf482bb53e04f/pom.xml#L151-L182

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

This basic recipe with any code that has a System.out.println("Something")

---
type: specs.openrewrite.org/v1beta/recipe
name: com.gdupontf.Opinionated
recipeList:
  - org.openrewrite.java.logging.SystemPrintToLogging:
      addLogger: true
      loggerName: log
      loggingFramework: JUL

You can try it on this repo at linked state Basically by reversing the commented recipes and running ./mvnw rewrite:run

What did you expect to see?

No files should be deleted. The println(...) call should be replaced by the standard JUL library.

What did you see instead?

Executing run or dryRun leads to deletion (or a diff describing one).

What is the full stack trace of any errors you encountered?

None.

Are you interested in contributing a fix to OpenRewrite?

I don't have a clue to the internals that lead to this issue, so no on this one.

timtebeek commented 10 months ago

Thanks for reporting your issue here! I've run your demo from the commit you linked, and I'm seeing the following in the output:

[INFO] Running recipe(s)...
[WARNING] Deleted file src/main/java/org/gdupontf/App.java by:
[WARNING]     com.gdupontf.Opinionated
[WARNING]         org.openrewrite.staticanalysis.ReplaceValidateNotNullHavingSingleArgWithObjectsRequireNonNull
[WARNING]             org.openrewrite.java.ChangeMethodTargetToStatic: {methodPattern=org.apache.commons.lang3.Validate requireNonNull(Object), fullyQualifiedTargetTypeName=java.util.Objects}
[WARNING] Please review and commit the results.

We print that information to aid debugging; in this case it seems ReplaceValidateNotNullHavingSingleArgWithObjectsRequireNonNull causes the issue, so I'll move the issue to the corresponding repository.

timtebeek commented 10 months ago

Debugging this lead me down some weird paths, but a proposed fix is now in #126. Thanks for reporting this issue! I hope that indeed resolves the problem you're having, and neat to see the workflow you're building.

timtebeek commented 10 months ago

The above fix has been merged; in a couple minutes it should become available through our snapshot repository. Could you let me know if that does indeed work for your use case? We typically follow up with a release in the next two weeks.

gdupontf commented 10 months ago

With the latest snapshot (2.3.0-20231128.181859-7 to be ultra-precise) all's good!

[INFO] Project [sandbox] Resolving Poms...
[INFO] Project [sandbox] Parsing source files
[INFO] Running recipe(s)...
[INFO] Applying recipes would make no changes. No patch file generated.
timtebeek commented 10 months ago

Glad to hear! Thanks for confirming here. :)