Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ public void apply(final BiConsumer<String, List<String>> consumer) {

/**
* Validate that the option is defined in Args and has not already been set.
* This check will verify that only one of the keys in the group can be set.
* This check will verify that only one of the keys in the group can be set, unless the option accepts
* multiple arguments, in which case every alias of the group accumulates (see
* {@link #resolveStorageKey(String)}) and there is nothing to conflict with.
* @param key the key to check
* @return {@code true} if the key may be set.
*/
Expand All @@ -113,6 +115,9 @@ private boolean validateSet(final String key) {
if (arg != null) {
final Option opt = arg.find(key);
final Option main = arg.option();
if (opt.hasArgs()) {
return true;
}
if (opt.isDeprecated()) {
args.remove(extractKey(main));
// deprecated options must be explicitly set so let it go.
Expand All @@ -129,6 +134,42 @@ private boolean validateSet(final String key) {
return false;
}

/**
* Returns the key under which the value(s) for {@code trackerKey} are to be stored, migrating any values
* already held under another alias of the same group into that key first.
* As long as only one alias of a multi-valued option's group has ever been used, values stay under that
* alias's own key, so the final argument list built by {@link #args()} still names the deprecated option
* when that is the one the caller actually used, preserving the commons-cli deprecation notice for it.
* Only once a second alias of the group is written to does this method consolidate: it moves the values
* already stored under the other alias into the key of the group's non-deprecated option and returns that
* key, so the deprecated and non-deprecated spellings of the same multi-valued option (for instance
* {@code --exclude} and {@code --input-exclude}) accumulate their values together instead of one silently
* discarding the other's (RAT-552), while {@link #args()} still names each option of a group only once, as
* commons-cli requires. Any single-valued option is stored under its own key, as passed in.
* @param trackerKey the key as passed to {@link #setArg(String, String)} or {@link #addArg(String, String...)}.
* @return the key under which the value(s) are to be stored in {@link #args}.
*/
private String resolveStorageKey(final String trackerKey) {
final Arg arg = Arg.findArg(trackerKey);
if (arg != null) {
final Option opt = arg.find(trackerKey);
if (opt.hasArgs()) {
final String canonicalKey = extractKey(arg.option());
for (final Option o : arg.group().getOptions()) {
final String otherKey = extractKey(o);
if (!otherKey.equals(trackerKey)) {
final List<String> otherValues = args.remove(otherKey);
if (otherValues != null) {
args.computeIfAbsent(canonicalKey, k -> new ArrayList<>()).addAll(otherValues);
return canonicalKey;
}
}
}
}
}
return trackerKey;
}

/**
* Set a key and value into the argument list.
* Replaces any existing value.
Expand All @@ -155,7 +196,7 @@ public void setArg(final String trackerKey, final String value) {
DefaultLog.getInstance().debug(String.format("Setting %s to '%s'", trackerKey, value));
}
values.add(value);
args.put(trackerKey, values);
args.put(resolveStorageKey(trackerKey), values);
} else {
DefaultLog.getInstance().warn(String.format("Key '%s' does not accept arguments.", trackerKey));
}
Expand Down Expand Up @@ -209,7 +250,7 @@ public void addArg(final String trackerKey, final String... value) {
if (DefaultLog.getInstance().isEnabled(Log.Level.DEBUG)) {
DefaultLog.getInstance().debug(String.format("Adding [%s] to %s", String.join(", ", Arrays.asList(value)), trackerKey));
}
List<String> values = args.computeIfAbsent(trackerKey, k -> new ArrayList<>());
List<String> values = args.computeIfAbsent(resolveStorageKey(trackerKey), k -> new ArrayList<>());
values.addAll(newValues);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,32 @@ void setOverridesAddArg() {
assertThat(underTest.getArg(mappedOption.keyValue())).contains(List.of("baz"));
}

@Test
void addArgOnDeprecatedAliasAccumulatesWithNonDeprecatedAlias() {
// RAT-552: --exclude (deprecated) and --input-exclude (not deprecated) are two spellings of the same
// multi-valued option; adding values under one must not discard values already added under the other.
underTest.addArg("exclude", "hello.txt");
underTest.addArg("input-exclude", "module/**");

assertThat(underTest.getArg("input-exclude")).contains(List.of("hello.txt", "module/**"));
// the deprecated alias is not a separate entry: commons-cli would reject an argument list naming two
// options of the same group.
assertThat(underTest.getArg("exclude")).isEmpty();
assertThat(underTest.args()).containsExactly("--input-exclude", "hello.txt", "module/**");
}

@Test
void addArgOnOnlyDeprecatedAliasKeepsItsOwnKey() {
// RAT-552: using only the deprecated alias, with no competing non-deprecated value, must keep the value
// under the deprecated key itself, not move it to the non-deprecated one. The deprecated flag name has
// to be the one that reaches commons-cli, or its own deprecation notice for that option never fires.
underTest.addArg("exclude", "hello.txt");

assertThat(underTest.getArg("exclude")).contains(List.of("hello.txt"));
assertThat(underTest.getArg("input-exclude")).isEmpty();
assertThat(underTest.args()).containsExactly("--exclude", "hello.txt");
}

@Test
void invalidAbstractOption() {
Option option = Option.builder().longOpt("notAValidOption").build();
Expand Down
16 changes: 16 additions & 0 deletions apache-rat-plugin/src/it/RAT-552/invoker.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

invoker.goals = -X clean apache-rat:rat
1 change: 1 addition & 0 deletions apache-rat-plugin/src/it/RAT-552/notes.bak
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Scratch notes with no license header, excluded via the deprecated <excludes> configuration.
59 changes: 59 additions & 0 deletions apache-rat-plugin/src/it/RAT-552/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
RAT-552 integration test
Copyright (C) 2026, ASF Apache Creadur

Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.apache.rat.test</groupId>
<artifactId>it-rat552</artifactId>
<version>1.0</version>
<packaging>pom</packaging>
<modules>
<module>submodule</module>
</modules>
<build>
<plugins>
<plugin>
<groupId>org.apache.rat</groupId>
<artifactId>apache-rat-plugin</artifactId>
<version>@pom.version@</version>
<configuration>
<verbose>true</verbose>
<consoleOutput>true</consoleOutput>
<ignoreErrors>false</ignoreErrors>
<excludeSubProjects>true</excludeSubProjects>
<excludes>
<exclude>notes.bak</exclude>
</excludes>
</configuration>
</plugin>
</plugins>
<!-- RAT-354: prevent warning about missing version property of maven-clean-plugin -->
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-clean-plugin</artifactId>
<version>3.5.0</version>
</plugin>
</plugins>
</pluginManagement>
</build>
</project>
2 changes: 2 additions & 0 deletions apache-rat-plugin/src/it/RAT-552/submodule/README.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Content of the submodule. The parent build must exclude this whole directory from its own
RAT report when excludeSubProjects is combined with a deprecated exclude configuration.
30 changes: 30 additions & 0 deletions apache-rat-plugin/src/it/RAT-552/submodule/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
RAT-552 integration test
Copyright (C) 2025, ASF Apache Creadur

Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.apache.rat.test</groupId>
<artifactId>it-rat552</artifactId>
<version>1.0</version>
</parent>
<artifactId>it-rat552-submodule</artifactId>
</project>
37 changes: 37 additions & 0 deletions apache-rat-plugin/src/it/RAT-552/verify.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

content = new File(basedir, 'build.log').text

// verify debug output
assert content.contains('BUILD SUCCESS')

// RAT-552: excludeSubProjects derives an "input-exclude" pattern per module. Before the fix, that call was
// silently dropped whenever the deprecated "excludes" configuration had already been set, so this line would
// never appear at all (no warning either, since the drop happens before any logging).
assert content.contains('Adding [submodule/**] to input-exclude')

// the deprecated alias must still be honoured
assert content.contains('Adding [notes.bak] to exclude')

// Report is in apache-rat-plugin/target/invoker-reports
// Make sure that report is generated completely, and that the submodule directory was actually excluded
// from the parent's own scan, rather than merely logged as excluded.
report = new File(basedir, 'target/site/rat-report.html').text
assert ! report.contains('/submodule/pom.xml')
assert ! report.contains('/submodule/README.txt')
assert report.contains('Unapproved: 0')
50 changes: 46 additions & 4 deletions apache-rat-tools/src/main/resources/Args.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@
if (arg != null) {
Option opt = arg.find(key);
Option main = arg.option();
if (opt.hasArgs()) {
// a multi-valued option accumulates values from every alias of its group, deprecated or not,
// via resolveStorageKey(), so there is nothing to conflict with here.
return true;
}
if (opt.isDeprecated()) {
args.remove(argsKey(main));
// deprecated options must be explicitly set so let it go.
Expand All @@ -59,6 +64,42 @@
return false;
}

/**
* Returns the key under which the value(s) for {@code key} are to be stored, migrating any values already
* held under another alias of the same group into that key first.
* As long as only one alias of a multi-valued option's group has ever been used, values stay under that
* alias's own key, so the final argument list built by {@link #args()} still names the deprecated option
* when that is the one the caller actually used, preserving the commons-cli deprecation notice for it.
* Only once a second alias of the group is written to does this method consolidate: it moves the values
* already stored under the other alias into the key of the group's non-deprecated option and returns that
* key, so the deprecated and non-deprecated spellings of the same multi-valued option (for instance
* {@code --exclude} and {@code --input-exclude}) accumulate their values together instead of one silently
* discarding the other's (RAT-552), while {@link #args()} still names each option of a group only once, as
* commons-cli requires. Any single-valued option is stored under its own key, as passed in.
* @param key the key as passed to {@link #setArg(String, String)} or {@link #addArg(String, String[])}.
* @return the key under which the value(s) are to be stored in {@link #args}.
*/
private String resolveStorageKey(String key) {
Arg arg = Arg.findArg(key);
if (arg != null) {
Option opt = arg.find(key);
if (opt.hasArgs()) {
String canonicalKey = argsKey(arg.option());
for (Option o : arg.group().getOptions()) {
String otherKey = argsKey(o);
if (!otherKey.equals(key)) {
List<String> otherValues = args.remove(otherKey);
if (otherValues != null) {
args.computeIfAbsent(canonicalKey, k -> new ArrayList<>()).addAll(otherValues);
return canonicalKey;
}
}
}
}
}
return key;
}

/**
* Set a key and value into the argument list.
* Replaces any existing value.
Expand All @@ -73,7 +114,7 @@
DefaultLog.getInstance().debug(String.format("Setting %s to '%s'", key, value));
}
values.add(value);
args.put(key, values);
args.put(resolveStorageKey(key), values);
}
}
}
Expand Down Expand Up @@ -101,7 +142,7 @@
if (DefaultLog.getInstance().isEnabled(Log.Level.DEBUG)) {
DefaultLog.getInstance().debug(String.format("Adding [%s] to %s", String.join(", ", Arrays.asList(value)), key));
}
List<String> values = args.computeIfAbsent(key, k -> new ArrayList<>());
List<String> values = args.computeIfAbsent(resolveStorageKey(key), k -> new ArrayList<>());
values.addAll(newValues);
}
}
Expand All @@ -116,13 +157,14 @@
protected void addArg(String key, String value) {
if (StringUtils.isNotBlank(value)) {
if (validateSet(key)) {
List<String> values = args.get(key);
String storageKey = resolveStorageKey(key);
List<String> values = args.get(storageKey);
if (DefaultLog.getInstance().isEnabled(Log.Level.DEBUG)) {
DefaultLog.getInstance().debug(String.format("Adding [%s] to %s", String.join(", ", Arrays.asList(value)), key));
}
if (values == null) {
values = new ArrayList<>();
args.put(key, values);
args.put(storageKey, values);
}
values.add(value);
}
Expand Down
3 changes: 3 additions & 0 deletions src/changes/changes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ in order to be properly linked in site reports.
</release>
-->
<release version="1.0.0-SNAPSHOT" date="xxxx-yy-zz" description="Current SNAPSHOT - release to be done">
<action issue="RAT-552" type="add" dev="pottlinger" due-to="Arturo Bernal">
Fix using deprecated exclusion configuration and excludeSubProjects configuration with RAT Maven plugin.
</action>
<action issue="RAT-585" type="fix" dev="pottlinger" due-to="Konrad Windszus">
Respect filesystem case sensitivity when matching path roots, preventing lowercase Windows drive letters from being treated as relative report paths.
</action>
Expand Down
Loading