Apply configured styles after auto-detected styles#1116
Apply configured styles after auto-detected styles#1116
Conversation
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.
|
Would also need to see adjustments downstream in the CLI. |
github-project-automation
bot
moved this from In Progress
to Ready to Review
in OpenRewrite
| .styles(styles) | ||
| .logCompilationWarningsAndErrors(false); | ||
|
|
||
| // todo, add styles from autoDetect |
There was a problem hiding this comment.
| // todo, add styles from autoDetect |
| 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, |
There was a problem hiding this comment.
This method is used by the CLI, and removing the styles here would cause problems there.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
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. |
2 participants
checkstyle rules)