[arturobernalg]

excludeSubProjects becomes ineffective as soon as the deprecated excludes config is also used, because ArgumentTracker#validateSet (and the same logic generated from Args.tpl for BaseRatMojo/the Ant task) treats input-exclude and the deprecated exclude as mutually exclusive, which is wrong for a multi-valued option: both should accumulate.

validateSet now returns immediately for a multi-valued option, and values from every alias merge into the non-deprecated key once more than one alias is actually used (not unconditionally, since that broke commons-cli's own deprecation notice for the common case — see the second commit).

Applied in both ArgumentTracker and Args.tpl (shared by the Maven plugin and the Ant task).

Tests: a unit test reproducing the silent drop, and a new apache-rat-plugin IT (src/it/RAT-552) combining excludeSubProjects with a deprecated <excludes>.

[ottlinger]

Thanks for your help here - would you mind taking a look at RAT-97 as well as it addresses errors with the same parameter?

[ottlinger]

LGTM - thanks for your contribution.
I've applied minor changes and added a changelog entry.

@Claudenw do you see problems with the new layout in 1.0.0? Thanks

[Claudenw]

@ottlinger I think the solution is in the wrong place, but I may be convinced otherwise.

@arturobernalg excludeSubProjects is a Maven only option, and this is the root of my quandary.

RAT is a CLI first implementation. So all general implementations start in the RAT CLI code. The RAT CLI configurations are supposed to drive the other UI (Ant, Maven, and eventually Gradle).

The UI should not allow multiple configuration options from the same Arg group. That is how we do deprecation.

The issue is that the Maven UI does not determine if the current or deprecated Arg group option was used. It should do that and add the modules to the correct one.

This means that the addition of the exccludeSubProjects files to the exclude list must be done later than it is. Currently this is in the AbstractRatMojo at line 441, however that all changes in #749 .

However, I see the point of your change in that if any UI ends up using 2 or more Arg group options the correct processing is completed.

We do have on Arg group that has 2 deprecated options, however it has only one argument.

OUTPUT_STYLE(new OptionGroup()
            .addOption(Option.builder().longOpt("output-style").hasArg().argName("StyleSheet")
                    .desc("XSLT stylesheet to use when creating the report. "
                            + "Either an external xsl file may be specified or one of the internal named sheets.")
                    .build())
            .addOption(Option.builder("s").longOpt("stylesheet").hasArg().argName("StyleSheet")
                    .deprecated(DeprecatedAttributes.builder().setSince("0.17").setForRemoval(true).setDescription(StdMsgs.useMsg("--output-style")).get())
                    .desc("XSLT stylesheet to use when creating the report.")
                    .build())
            .addOption(Option.builder("x").longOpt("xml")
                    .deprecated(DeprecatedAttributes.builder()
                            .setSince("0.17")
                            .setForRemoval(true)
                            .setDescription(StdMsgs.useMsg("--output-style with the 'xml' argument")).get())
                    .desc("forces XML output rather than the textual report.")
                    .build()),
    ```

Would you write a test that shows that if there is an Arg group that accepts multiple arguments and has 2 deprecated options that your solution still holds?