Issue #122: Support multiple instances of the same Checkstyle check by Anmol202005 · Pull Request #123 · checkstyle/checkstyle-openrewrite-recipes

Anmol202005 · 2025-10-28T23:09:26Z

Fixes: #122

Anmol202005 · 2025-10-29T16:11:37Z

@romani @rdiachenko This PR currently covers most of the cases (all cases for sarif report) but there are some failing edge cases for xml report.

Issue: XML reports only mention the check ID, which creates ambiguous matching when the ID value matches another check's class name, such as:

<module name="Checker">
  <module name="TreeWalker">
    <module name="com.puppycrawl.tools.checkstyle.checks.UpperEllCheck"/>
    <module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck">
      <property name="id" value="com.puppycrawl.tools.checkstyle.checks.UpperEllCheck"/>
    </module>
  </module>
</module>

No such issue with sarif since it mentions both the check name as well as the id.
for the above case the sarif report provides the source as:
com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck#com.puppycrawl.tools.checkstyle.checks.UpperEllCheck

Can we make a similar thing for xml report in the main checkstyle repo otherwise there will be uncertainty with the logic always.

rdiachenko · 2025-10-29T22:06:31Z

@Anmol202005 please create an issue in the main checkstyle repo and include examples and expected output.

Regarding:

    <module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck">
      <property name="id" value="com.puppycrawl.tools.checkstyle.checks.UpperEllCheck"/>
    </module>

I think this is more theoretical than practical case. Users are in control of their configs, and I can't imagine the case when they want to set id pointing to a different module. This can be done by some odd refactoring or mistake.

If the above case happens, what's the result of autofix execution? Will it fail or do nothing? We should just ignore such cases, because you have violation belonging to a different module id, which means if you try to execute recipe, nothing should change, right?

romani · 2025-10-30T01:42:21Z

If people wanted to shoot in their own leg by such config, ok , it is their problem.
There are a lot of places where you can do stupid stuff in our config, we will not support it.

Anmol202005 · 2025-10-30T12:50:18Z

If the above case happens, what's the result of autofix execution? Will it fail or do nothing? We should just ignore such cases, because you have violation belonging to a different module id, which means if you try to execute recipe, nothing should change, right?

Yes nothing will change :)

Anmol202005 · 2025-10-30T19:41:03Z

@romani this PR is ready to review.
but IMO we should update the violation report to mention both check as well as the id as source (If it doesn't break anything else). It will make the logic much straight forward here.

Should i create an issue for same in the main checkstyle repo ?

romani · 2025-10-31T04:39:39Z

Please create , but please use CLI to show a problem, to let us see all details.

romani

we need some cleanup from "source" (xml attribute of checkstyle report) in naming to checkId or checkName or ...

as we have sarif report, so better to use better term that is not xml format special

src/main/java/org/checkstyle/autofix/parser/CheckstyleViolation.java

src/main/java/org/checkstyle/autofix/parser/SarifReportParser.java

src/main/java/org/checkstyle/autofix/CheckstyleCheck.java

Anmol202005 · 2025-11-13T20:32:18Z

Please create , but please use CLI to show a problem, to let us see all details.

checkstyle/checkstyle#18088

@romani issue created. This will make the flow straightforward here.

romani

Few more updates required

romani · 2025-11-16T02:34:56Z

src/main/java/org/checkstyle/autofix/parser/CheckstyleViolation.java


    public CheckstyleViolation(int line, int column, String severity,
-                               CheckstyleCheck source, String message, Path filePath) {
+                               String source, String message, Path filePath) {


Same for other places

romani · 2025-11-16T02:41:38Z

src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java

                .collect(Collectors.toList());
    }

+    private static CheckConfiguration findMatchingConfiguration(String source,


checkId

And for all new methods.

rdiachenko · 2025-11-16T15:42:14Z

src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java

+                .filter(configEntry -> matchesSource(configEntry.getKey(), source))
+                .map(Map.Entry::getValue)
+                .findFirst()
+                .orElse(null);


could you return Optional and filter out absent values before trying to create recipe after calling findMatchingConfiguration

rdiachenko · 2025-11-16T16:12:18Z

src/main/java/org/checkstyle/autofix/CheckstyleConfigModule.java

+
+package org.checkstyle.autofix;
+
+public class CheckstyleConfigModule {


The name is misleading, this is not a config, but just a wrapper around check, which adds id attribute from config properties. This abstraction makes the whole logic more complex, since you eventually use CheckstyleCheck.

Could you try to simplify the logic by combining both classes into one CheckstyleCheck and CheckstyleConfigModule? In reality, id field inside CheckstyleCheck is actually a fully qualified check's name

romani · 2025-11-20T23:15:19Z

@Anmol202005 , do you have time to finish this update?
You can presume that issue in main repo is already resolved.
If any extra update required, we always follow up in new PR.

romani · 2025-11-23T19:31:06Z

#127 (comment) after fixing this , update for suppression is easy activity.

@Anmol202005 , fyi, fix for id support is close to merge state, release of CS will be at last Saturday on november, so you can expect fix to be ready for you very soon.

romani · 2025-11-30T14:02:48Z

@Anmol202005 , fix for IDs in XML is released https://checkstyle.org/releasenotes.html#Release_12.2.0
please bump cs version.

Anmol202005 · 2026-01-18T15:36:16Z

@romani @rdiachenko sorry for delay.

I have simplified the flow :)

romani

Items

romani · 2026-01-18T22:47:01Z

mvnw.cmd

Why it was changed,?

This is a line ending normalization - the file content is identical (same checksum), but Git's metadata needed to be refreshed to match our .gitattributes rules, which caused every line to show as changed even though the actual content is unchanged. I've tried various workarounds (Git config changes, checkout options), but re-committing the file is the only reliable solution to fix the index mismatch.

Question is more on what you did to run into problems with this file.
Why it was not a problem before?

I just try avoid situation when next contributors with flip this file back.

well the reason is git attributes,

i needed to create git attributes because of a new check in the check config that flags window file ending.
checkstyle/checkstyle#18069

ok so there is a misunderstanding, Just to clarify no actual line endings were changed in the .cmd file. It was already using CRLF before, and it still uses CRLF now.

The diff you’re seeing is a side effect of adding .gitattributes to explicitly enforce eol=crlf for .cmd files. Git re-normalized the file against the new rule, which caused it to show every line as “modified” in the diff even though the bytes on disk are identical.

tomato@fedora:~/projects/checkstyle-openrewrite-recipes$ file mvnw.cmd mvnw.cmd: ASCII text, with CRLF line terminators tomato@fedora:~/projects/checkstyle-openrewrite-recipes$ git show HEAD~1:mvnw.cmd | file - /dev/stdin: ASCII text, with CRLF line terminators tomato@fedora:~/projects/checkstyle-openrewrite-recipes$ git show HEAD~2:mvnw.cmd | file - /dev/stdin: ASCII text, with CRLF line terminators

relying on this: https://docs.github.com/en/get-started/git-basics/configuring-git-to-handle-line-endings?spm=a2ty_o01.29997173.0.0.7b2e5171IoLqtQ#refreshing-a-repository-after-changing-line-endings

@romani how to proceed ?

@romani ping

@Anmol202005 , please add new line at the end of .gitattributes

romani

good

items

src/main/java/org/checkstyle/autofix/CheckstyleCheckInstance.java

Anmol202005 · 2026-01-25T09:08:30Z

@romani done, pls review.

Anmol202005 · 2026-01-25T12:29:59Z

@romani I’ve reverted https://github.com/checkstyle/checkstyle-openrewrite-recipes/pull/125/changes#diff-537e6a2fcf2f52dddfb99709a5ae95e22e1d9d2386ee4f21961e3827035cebf2
in the PR to restore the suppressions due to the updated .gitattributes. Is this okay, or should I add a test ?

romani · 2026-01-25T13:33:46Z

Please update suppression to make pitest happy.
We need to merge this PR sooner.

Anmol202005 · 2026-01-25T13:55:38Z

Please update suppression to make pitest happy. We need to merge this PR sooner.

done.

romani

I am ok to meet.

One item to be considered for future PR

romani · 2026-01-25T18:20:50Z

src/main/java/org/checkstyle/autofix/CheckstyleAutoFix.java

        final List<CheckstyleViolation> violations = reportParser
                .parse(Path.of(getViolationReportPath()));
-        final Map<CheckstyleCheck,
+        final Map<CheckstyleCheckInstance,


In next PR we need to improve names of this two records.

Agree, we should give it a better name. Having Instance in the class name is ambiguous in Java

#122 (comment)

rdiachenko

lgtm

Anmol202005 marked this pull request as draft October 28, 2025 23:09

Anmol202005 force-pushed the home branch from 5d50b1c to dc0b652 Compare October 29, 2025 11:57

Anmol202005 marked this pull request as ready for review October 29, 2025 13:25

Anmol202005 marked this pull request as draft October 29, 2025 13:26

Anmol202005 force-pushed the home branch from dc0b652 to a852ab9 Compare October 29, 2025 16:19

Anmol202005 marked this pull request as ready for review October 30, 2025 19:36

romani requested changes Nov 9, 2025

View reviewed changes

Anmol202005 force-pushed the home branch from a852ab9 to 32a5056 Compare November 13, 2025 20:02

romani requested changes Nov 16, 2025

View reviewed changes

rdiachenko reviewed Nov 16, 2025

View reviewed changes

romani mentioned this pull request Nov 20, 2025

Include full check name alongside ID in XML violation reports checkstyle/checkstyle#18088

Closed

romani force-pushed the home branch from 32a5056 to ae7fca1 Compare November 23, 2025 00:09

Anmol202005 force-pushed the home branch from ae7fca1 to d9c27e4 Compare January 18, 2026 15:33

Anmol202005 force-pushed the home branch 4 times, most recently from 195227e to 7260209 Compare January 18, 2026 18:45

romani requested changes Jan 18, 2026

View reviewed changes

Anmol202005 force-pushed the home branch from 7260209 to e01e588 Compare January 19, 2026 06:30

Anmol202005 force-pushed the home branch from e01e588 to b4d2f08 Compare January 19, 2026 06:47

Anmol202005 marked this pull request as draft January 19, 2026 06:48

Anmol202005 force-pushed the home branch 5 times, most recently from 3f04884 to 80bdccb Compare January 19, 2026 07:17

Anmol202005 marked this pull request as ready for review January 19, 2026 07:19

romani requested changes Jan 25, 2026

View reviewed changes

src/main/java/org/checkstyle/autofix/CheckstyleCheckInstance.java Outdated Show resolved Hide resolved

Anmol202005 force-pushed the home branch from 80bdccb to e0fbc20 Compare January 25, 2026 09:04

Anmol202005 force-pushed the home branch from e0fbc20 to 78b82af Compare January 25, 2026 11:48

Anmol202005 marked this pull request as draft January 25, 2026 11:55

Anmol202005 force-pushed the home branch from 78b82af to 45cb390 Compare January 25, 2026 12:03

Anmol202005 marked this pull request as ready for review January 25, 2026 12:12

Anmol202005 force-pushed the home branch from 45cb390 to eea3ab2 Compare January 25, 2026 12:26

Issue checkstyle#122: Added support for multiple instances of same check

fcfeaeb

Anmol202005 force-pushed the home branch from eea3ab2 to fcfeaeb Compare January 25, 2026 13:55

romani approved these changes Jan 25, 2026

View reviewed changes

romani assigned rdiachenko Jan 25, 2026

rdiachenko approved these changes Jan 25, 2026

View reviewed changes

rdiachenko merged commit a22e41a into checkstyle:main Jan 25, 2026
4 checks passed

rdiachenko mentioned this pull request Jan 25, 2026

Support multiple instances of the same Checkstyle check with different IDs #122

Closed

romani mentioned this pull request Jan 25, 2026

Better names for classes CheckstyleCheck, CheckstyleCheckInstance #132

Closed


		package org.checkstyle.autofix;

		public class CheckstyleConfigModule {

Conversation

Anmol202005 commented Oct 28, 2025

Uh oh!

Anmol202005 commented Oct 29, 2025

Uh oh!

rdiachenko commented Oct 29, 2025

Uh oh!

romani commented Oct 30, 2025

Uh oh!

Anmol202005 commented Oct 30, 2025

Uh oh!

Anmol202005 commented Oct 30, 2025

Uh oh!

romani commented Oct 31, 2025

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Anmol202005 commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romani commented Nov 20, 2025

Uh oh!

romani commented Nov 23, 2025

Uh oh!

romani commented Nov 30, 2025

Uh oh!

Anmol202005 commented Jan 18, 2026

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anmol202005 Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Anmol202005 commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Anmol202005 commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Anmol202005 commented Nov 13, 2025 •

edited

Loading

Anmol202005 Jan 19, 2026 •

edited

Loading

Anmol202005 commented Jan 25, 2026 •

edited

Loading

Anmol202005 commented Jan 25, 2026 •

edited

Loading