Skip to content
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

Add update hint - recipes are not found any more when updating from 5.40.6 to 6.0.0 #203

Closed
koppor opened this issue Jun 5, 2023 · 19 comments
Labels
bug Something isn't working question Further information is requested

Comments

@koppor
Copy link
Contributor

koppor commented Jun 5, 2023

When updating from 5.40.6 to 6.0.0, I get following output:

Recipe validation error in org.jabref.config.rewrite.cleanup.recipeList[34] (in file:/C:/git-repositories/jabref/rewrite.yml): recipe 'org.openrewrite.java.cleanup.RemoveExtraSemicolons' does not exist.
Recipe validation error in org.jabref.config.rewrite.cleanup.recipeList[35] (in file:/C:/git-repositories/jabref/rewrite.yml): recipe 'org.openrewrite.java.cleanup.RemoveJavaDocAuthorTag' does not exist.
Recipe validation error in org.jabref.config.rewrite.cleanup.recipeList[36] (in file:/C:/git-repositories/jabref/rewrite.yml): recipe 'org.openrewrite.java.cleanup.RemoveRedundantTypeCast' does not exist.
Recipe validation error in org.jabref.config.rewrite.cleanup.recipeList[37] (in file:/C:/git-repositories/jabref/rewrite.yml): recipe 'org.openrewrite.java.cleanup.RemoveUnneededAssertion' does not exist.

I checked https://docs.openrewrite.org/running-recipes/getting-started, but that still lists 5.40.6.

I checked the diff v5.40.5...v6.0.0, but could not see why this happens.

Full project setup at https://github.com/JabRef/jabref/blob/76c35f0afb6f9c700ee35a5475bbe9ff612fa75c/build.gradle#L487

@koppor koppor added the bug Something isn't working label Jun 5, 2023
@koppor
Copy link
Contributor Author

koppor commented Jun 5, 2023

Update: I get following output:

Unable to configure org.openrewrite.java.logging.SystemPrintToLogging
org.openrewrite.config.RecipeIntrospectionException: Unable to call primary constructor for Recipe class org.openrewrite.java.logging.SystemPrintToLogging
...
Caused by: java.lang.NoSuchMethodError: 'org.openrewrite.Recipe org.openrewrite.java.logging.SystemPrintToLogging.doNext(org.openrewrite.Recipe)'
        at org.openrewrite.java.logging.SystemPrintToLogging.<init>(SystemPrintToLogging.java:76)
        at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:67)

Which I didn't get at 5.40.6.

I disabled all logging checks, the error of not found recipes persists.

@timtebeek
Copy link
Contributor

Hi @koppor ; we're still working on our 8.0 release, and as a part of that have released some components already, but have not yet completed the full release, and as a result of that have not updated the documentation yet either.

Once we have fully released 8.0, there will be updated documentation and resources on how to upgrade, including some automated migration recipes.

In your specific case the associated recipes have been moved to a separate repository:
https://github.com/openrewrite/rewrite-static-analysis/tree/main/src/main/java/org/openrewrite/staticanalysis
This way it's easier for new contributors to get started there, as it's a common source of beginner friendly recipes.
We have already released a first 1.0.0 of rewrite-static-analysis, so you can already adopt that if you want to get ahead.

Just know that things are slightly in flux while we're working through this release; we hope to stabilize & document all of this quickly!

@timtebeek timtebeek added the question Further information is requested label Jun 5, 2023
@koppor
Copy link
Contributor Author

koppor commented Jun 5, 2023

Thank you for the quick answer. Seems, I need to be patient. A simple update of the recipes's names did not help

recipe 'org.openrewrite.staticanalysis.cleanup.RemoveUnneededAssertion' does not exist.

The compileOnly dependency to rewrite-java is also not helping.

compileOnly 'org.openrewrite:rewrite-java:8.0.0'

Ah, I need to use org.openrewrite.recipe:rewrite-static-analysis

compileOnly 'org.openrewrite.recipe:rewrite-static-analysis:1.0.0'

OK, I am giving up and waiting for a full release.

@koppor
Copy link
Contributor Author

koppor commented Jun 5, 2023

One more:

rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.0"))

also does not help

Link to mvnrepository: https://mvnrepository.com/artifact/org.openrewrite.recipe/rewrite-recipe-bom/2.0.0

@timtebeek
Copy link
Contributor

We're glad to have you verify our work early; to reply quickly to the above: the new class is at:
org.openrewrite.staticanalysis.RemoveUnneededAssertion in rewrite-static-analysis, with indeed the new GAV dependency you've discovered. Again: work in progress, but it should mostly work already. Let us know if you find any issues with the above!

@koppor
Copy link
Contributor Author

koppor commented Jun 6, 2023

I had a .cleanup too much in the rule names.

Still not sure how to add the dependency.

I have no success with

-    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:1.19.4"))
+    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.0"))
+    compileOnly 'org.openrewrite.recipe:rewrite-static-analysis:1.0.0'

and no success with

-    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:1.19.4"))
+    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.0"))
+    rewrite(platform("org.openrewrite.recipe:rewrite-static-analysis:1.0.0"))

@timtebeek
Copy link
Contributor

I've had a quick look; right now you're using:
https://github.com/JabRef/jabref/blob/76c35f0afb6f9c700ee35a5475bbe9ff612fa75c/build.gradle#L224-L225

    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:1.19.3"))
    rewrite("org.openrewrite.recipe:rewrite-logging-frameworks")

I'd expect things to work when using:

    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.0"))
    rewrite("org.openrewrite.recipe:rewrite-logging-frameworks")
    rewrite("org.openrewrite.recipe:rewrite-static-analysis")

Given that rewrite-recipe-bom-2.0.0.pom manages the version of rewrite-static-analysis-1.0.0.pom

Does that work on your end?

@koppor
Copy link
Contributor Author

koppor commented Jun 6, 2023

Thank you for the patience and quick look. I indeed did it locally only - and did not push anything.

Included your changes and comments (JabRef/jabref@7f6f080) and removed recipies no longer being available (JabRef/jabref@618996c)

Is there a chance that SimplifyBooleanReturn (applied at JabRef/jabref#9885) and SimplifyBooleanExpression are coming back?

I think, I won't miss MethodParamPad, PadEmptyForLoopComponents, UnnecessaryParentheses that much. UnnecessaryParentheses caused internal discussions (JabRef/jabref#9876 (comment)) ^^.

@timtebeek
Copy link
Contributor

Good to see those updates already; the SimplifyBooleanExpression never moved; that's still available by default in the original package.

@koppor
Copy link
Contributor Author

koppor commented Jun 6, 2023

Good to see those updates already; the SimplifyBooleanExpression never moved; that's still available by default in the original package.

One more additional dependency for me then? ^^ - Shouldn't it move, too?

@timtebeek
Copy link
Contributor

One more additional dependency for me then? ^^ - Shouldn't it move, too?

You don't need a dependency for the recipes (still) in the original cleanup package. We've kept the recipes that we call directly from other recipes in openrewrite/rewrite. The others we have since moved to rewrite-static-analysis, as you've found.

We're still in the midst of this release and documenting it all, so I appreciate you trying it out and finding where we should provide guidance in the migration.

@koppor
Copy link
Contributor Author

koppor commented Jun 7, 2023

We're still in the midst of this release and documenting it all, so I appreciate you trying it out and finding where we should provide guidance in the migration.

I summarize what we discussed, maybe it helps:

Migration guide

  1. Replace id("org.openrewrite.rewrite") version("5.40.6") by id("org.openrewrite.rewrite") version("6.0.0")

  2. Replace rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:1.19.4")) by following three lines:

    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.0"))
    rewrite("org.openrewrite.recipe:rewrite-logging-frameworks")
    rewrite("org.openrewrite.recipe:rewrite-static-analysis")
  3. Remove following rules

    • org.openrewrite.java.cleanup.MethodParamPad
    • org.openrewrite.java.cleanup.PadEmptyForLoopComponents
  4. Replace org.openrewrite.java.cleanup by org.openrewrite.staticanalysis in the recipe references (most rules have been migrated to staticanalysis namespace)

  5. Replace back the names of following recipes (they still reside in the java.cleanup namespace)

    • SimplifyBooleanExpression: org.openrewrite.java.cleanup.SimplifyBooleanExpression
    • SimplifyBooleanReturn: org.openrewrite.java.cleanup.SimplifyBooleanReturn
    • UnnecessaryParentheses: org.openrewrite.java.cleanup.UnnecessaryParentheses

@timtebeek
Copy link
Contributor

timtebeek commented Jun 7, 2023

@mike-solomon see the above comment for when it comes time to document and announce the new release. we might even be able to automate some of those moved recipe changes in code and yaml files. Also captured in openrewrite/rewrite#3234

@koppor
Copy link
Contributor Author

koppor commented Jun 26, 2023

I think, this thing is really in flux, therefore, I just comment here instead of rasing a new issue.

Bumping org.openrewrite.rewrite from 6.1.4 to 6.1.6 (JabRef/jabref#10037) leads to following error:

> java.lang.RuntimeException: Error while visiting src/jmh/java/org/jabref/benchmarks/Benchmarks.java: java.lang.NoSuchMethodError: 'boolean org.openrewrite.java.MethodMatcher.matches(org.openrewrite.java.tree.J$MethodInvocation)'

I was hoping that an increase of a patch version does not lead to errors during usage. The solution is unfortunately not to bump org.openrewrite.recipe:rewrite-recipe-bom from 2.0.1 to 2.0.2 at the same time (tried at JabRef/jabref#10040). Of only that is bumped, it fails too.

@timtebeek
Copy link
Contributor

There has unfortunately been a bit of a botched change recently, which was subsequently reverted, that changed the signature of the MethodMatcher. It should all work again with the latest versions, so it's curious to hear you're having these issues. In general we recommend using all the latest version, but if there's a combination that's not working yet we should maybe bring out an additional patch release to get things going.

@timtebeek
Copy link
Contributor

We're squashing every last instance of that NoSuchMethodError with patch releases as needed; We'll follow up with a rewrite-recipe-bom release once confirmed fixed, such that it's not necessary to pin individual modules anymore.

We've also documented the changes needed to pick up OpenRewrite 8.x over on
https://docs.openrewrite.org/changelog/8-1-2-release
That should hopefully resolve this issue, as we've incorporated the much of the suggestions above into either automated or documented migration steps.

Would you agree with closing the issues as done @koppor ? Or is there anything you'd want to still tackle as part of this issue, or in a subsequent issue?

@koppor
Copy link
Contributor Author

koppor commented Jun 28, 2023

For me, the upgrade worked with your help. Thank you again. - The linked update doc looks good.

In case, I miss something at subsequent updates, I'll raise my voice.

@koppor
Copy link
Contributor Author

koppor commented Jun 28, 2023

Think, I need to drop recipies at the update.

-    id 'org.openrewrite.rewrite' version '6.1.4'
+    id 'org.openrewrite.rewrite' version '6.1.7'

-    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.1"))
+    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.0.2"))

Would be nice if the default error output would include the failed recipe, because manually disabling recipeies one by one is difficult. And there is no good "gid bisect" equivalent for the rewrite.yml file ^^.

* What went wrong:
Execution failed for task ':rewriteDryRun'.
> java.lang.RuntimeException: Error while visiting src\jmh\java\org\jabref\benchmarks\Benchmarks.java: java.lang.NoSuchMethodError: 'boolean org.openrewrite.java.MethodMatcher.matches(org.openrewrite.java.tree.J$MethodInvocation)'
    org.openrewrite.staticanalysis.EqualsAvoidsNullVisitor.visitMethodInvocation(EqualsAvoidsNullVisitor.java:48)
    org.openrewrite.staticanalysis.EqualsAvoidsNullVisitor.visitMethodInvocation(EqualsAvoidsNullVisitor.java:32)
    org.openrewrite.java.tree.J$MethodInvocation.acceptJava(J.java:3706)
    org.openrewrite.java.tree.J.accept(J.java:59)
    org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
    org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:361)
    org.openrewrite.java.JavaVisitor.visitLeftPadded(JavaVisitor.java:1312)
    org.openrewrite.java.JavaVisitor.visitAssignment(JavaVisitor.java:310)
    ...

@timtebeek
Copy link
Contributor

The rewrite-recipe-bom still needs a patch release; will go out as soon as I hear the issues with rewrite-static-analysis and rewrite-migrate-java have been resolved with their latest patch releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
Archived in project
Development

No branches or pull requests

2 participants