Irrelevant Check inputs

[Saket]

I was approaching Issue#11163 and was scoping out the different Check inputs in the process. I observed that more often than not, various checks contained same input file. The said file has variety of unrelated inputs for name format, accessor, method size, method pattern etc.

This isn't an isolated issue, running
grep -r "Contains simple mistakes" src/ | tee >(wc -l)
gives out 31 matches for that javadoc comment found in all these files. Output list is appended at the end.
Note: This may not contains all occurrence of this issue as this relies on the fact that such file carry on that original javadoc comment.

Example Check input:
ParameterNumber (Documentation) (Check test)

Files with said issue:
InputParameterNumberSimple,InputParameterNumberSimple2,InputParameterNumberSimple3,InputParameterNumberSimple4

Explanation for why the files above contains irrelevant inputs:
The check is for enforcing number of parameters of a method or constructor, but these inputs are related to various other checks like name format, method patterns, constants etc.

Examples:
/** Invalid format **/
public static final int badConstant = 2;
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
/** should be private **/
protected static int sTest3;

That being said, these input checks may contain related tests for a few Check inputs. In this case an example for the relevant thing is void toManyArgs:

• Issue is that there is no need to have all the other irrelevant inputs packaged with ParameterNumber check.
• In this case, similar type of input with same configuration is present in another input for that check, but even if it wasn't present it could be refactored or made into separate input with changed config if needed.

Also, all of these files contains unused java.io import (Line 11).

Proposed fix:
Keep only relevant portions of the inputs from these file or refactor them into existing input for the same check, or create a separate file if required, and change the corresponding Check test accordingly.

File list that contains this issue but may not be limited to: Google sheets

Should I open an issue? or am I missing something here and this is supposed be this way?

[Roman Ivanov]

Please open issue with description of this email 

--
You received this message because you are subscribed to the Google Groups "checkstyle-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to checkstyle-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/checkstyle-devel/c4f67134-9a77-4e48-970f-eafaf272b687n%40googlegroups.com.

[Manish Krishna]

Hey!!

I have been working on Issue #11163 and Issue #13213 for almost a month and in the process, had the oppurtunity of browsing through numerous Check inputs. Now, I couldn't agree more that there are duplicate checks found in multiple files. This is in fact increasing the vastness of the code per say. Also, a part of the reason for checkstyle Issue #11163: Enforce file size on Java inputs is the increased number of lines per file. 

However, I don't think it is easy or in fact possible to implement your idea of removing the said irrelevant checks. If you have observed, there are pitests enabled in the project which I think are prone to failing in case of modifying the files by removing all such duplicate checks.

To add, I myself have included duplicate checks multiple times while working on the files to enforce the limit of 120 lines in order for the files to work in accordance to the pitests. You can check the files changed in one of my PR's Issue #11163: Enforced file size MethodCount1#14305 for reference.

But again, I am no expert and this is just on basis of my observations and from what I have learnt about the project functionality so far.