Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Multiple Configuration Files #991

Closed
rnveach opened this issue Apr 26, 2015 · 16 comments
Closed

Support Multiple Configuration Files #991

rnveach opened this issue Apr 26, 2015 · 16 comments

Comments

@rnveach
Copy link
Member

rnveach commented Apr 26, 2015

I would like support for running with multiple configuration files.
Example: java com.puppycrawl.tools.checkstyle.Main -c /google_checks.xml -c /custom_checks.xml

I tried this in 6.5 and it only run the first check and ignored the second.

I see this as useful if we want to add our own custom checks to be run alonside a standard one, like google's. The current way around this would be to copy the google xml and append new checks to the end of it, but then we would be required to do this for every project. The configurations in jar are there so we don't have to keep reproducing it. Not to mention if the original xml should be updated, this would require me to update the custom xml, along with the jar.

There are also other times where I can't put all my checks in the same file because of the "RegexpSingleline" module where I want to run different regular expressions on different file types. Having them all in the same configuration on all file types will produce incorrect errors because some checks are only made for one type, and it is allowed in another. I could run checker twice with the different configurations, but that is extra work and time going through the directory tree multiple times.

@romani
Copy link
Member

romani commented Apr 26, 2015

That issue will be considered after moratorium period, as it require serious changes.

List of problems/tasks:

  • override rules of checks,
  • override rules for filters, suppressions
  • override of severity.

The current way around this would be to copy the google xml and append new checks to the end of it, but then we would be required to do this for every project.

Google's config is 1/3 of all Checks, we will offer user more to force him to use custom config.
If you have a problem to use "global" checkstyle config file (hosted somewhere in web as config location could be even remote), you need to deploy to your maven repository updated chekstyle jar with updated config.

There are also other times where I can't put all my checks in the same file because of the "RegexpSingleline" module where I want to run different regular expressions on different file types.

Few projects, each of them will use base configuration that is inside a jar, and some custom config file that is specific to each project. OK , make sense.
As workaround for now, you can put a name to Check/Module in config and provide suppression base on class packages or file path - http://checkstyle.sourceforge.net/config.html#Filters (see first examples block).

@rnveach
Copy link
Member Author

rnveach commented Dec 5, 2017

@romani I am bringing this issue back up.

override rules of checks,
override rules for filters, suppressions
override of severity.

I think this issue should only concern itself with allowing multiple, separate configurations to be run. Issue #2873 already started a discussion of inheritance for configurations and I think it requires many changes to the configuration to allow fine granularity of changing things.

For this, I think we should allow running multiple configurations, one after the other, with the same set of files passed in through the CLI. It would allow us to chain multiple configurations together while only scanning the directories for files once.
I, myself, specifically use multiple configurations to split files by types since Checkstyle only supports Java natively and requires regular expressions for other files. One regular expression for one file type could create false positives for another, so this is why I took this approach.

@danon
Copy link

danon commented Jul 8, 2019

So are multiple configuration files going to be supported?

@romani
Copy link
Member

romani commented Jul 9, 2019

@danon , please share your use-case of multiple configuration files.
we did not do any progress as we are not sure how to design it to work in all extensions/plugins the same.

@danon
Copy link

danon commented Jul 9, 2019

I don't need any inheritance/override. I would like to have separate files, like so:

checkstyle_lint_rules_functions.xml
checkstyle_lint_rules_string.xml
checkstyle_lint_rules_indents.xml
checkstyle_lint_rules_misc.xml

simply to separate rules into files. I would expect checkstyle to simply sum the rules into one checkstyle_lint_rules.xml under the hood. Simple concatenation would be enough for me.

Kinda like @import in CSS doesn't override any styles, only adds new styles.

@romani
Copy link
Member

romani commented Jul 10, 2019

Look at how DTDs ENTITY feature can be useful for you, example https://android.googlesource.com/platform/prebuilts/checkstyle/+/master/android-style.xml .
Please be aware of #6474 , you need to activate ENTITY loading by special java property.
Please if such approach work for you, please share details, for other users.
If such approach is not good for you, please share more details on why you want to split file

@rnveach
Copy link
Member Author

rnveach commented Jul 10, 2019

@romani see my reason why at #991 (comment) . I think this is why I personally can't use ENTITY loading. These regular expressions are specialized for the file type and combining them with the wrong types will create false positives.

@romani
Copy link
Member

romani commented Jul 10, 2019

So requests are very different:
@danon looks like just try to keep modules configuration in separate files to ease something on his side, but ant the end it will be executed as single configuration on one execution. I am not sure why checkstyle config become so big, we use all Checks in our config and is it reasonably sized in comparison to other configs.
Attention that @danon have to "include" Modules(Checks/Filters) under common TreeWalker module to do parsing of files ones.

@rnveach looks like interested in approach to use different configs BUT to reuse checkstyles scanning of files and content load (parsing of java files). This was originally requested in issue.
So @rnveach requesting merge of configs, to let in second config extend module set or override certain Check (for example to set severity=ignore). but it will be merge of configs NOT chaining.
I think we can do this as special mode, merge of configs will be done ones before whole checkstyle execution, so no API updates will be required.

Chaining of configs is reasonable also but probably it is better to implement it in plugins(ant, maven, eclipse). As in Checkstyle we have all under Checker module, so all parsed/loaded files are under its instance. Second config will mean separate Checker instance, so nothing will be shared. Sharing of content is "blocked" by Multithread mode discussion. Lets create separate issue on this, and collect ideas there.

NOTE: we might make something like PMD have for including content from another file - https://github.com/checkstyle/checkstyle/blob/master/config/pmd-main.xml#L12 but it will help @danon , not @rnveach . But problem is that we have Treewalker module in config and some rules/checks/modules are under Checker or Treewalker. Simple include will not be possible, but we can invent new config format that will contain only modules for include (as defined by user) under Checker or TreeWalker in main config. If you both agree on such idea, lets create separate issue on this.

@danon
Copy link

danon commented Jul 10, 2019

@romani any way of splitting config into separate files would be great, so I'm ok with this :)

@romani
Copy link
Member

romani commented Jul 11, 2019

Please share a reason of split, without reason we unlikely do anything.

@rnveach
Copy link
Member Author

rnveach commented Jul 12, 2019

to reuse checkstyles scanning of files and content load (parsing of java files).
so nothing will be shared

re-use scanning of files only. The files are all different types, so there is no more Java files after the other configs. Example file types are Java, properties, XML, and JSP. I don't expect anything to be shared between multiple configs.

So @rnveach requesting merge of configs

No. Merging the configs into 1 will cause issues with regular expression checks like I mentioned before. A expression for 1 file type won't work for another.

Chaining of configs is reasonable also but probably it is better to implement it in plugins(ant, maven, eclipse).

I personally don't use plugins. Just checkstyle directly at this time. My project hasn't switched to Maven yet.

@danon
Copy link

danon commented Jul 12, 2019

I also don't use plugins, I like my project clean and without any redundancies (for now). Loading rules from multiple files would really help me. Or even a single rule

<include file="check_style_custom.xml"/>
// or

<additional-rules file="check_style_custom.xml"/>

would be enough.

@romani
Copy link
Member

romani commented Jul 16, 2019

Loading rules from multiple files would really help me.

Please explain a reason to split config in multiple files. Right now we are and more focused to allow have all in one config file ( for good reason). your reason is still unclear for me, I see no benefits.

@romani
Copy link
Member

romani commented Jul 16, 2019

I don't expect anything to be shared between multiple configs.

Please create new issue on this to support by CLI only. We will discuss exact names and workflow.
Ant already can do this.

Merging the configs into 1 will cause issues with regular expression checks like I mentioned before. A expression for 1 file type won't work for another.

It is separate mode, user will be responsible for all conflicts. We will provide ability to do this, user should think on how to override properly to get required result. It will not be solution for all cases, but it will help to slightly modify Google config without coping of it .
Do you agree on new issue for this mode ? To discuss details.

@rnveach
Copy link
Member Author

rnveach commented Jul 16, 2019

@romani

Please create new issue on this to support by CLI only.

Maybe I am confused, but we are discussing a new issue for running multiple configurations in Checkstyle, right?
I can do this but it would be the exact same issue description and similar title. First post already talks about multiple config run and mentions CLI.

user should think on how to override properly

This issue was never about inheritance, if this is what you mean by "override". There is another issue on that. I am just literally looking for running multiple configs side by side with the same list of files scanned.

@romani
Copy link
Member

romani commented Aug 5, 2019

discussion is moved to #6942 for chaining of configs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants