Documentation regarding rewrite.yml is misleading: documentation says the default location is rootDir while the actual default is projectDir

[vlsi]

What version of OpenRewrite are you using?

org.openrewrite.rewrite.gradle.plugin:6.4.3

How are you running OpenRewrite?

./gradlew rewriteDiscover

The are four issues: 1) Documentation says rootDir is the default location: https://docs.openrewrite.org/reference/gradle-plugin-configuration#configuring-the-rewrite-dsl

    // These are default values, shown for example. It isn't necessary to supply these values manually:
    configFile = project.getRootProject().file("rewrite.yml")

In practice, the default location is projectDir/rewrite.yml: https://github.com/openrewrite/rewrite-gradle-plugin/blob/0daa455128fe26ab97e1754adc23e1b55b931445/plugin/src/main/java/org/openrewrite/gradle/DefaultRewriteExtension.java#L74

2) I would expect OpenRewrite to fail when the user provides a non-existent style. It took me a while to realize that my rewrite.yml was not loaded at all

3) A better alternative to File configFile would be https://docs.gradle.org/current/dsl/org.gradle.api.resources.TextResourceFactory.html (since Gradle 2.2) as it would reduce IO during the configuration time. In other words, if user does not execute OpenRewrite tasks, users won't need executing IO for rewrite.yml. See https://github.com/gradle/gradle/blob/bb4604586434a1c781ff3996ffaff66d079cb43c/platforms/jvm/code-quality/src/main/groovy/org/gradle/api/plugins/quality/CheckstyleExtension.java#L39

4) The documentation should probably prefer Kotlin DSL syntax rather than Java DSL as people rarely use Gradle Plugins from Java. In other words rootProject should be preferred over getRootProject()

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

Add the following rewrite.yml to the root dir

---
# See reference at https://docs.openrewrite.org/concepts-explanations/styles#declarative-styles
type: specs.openrewrite.org/v1beta/style
name: io.github.pgjdbc.style.Style
styleConfigs:
  - org.openrewrite.java.style.TabsAndIndentsStyle:
      # See https://github.com/openrewrite/rewrite/issues/3666
      # For now, it should be the same as in .editorconfig
      useTabCharacter: false
      tabSize: 2
      indentSize: 2
      continuationIndent: 4
  - org.openrewrite.java.style.SpacesStyle:
      # See https://github.com/openrewrite/rewrite/blob/1d0aeec11e2d7f239f2f656bfd964b17ca8e4d17/rewrite-java/src/main/java/org/openrewrite/java/style/SpacesStyle.java#L26C14-L26C25
      other:
        afterForSemicolon: true

Configure Gradle plugin:

plugins {
    id("org.openrewrite.rewrite")
}

dependencies {
    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.4.1"))
    rewrite("org.openrewrite.recipe:rewrite-static-analysis")
}

rewrite {
    activeStyle("io.github.pgjdbc.style.Style")
    activeRecipe("org.openrewrite.java.OrderImports")
    activeRecipe("org.openrewrite.staticanalysis.MissingOverrideAnnotation")
}

What did you see instead?

Available Styles:
    org.openrewrite.java.IntelliJ
    org.openrewrite.kotlin.IntelliJ
    com.netflix.genie.Style
    org.openrewrite.java.SpringFormat
    com.netflix.eureka.Style
    org.openrewrite.java.GoogleJavaFormat

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

Available Styles: does not list io.github.pgjdbc.style.Style

Are you interested in contributing a fix to OpenRewrite?

May be

[timtebeek]

Thanks for the detailed report! Just to be sure: are you running from the root of the repository? Or is there a different folder involved?

Appreciate your suggestions as well, for better API use such as 3, proper validation and warning as mentioned in 2, and the small tweaks to the documentation to better match reality, or have the plugin match the docs (and likely the Maven plugin and Moderne CLI).

If there's anything you would like to pick up out of those items that'd be much appreciated; just let us know which ones you think are a good fit to contribute, and which other ones you'd like us to take a look at. Thanks again!

[vlsi]

Just to be sure: are you running from the root of the repository?

In Gradle model, it should not matter. project.file("rewrite.yml"); does not use "current working directory" but it resolves the file relative to the project directory.

In multi-project build, every project has its directory, so by default openrewrite-gradle-plugin would search for many individual rewrite.yml files.

[vlsi]

for better API use such as 3, proper validation and warning as mentioned in 2

Could you please clarify what you mean by "use API 3"?

[timtebeek]

for better API use such as 3, proper validation and warning as mentioned in 2

Could you please clarify what you mean by "use API 3"?

There I meant your suggestion outlined above as the third bullet point to switch from File configfile to TextResource, if I understood you correctly.

A better alternative to File configFile would be https://docs.gradle.org/current/dsl/org.gradle.api.resources.TextResourceFactory.html (since Gradle 2.2) as it would reduce IO during the configuration time. In other words, if user does not execute OpenRewrite tasks, users won't need executing IO for rewrite.yml. See https://github.com/gradle/gradle/blob/bb4604586434a1c781ff3996ffaff66d079cb43c/platforms/jvm/code-quality/src/main/groovy/org/gradle/api/plugins/quality/CheckstyleExtension.java#L39

[shanman190]

Documentation says rootDir is the default location: https://docs.openrewrite.org/reference/gradle-plugin-configuration#configuring-the-rewrite-dsl

For this, it's both. The OSS plugin is really only designed right now to be fully functional when applied at the root project. If you apply it at a subproject level, it may work but there may end up being missing information which would impact recipe execution.

I would expect OpenRewrite to fail when the user provides a non-existent style. It took me a while to realize that my rewrite.yml was not loaded at all

So the problem here is a bit more tricky. By default OpenRewrite begins with a Intellij style. The user can opt to provide one or more parts of an overall style, but OpenRewrite ultimately requires a complete style. As an example, when OpenRewrite would load your style as defined it would only contain 2 of the 4 total configurations that represent a complete style definition. That's fine as you can tell because each user style can merge their overlays onto the defaults resulting in a final blended, but merged overall style.

I do agree with you though that there should be some information telling you that the style failed to load and listing it within the discovered styles. Whether that failed to load is a warning message or a terminal error, I do not know.

A better alternative to File configFile would be https://docs.gradle.org/current/dsl/org.gradle.api.resources.TextResourceFactory.html (since Gradle 2.2) as it would reduce IO during the configuration time. In other words, if user does not execute OpenRewrite tasks, users won't need executing IO for rewrite.yml. See https://github.com/gradle/gradle/blob/bb4604586434a1c781ff3996ffaff66d079cb43c/platforms/jvm/code-quality/src/main/groovy/org/gradle/api/plugins/quality/CheckstyleExtension.java#L39

So this could be, but something of note is that only the file handle is created during configuration time. The file isn't read until execution time, so the amount of IO should remain the same since OpenRewrite also doesn't have access to the TaskContainer#register methods due to attempting to keep a low point of entry for all users (OSS and Enterprise) of 4.0 (but has been agreed upon to begin moving to 4.10). The amount of IO then from the TextResource should then be about the same with respect to the File implementation when viewed through the lens of IO during configuration time though.

The documentation should probably prefer Kotlin DSL syntax rather than Java DSL as people rarely use Gradle Plugins from Java. In other words rootProject should be preferred over getRootProject()

This one can be interesting, but in general if the context is used within defining a build.gradle(.kts) file then it would probably be cleaner to follow the common idioms that Gradle exposes there. There are users whom are adding OpenRewrite to internal binary plugins though. 🙂

[vlsi]

https://docs.openrewrite.org/reference/gradle-plugin-configuration#multi-module-gradle-projects says the plugin can be applied to individual projects.

When applied to a multi-project build, plugin behavior differs depending on whether the plugin is applied to the root project or to a sub-project. Applied to the root project, the plugin will parse and refactor all sources from all projects. Applied to any project other than the root project, the plugin will parse and refactor only sources from that project.

Applied to the root project, the plugin will parse and refactor all sources from all projects

Frankly speaking, I find this behaviour surprising, and it violates Gradle best practices. Ideally, there should be two plugins: the first one would be "per project plugin" that configures a specific project only, and the second plugin could be a helper one that applies the first plugin to all the projects.

[vlsi]

OpenRewrite also doesn't have access to the TaskContainer#register methods due to attempting to keep a low point of entry for all users (OSS and Enterprise) of 4.0

Have you considered adding a runtime check, and using tasks.register vs tasks.create depending on the runtime Gradle version?

[vlsi]

By default OpenRewrite begins with a Intellij style

...

Whether that failed to load is a warning message or a terminal error, I do not know.

Well, user asked activeStyle("com.example..."). I would expect to have a plain build error that says "the style was not found in ..." A warning will be easy to miss.

OpenRewrite must not ignore missing styles in https://github.com/openrewrite/rewrite/blob/a94650d765fea173629655e4c1a300805054a7f4/rewrite-core/src/main/java/org/openrewrite/config/Environment.java#L182, and ideally it should have listed all the ResourceLoaders it attampted.