Check.suppressForClasses as alternative to suppressions
Yuriy Chulovskyy
Yuriy:
When somebody tries to introduce Checkstyle to project he sees thousands of violations.
Current approach is to create/generate suppressions XML document that contains a set of suppress elements.
Cons: creating it manually is painful. Generation from source code is
not yet implemented. Suppression on line level is not "stable" because
adding a line into the source file will brake suppressions.
Suggested approach:
The natural way is when person goes through the vilotations and adds "suppression" for the rule.
CodeNarc has doNotApplyToClassNames property for each rule
https://github.com/groovy/groovy-core/blob/master/config/codenarc/codenarc.groovy
We can introduce suppressForClasses property for each Check that works with classes.
Field format should be natural for developer: simple class name,
canonical calss name, package name with .* at the end (e.g.
com.mycompany.dao.*)
yes , but suggested apprach is also not implemented.
Suppression on line level is not "stable" because adding a line into the source file will brake suppressions.
I we failed to pass to GSoC 2015 , that idea is become public - https://github.com/checkstyle/checkstyle/wiki/Checkstyle-GSoC-2015-Project-Ideas#project-name-flexible-suppression-model
The natural way is when person goes through the vilotations and adds "suppression" for the rule.
http://checkstyle.sourceforge.net/config.html#Examples here is how you can suppress Whole Check on file(folders) easily. But we have problem that eclipse-CS and Idea plugins doesnot allow you to add suppress to a Check easily, we needto report issue against them.
CodeNarc has doNotApplyToClassNames property for each rule
For now I am not ready for that, as it is extension of API (I am afraid to extend API , but I do not see benefits), as each option to check is API for us. And it is not a Check business - it is business of logger/filter . So it have to be hosted in Filter area of Checkstyle. Check should handle cases that are not possible to be done by Filters.
Yuriy:
@romani The natural way is when you see the violation and you don't want to fix it, you just open the config file and add exception.
You don't want to open separate suppression file since it is context switch.
The suppression file doesn't allow to suppress the rule for a list of
files (at list in one place, like
RuleName.suppress="MyTest.java,AnotheTest.java"
That is why I would like to keep rule and esception in one place. It doesn't mean that Check should filter messages. It just configuration. It's up to us how we parse the configuration file.
Another issue is XML.
I'll try to create proove of concept for configurations like CodeNarc has https://github.com/groovy/groovy-core/blob/master/config/codenarc/codenarc.groovy (may require Groovy)
Another solution we may try is HOCON format https://github.com/typesafehub/config/blob/master/README.md#features-of-hocon (pure Java AFAIK)
Roman:
The natural way is when you see the violation and you don't want to fix it, you just open the config file and add exception.
NO :) , the natural way then you see a violation and do right click on it and select "False-positive" or "Ignore" and IDE do the trick. This is how Sonar do their ignore management.
As soon as you open a config - you are advanced user, so welcome to config nuances.
You don't want to open separate suppression file since it is context switch.
you already switched the context, by switching from java code to some strange xml file (checkstyle config). But I agree that having two config files for suppression is extra complexity (that might be done like that to avoid overcomplicate dtd schema for config file, and test suppression before big changes). We could add ability to host suppressions in the same file - here I agree - two files for config is inconvenient.
The suppression file doesn't allow to suppress the rule for a list of files (at list in one place, like RuleName.suppress="MyTest.java,AnotheTest.java"
Yes, current suppressions are not ideal but they do cover your simple case, please review http://checkstyle.sourceforge.net/config.html#Examples, for your example:
<suppress checks="RuleName" files="MyTest.java|AnotheTest.java"/>
That is why I would like to keep rule and esception in one place. It doesn't mean that Check should filter messages. It just configuration. It's up to us how we parse the configuration file.
So what about to teach Checkstyle to keep suppression in the same config file as first step ?
Right now what is placed inside a module tag are fields of Check, if we
put options of smth other (Filter) that will be easy place for
misunderstanding. More over how it should work if no Filter is declared
in config ..... I need from you a bit more detailed picture/design of
it.
Groovy
I like ideas to extend Checkstyle to be more modern in configuration, but I will not be ok to put Groovy as a binary dependency, we need some light way to parse that format.
HOCON
It might be fun too, but we should not become heavy on extra dependencies.
Another issue is XML.
yes, I even thought that json will also be good for config and output
format, to let others make online services on it easily, that will
bring us more to idea checkstyle as a service.
BUT !!! before you rush to implementation of any extra config format ,
think a bit more about documentation of that new formats, we have a lot
of examples of XML on our web site. If we will not provide easy way to
see config all supported format, user will stay with XML as it there are
a lot of examples. That problem I do not know how to resolve easily for
now.
Yuriy:
BTW, another good idea for suppression: keep context (current line text. previous line text and next line text) instead of line number to identify place of suppressed check. Inspired by http://habrahabr.ru/company/pvs-studio/blog/252493/
Roman:
Please read about SuppressWithNearbyCommentFilter ant its option influenceFormat.
please close this issue and lets switch to mail list to discuss new ideas.