Skip to content

Apply configured styles after auto-detected styles; tighten parser visibility - #1116

Merged
timtebeek merged 5 commits into
mainfrom
apply-configured-styles-last
Jun 16, 2026
Merged

Apply configured styles after auto-detected styles; tighten parser visibility#1116
timtebeek merged 5 commits into
mainfrom
apply-configured-styles-last

Conversation

@timtebeek

@timtebeek timtebeek commented Feb 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Apply configured styles after auto-detected styles

  • With Parse more checkstyle rules into OpenRewrite styles. rewrite#6761 changing style merging to "last wins" semantics, configured styles must be applied after auto-detected styles so they take precedence.
  • Previously configured styles were set on the Java parser builder before parsing, while auto-detected styles were added to markers after parsing — meaning auto-detected styles were last and would unintentionally override user configuration (e.g. checkstyle rules).
  • Now both auto-detected and configured styles are applied together in AbstractRewriteBaseRunMojo, in the correct order: auto-detected first, configured last.
  • Removed the now-unnecessary styles parameter from MavenMojoProjectParser#listSourceFiles.

Tighten MavenMojoProjectParser visibility

The Moderne CLI no longer reuses rewrite-maven-plugin, so the parsing methods that were public solely for that reuse can be tightened. Only listSourceFiles(MavenProject, ctx) still needs package visibility (for the Mojo); the rest are now private:

  • listSourceFiles(...) overloads → private (kept the (MavenProject, ctx) entry point package-private)
  • generateProvenance(...) → private
  • parseMaven(...) (both overloads) → private
  • buildSettings() → private
  • POM_CACHE → private static

@timtebeek
Apply configured styles after auto-detected styles
8ecf687 
With openrewrite/rewrite#6761 changing style merging to "last wins"
semantics, configured styles must be applied after auto-detected styles
so they take precedence. Previously configured styles were set on the
parser (applied first) and auto-detected styles were added after parsing
(applied last), meaning auto-detected styles would unintentionally
override user configuration.

Remove the now-unnecessary styles parameter from
MavenMojoProjectParser#listSourceFiles and instead apply both
auto-detected and configured styles in the correct order within
AbstractRewriteBaseRunMojo.
@timtebeek timtebeek self-assigned this Feb 26, 2026
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Feb 26, 2026
@timtebeek

timtebeek commented Feb 26, 2026

Copy link
Copy Markdown
Member Author

Would also need to see adjustments downstream in the CLI.

@timtebeek
timtebeek marked this pull request as ready for review February 26, 2026 22:57
@timtebeek
timtebeek requested a review from sambsnyd February 26, 2026 22:57
@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Feb 26, 2026
timtebeek
timtebeek commented Feb 26, 2026
View reviewed changes
Comment thread src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java Outdated
.styles(styles)
.logCompilationWarningsAndErrors(false);

// todo, add styles from autoDetect

@timtebeek timtebeek Feb 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// todo, add styles from autoDetect

timtebeek
timtebeek commented Feb 27, 2026
View reviewed changes
Comment thread src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java Outdated
return listSourceFiles(mavenProject, maven, projectProvenance, Arrays.asList(MAIN, TEST), ctx);
}

public Stream<SourceFile> listSourceFiles(MavenProject mavenProject, Xml.@Nullable Document maven, List<Marker> projectProvenance, List<MavenScope> scopes,

@timtebeek timtebeek Feb 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is used by the CLI, and removing the styles here would cause problems there.

@timtebeek timtebeek Feb 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some weird interplay here between the Maven plugin detecting styles after and separate from listSourceFiles, whereas the CLI only calls listSourceFiles and itself in a limited capacity loads styles.

Would we need to detect styles as part of listSourceFiles perhaps, and remove the duplicate logic in the CLI? Any ideas here Sam since this follows on from your earlier work?

@sambsnyd sambsnyd Feb 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we should make things consistent between the CLI, gradle plugins, and maven plugins. I believe there is some inconsistency with how styles are treated / autodetected in the CLI and between the two plugins. There is likely some redundancy.

@timtebeek
timtebeek marked this pull request as draft February 27, 2026 14:00
@timtebeek

timtebeek commented Mar 11, 2026

Copy link
Copy Markdown
Member Author

Heard we might sever the reuse of rewrite-maven-plugin in the Moderne CLI, so might just wait for that to land before making changes here, as it will be easier if there's no backwards compatibility across projects.

@timtebeek timtebeek mentioned this pull request Mar 24, 2026
Closed
@timtebeek timtebeek linked an issue Mar 24, 2026 that may be closed by this pull request
Style defined in <configLocation> is ignored by OrderImports / RemoveUnusedImports #1128
Closed
@timtebeek timtebeek moved this from Ready to Review to In Progress in OpenRewrite Apr 8, 2026
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in two weeks. PRs may be reopened when there is renewed interest.

@github-actions github-actions Bot added the Stale label Jun 15, 2026
@timtebeek

timtebeek commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved and pushed in commit f033214 by merging origin/main and fixing the conflict in MavenMojoProjectParser.

@timtebeek timtebeek removed the Stale label Jun 16, 2026
@timtebeek
Reduce visibility of MavenMojoProjectParser methods exposed for the CLI
cbc8fd6 
The Moderne CLI no longer reuses rewrite-maven-plugin, so the parsing
methods that were public solely for that reuse can be tightened. Only
listSourceFiles(MavenProject, ctx) needs package visibility for the
Mojo; the remaining overloads, generateProvenance, parseMaven, and
buildSettings are now private, and POM_CACHE is private static.
@timtebeek timtebeek changed the title Apply configured styles after auto-detected styles Apply configured styles after auto-detected styles; tighten parser visibility Jun 16, 2026
@timtebeek
timtebeek marked this pull request as ready for review June 16, 2026 17:48
@timtebeek
timtebeek merged commit dbe81d4 into main Jun 16, 2026
1 check passed
@timtebeek
timtebeek deleted the apply-configured-styles-last branch June 16, 2026 17:49
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Jun 16, 2026
mergify Bot added a commit to robfrank/linklift that referenced this pull request Jun 18, 2026
@mergify
chore(deps-dev): bump org.openrewrite.maven:rewrite-maven-plugin fromâ€Ķ
c45cb2c 
â€Ķ 6.41.0 to 6.42.0 [skip ci]

Bumps [org.openrewrite.maven:rewrite-maven-plugin](https://github.com/openrewrite/rewrite-maven-plugin) from 6.41.0 to 6.42.0.
Release notes

*Sourced from [org.openrewrite.maven:rewrite-maven-plugin's releases](https://github.com/openrewrite/rewrite-maven-plugin/releases).*

> 6.42.0
> ------
>
> What's Changed
> --------------
>
> * chore(ci): bump rtCamp/action-slack-notify from 2.3.3 to 2.4.0 by [`@​dependabot`](https://github.com/dependabot)[bot] in [openrewrite/rewrite-maven-plugin#1162](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1162)
> * chore(ci): bump org.openrewrite:rewrite-bom from 8.84.0 to 8.84.4 by [`@​dependabot`](https://github.com/dependabot)[bot] in [openrewrite/rewrite-maven-plugin#1163](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1163)
> * Change the MavenPomCache init logic by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-maven-plugin#1164](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1164)
> * Remove .context files by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-maven-plugin#1165](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1165)
> * Switch to packages.nuxeo.com in BasicIT by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-maven-plugin#1167](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1167)
> * chore(ci): bump io.micrometer:micrometer-core from 1.16.5 to 1.17.0 by [`@​dependabot`](https://github.com/dependabot)[bot] in [openrewrite/rewrite-maven-plugin#1166](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1166)
> * Apply configured styles after auto-detected styles; tighten parser visibility by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-maven-plugin#1116](https://redirect.github.com/openrewrite/rewrite-maven-plugin/pull/1116)
>
> **Full Changelog**: <openrewrite/rewrite-maven-plugin@v6.41.0...v6.42.0>


Commits

* [`64b0e5f`](openrewrite/rewrite-maven-plugin@64b0e5f) [maven-release-plugin] prepare release v6.42.0
* [`d360daa`](openrewrite/rewrite-maven-plugin@d360daa) Bump rewrite.version property
* [`dbe81d4`](openrewrite/rewrite-maven-plugin@dbe81d4) Apply configured styles after auto-detected styles; tighten parser visibility...
* [`c4cc317`](openrewrite/rewrite-maven-plugin@c4cc317) chore(ci): bump io.micrometer:micrometer-core from 1.16.5 to 1.17.0 ([#1166](https://redirect.github.com/openrewrite/rewrite-maven-plugin/issues/1166))
* [`af2ae2f`](openrewrite/rewrite-maven-plugin@af2ae2f) Switch to packages.nuxeo.com ([#1167](https://redirect.github.com/openrewrite/rewrite-maven-plugin/issues/1167))
* [`1f337f0`](openrewrite/rewrite-maven-plugin@1f337f0) Bump rewrite.version property
* [`c6d8097`](openrewrite/rewrite-maven-plugin@c6d8097) git-ignore .context/
* [`cfd5b08`](openrewrite/rewrite-maven-plugin@cfd5b08) Remove .context files ([#1165](https://redirect.github.com/openrewrite/rewrite-maven-plugin/issues/1165))
* [`962cc40`](openrewrite/rewrite-maven-plugin@962cc40) Change the MavenPomCache init logic ([#1164](https://redirect.github.com/openrewrite/rewrite-maven-plugin/issues/1164))
* [`45c5bd6`](openrewrite/rewrite-maven-plugin@45c5bd6) chore(ci): bump org.openrewrite:rewrite-bom from 8.84.0 to 8.84.4 ([#1163](https://redirect.github.com/openrewrite/rewrite-maven-plugin/issues/1163))
* Additional commits viewable in [compare view](openrewrite/rewrite-maven-plugin@v6.41.0...v6.42.0)
@timtebeek timtebeek mentioned this pull request Jul 11, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sambsnyd sambsnyd sambsnyd approved these changes

Assignees

@timtebeek timtebeek

Labels

None yet

Projects

OpenRewrite
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Style defined in <configLocation> is ignored by OrderImports / RemoveUnusedImports

3 participants

@timtebeek @sambsnyd