CLI: support sequential execution of multiple configs

[romani]

Originally was reported and discussed at #991 .

Example: java com.puppycrawl.tools.checkstyle.Main -c /google_checks.xml -c /custom_checks.xml my-source-folder/

It is 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. If the original google xml should be updated, this would require users 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.

Chaining of configs is reasonable also but probably it is better to implement it in application/plugins(cli, 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.
CLI will be responsible reuse files-scanning is used defined target as folder. Nothing else should be shared, between executions.

This new extension of -c should recheck compatibility with all others arguments:

• suppression file generation should be separate as it suppose to be used in certain config. Or make such modes incompatible.
• output file is consolidated or separate ??!!

Attention:
This issue is not about inheritance of configs that is separate issue - #2873 .
This issue is not about ability to "include/inline/aggregate" some other config files - requested at #991 (comment) .

[romani]

@rnveach , do you really think it is reasonable to update CLI ? as whole benefit is just to share list of files for validation in folder.
Drawback is to reconcile conflicts with other arguments.

Similar mode for Ant will be a struggle, https://checkstyle.org/anttask.html , as it will be big question consolidation or separation of outputs/results. Example: maxErrors most likely will be reasonable to apply on consolidated result.

Maven plugin have property like - https://maven.apache.org/plugins/maven-checkstyle-plugin/checkstyle-mojo.html#suppressionsLocation that will be a struggle if we go with separation approach, but aggregation is also a problem, as same suppression filter should be used in different configs (performance issue).

So, all tell us that results/reports should be consolidated.

Questions:

1. Does it really make sense ? if we try to reuse scanning, but in contrast deal with bunch of issues at the end of validation (aggregation or separation).
   Why not simply run checkstyle CLI twice ?

2. How do you plan to use suppression for Google config only ? suppression module have to inserted into config, so config have to changed. On big sources it is hard to run without any false-positive (we already know issues to match Google style). IDEA: we can place https://checkstyle.org/config_filters.html#SuppressionFilter in google config and use some property expansion with optional=true so user will be able to define suppression if required.

3. if scanning is an issue on your side, you can make it ones and give list of all files paths as one file.
   https://checkstyle.org/cmdline.html#Command_line_usage

   Our CLI supports definition of arguments in file by means of AtFiles feature and also command line completion in Bash or ZSH Unix shells, see how to make it here.

[rnveach]

@romani I leave it to you to decide on if something is reasonable or not when I present the requested change. I am fine if this won't be supported.

Maven plugin

It would be up to them to add support or not. As long as we don't break their usage, then I am fine.

Why not simply run checkstyle CLI twice ?
maxErrors

There is no reason not to. It is just a performance issue, of re-scanning files.
I am not looking to combine configs in anyway. They should just output one after the other with no interaction between them. Possibly the only interaction I would want to see is the combination of errors from all configs after the full run.

How do you plan to use suppression for Google config only ?

I saw this issue as configurations and violations are completely separate. There would be no way to suppress a violation from 1 config with changes to another. This requires inheritance in other issue.

you can make it ones and give list of all files paths as one file.

Command lines have a max length. I would also have to duplicate any exclusions to create the list of files in my new external routine. Checkstyle would still have to re-parse the files to check if they are directories or not. I don't see much benefit in this over just running checkstyle twice.

if scanning is an issue on your side

It is not a true issue. It is just something I would like to see as a small performance boost.

[romani]

ok, I recommend you to propose behavior change before codding.
There will be bunch of nuances in CLI arguments conflicts.
Do not forget about -o option. Do not forget pipe processing by other tools. In case of xml type output ... if you output two xml one by one ... piped xml processor will fail.

It is just a performance issue, of re-scanning files.
I would like to see as a small performance boost

on the moment of declaring changes ... please share statistic on what time boost we speak, for example on most extream - openjdk11 source code base.

I saw this issue as configurations and violations are completely separate.

It is a bit off topic .... my bad ... but slightly related. I was inspired by your idea to help others to not change google config, and still do not see a how it would help others.
If this is not going to help others, it is better to host such mode in your forked project or in jdk6 port project, to let you have it and benefit from it. Contribute it to upstream ones it become mature.

[linusjf]

How about having a parent file that specifies the multiple configuration files in use on the project ? This way you could also specify single or multiple suppression files either in append mode or matched separately to the config files? Even specify suppression file names generated as per the matching config file name. That ought to avoid cluttering up the command line. Or do you want to do this via properties file?

[jensli]

Why not simply run checkstyle CLI twice ?

This would probably work for my usecase! But how can I configure the Maven plug-in for this?

[romani]

IDEA: we can place https://checkstyle.org/config_filters.html#SuppressionFilter in google config and use some property expansion with optional=true so user will be able to define suppression if required.

This is already done.
https://checkstyle.org/google_style.html#Google_Suppressions

How about having a parent file that specifies the multiple configuration files in use on the project ?

This is already possible by dtd magic syntax.
https://checkstyle.org/config_system_properties.html#Enable_External_DTD_load

This way you could also specify single or multiple suppression files either in append mode or matched separately to the config files?

You can use properties and optional field of filters.
See example

checkstyle/src/main/resources/google_checks.xml

Lines 363 to 368 in c3c7ab8

	<module name="SuppressionXpathFilter">
	<property name="file" value="${org.checkstyle.google.suppressionxpathfilter.config}"
	default="checkstyle-xpath-suppressions.xml" />
	<property name="optional" value="true"/>
	</module>
	<module name="SuppressWarningsHolder" />

But how can I configure the Maven plug-in for this?

Just make two instances of maven plugin in pom.xml. it works. Or two execution https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/46499e521b4bdd2783177fe5563069835fdfa68c/sevntu-checks/pom.xml#L263

[romani]

I think this issue does not have a future for now.
Too law demand with a lot of problems to implement.

[rnveach]

How about having a parent file that specifies the multiple configuration files in use on the project ?
This is already possible by dtd magic syntax.

It only supports combining partial configurations (see checkstyle-common.xml), not full configurations.
Also, this is only possible if you have write access to all configurations to make the change and can't be used with things like the google/sun config, which has been requested many times.

[josephmate]

Originally, I thought I needed this because we have a large multi repo, multi module codebase. All the repos and modules share the same checkstyle config. I cannot introduce new checks into the shared checkstyle because it will cause all repos to to fail CI until violations are fixed. It's too difficult to update all repos at once. Unfortunately the versioning strategy, which I cannot change, doesn't allow us put a separate version on our checkstyle config module.

As a result, I was thinking I needed multiple checkstyle config support. In the repos where I was rolling in the new change, I wanted to include the new checks under that new file, in addition to the existing check. Once the rollout to all repos was complete, I would combine them and get rid of the second file.

I realized that I could just run two instance of checkstyle in maven, one for each file, allowing me to slowly rollout the change repo by repo.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>${maven.checkstyle.plugin.version}</version>
    <dependencies>
        <dependency>
            <groupId>com.puppycrawl.tools</groupId>
            <artifactId>checkstyle</artifactId>
            <version>${checkstyle.version}</version>
        </dependency>
        <dependency>
            <groupId>company</groupId>
            <artifactId>checkstyle.config</artifactId>
            <version>${checkstyle.config.version}</version>
        </dependency>
    </dependencies>
    <executions>
        <execution>
            <id>validate</id>
            <phase>validate</phase>
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
    <configuration>
        <configLocation>checkstyle.xml</configLocation>
...
    </configuration>
</plugin>
<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>${maven.checkstyle.plugin.version}</version>
    <dependencies>
        <dependency>
            <groupId>com.puppycrawl.tools</groupId>
            <artifactId>checkstyle</artifactId>
            <version>${checkstyle.version}</version>
        </dependency>
        <dependency>
            <groupId>company</groupId>
            <artifactId>checkstyle.config</artifactId>
            <version>${checkstyle.config.version}</version>
        </dependency>
    </dependencies>
    <executions>
        <execution>
            <id>rolling-out-new-checkstyle</id>
            <phase>validate</phase>
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
    <configuration>
        <configLocation>rolling-out-new-checkstyle.xml</configLocation>
 ...
    </configuration>
</plugin>

I hope someone finds this solution useful if you're in a similar situation. It's not as nice as having proper versioning on the config, but it works :)