openrewrite / rewrite-gradle-plugin

OpenRewrite's Gradle plugin.
Apache License 2.0
56 stars 34 forks source link

Print patch produced by rewriteDryRun to build console so users can understand violations when running in CI #255

Open vlsi opened 6 months ago

vlsi commented 6 months ago

What problem are you trying to solve?

I want keep the project style consistent, so I want adding rewriteDryRun in CI, and I want PRs to fail in case the style is not in line. Currently, OpenRewrite does not clarify what is the issue, so it is not very user-friendly

These recipes would make changes to pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java:
    io.github.pgjdbc.staticanalysis.CodeCleanup
        org.openrewrite.java.OrderImports
Report available:
    /.../pgjdbc/pgjdbc/build/reports/rewrite/rewrite.patch

Describe the solution you'd like

I would like OpenRewrite to show the diff (e.g. the first 200 lines of it) so the user can understand what are the deviations.

An additional option would be generating GitHub Action annotations via ::error file=app.js,line=1::Missing semicolon , see https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#example-creating-an-annotation-for-an-error and https://github.com/actions/toolkit/tree/5430c5d84832076372990c7c27f900878ff66dc9/packages/core#annotations

An extra option could be using Action Summary, however, I am not sure OpenRewrite has something relevant to show on the summary: https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/

vlsi commented 6 months ago

Workaround:

tasks.withType<RewriteDryRunTask>().configureEach {
    doFirst {
        if (reportPath.exists()) {
            // RewriteDryRunTask keeps the report file if there are no violations, so we remove it
            reportPath.delete()
        }
    }
    doLast {
        if (reportPath.exists()) {
            throw GradleException(
                "The following files have format violations. " +
                        "Execute ./gradlew ${path.replace("Dry", "")} to apply the changes:\n" +
                        reportPath.readText()
            )
        }
    }
}
timtebeek commented 6 months ago

Thanks for the suggestion! Not sure if this would make it into the Gradle and Maven plugins, but I do like the hints that you've given as I'd been eyeing something similar myself for a while now.

From your workaround it seems we should at least remove the reportPath when we start a new dry run task; Feel free to add that to the plugin here if that helps you simplify the integration there!

timtebeek commented 6 months ago

So to give you a little more context on what I'm eyeing for OpenRewrite: here's a short reproducer I'd worked on previously:

As you can see there I use https://github.com/googleapis/code-suggester to not just post a check result, but the actual fix that folks can apply with a mouse click. I think that fits OpenRewrite better as we actually make code changes. I think that might work well for you too, although I've not yet found the time to work out the final details. Welcome to have a go and let me know what you come up with!

vlsi commented 6 months ago

Thank you for mentioning code-suggester. It might be worth looking into. However, I have already merged "print diff"-based openrewrite to pgjdbc, so it still looks like a reasonable way to go for the first openrewrite integration

knutwannheden commented 6 months ago

I think it could we interesting to add a new dedicated task/ goal to our build plugins for this purpose. Some may not want to use code-suggester and it does seems to be a generally useful thing.

vlsi commented 6 months ago

I've created https://github.com/openrewrite/rewrite/issues/3891 to track googleapis/code-suggester integration

timtebeek commented 2 months ago

I think it could we interesting to add a new dedicated task/ goal to our build plugins for this purpose. Some may not want to use code-suggester and it does seems to be a generally useful thing.

If we're considering additional tasks/goals and output formats, then it might make sense to look at the Static Analysis Results Interchange Format (SARIF):

That already aims to provide a common format that other tools (like GitHub and review-dog) can use, and would also allow us to report additional information such as which recipes made changes, and links to those recipes.