Issue #122: Support multiple instances of the same Checkstyle check

[Anmol202005]

Fixes: #122

[Anmol202005]

@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]

@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]

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]

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]

@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]

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

[romani]

can we rename "source" to "checkId" ?

I know xml attribute is "source", very bad name, very confusing, and now we copied it new project. Better to have better name that is logical and explain what is this.

[romani]

is first parameter checkId ?

[romani]

rename parameter please.