Skip to content

Commit

Permalink
RAT-98: Add file arg converter (#425)
Browse files Browse the repository at this point in the history
* added working directory

* updated merge issues

* Added FSInfo to handle file system differences

* updated spotbugs

* attempt to fix windows error

* fixed merge error

* fixed pattern match

* fix for file delete on windows

* added more descriptive failure messages

* added file converter + test

* fixes file list walker

* Prevent possible NPE

* Minor review changes

---------

Co-authored-by: P. Ottlinger <[email protected]>
  • Loading branch information
Claudenw and ottlinger authored Feb 4, 2025
1 parent ac0ebf3 commit 34dcb05
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 38 deletions.
38 changes: 22 additions & 16 deletions apache-rat-core/src/main/java/org/apache/rat/commandline/Arg.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,14 @@ public enum Arg {
CONFIGURATION(new OptionGroup()
.addOption(Option.builder().longOpt("config").hasArgs().argName("File")
.desc("File names for system configuration.")
.converter(Converters.FILE_CONVERTER)
.type(File.class)
.build())
.addOption(Option.builder().longOpt("licenses").hasArgs().argName("File")
.desc("File names for system configuration.")
.deprecated(DeprecatedAttributes.builder().setSince("0.17").setForRemoval(true).setDescription(StdMsgs.useMsg("--config")).get())
.converter(Converters.FILE_CONVERTER)
.type(File.class)
.build())),

/**
Expand Down Expand Up @@ -186,7 +190,6 @@ public enum Arg {
.hasArg().argName("File").type(File.class)
.converter(Converters.FILE_CONVERTER)
.desc("Name of file containing the denied license IDs.")
.converter(Converters.FILE_CONVERTER)
.build())),

/**
Expand Down Expand Up @@ -234,6 +237,7 @@ public enum Arg {
"File names must use linux directory separator ('/') or none at all. " +
"File names that do not start with '/' are relative to the directory where the " +
"argument is located.")
.converter(Converters.FILE_CONVERTER)
.type(File.class)
.build())),

Expand Down Expand Up @@ -585,8 +589,9 @@ private static void processConfigurationArgs(final ArgumentContext context) thro
try {
Defaults.Builder defaultBuilder = Defaults.builder();
if (CONFIGURATION.isSelected()) {
for (String fn : context.getCommandLine().getOptionValues(CONFIGURATION.getSelected())) {
defaultBuilder.add(fn);
File[] files = CONFIGURATION.getParsedOptionValues(context.getCommandLine());
for (File file : files) {
defaultBuilder.add(file);
}
}
if (CONFIGURATION_NO_DEFAULTS.isSelected()) {
Expand All @@ -607,7 +612,7 @@ private static void processConfigurationArgs(final ArgumentContext context) thro
try (InputStream in = Files.newInputStream(f.toPath())) {
context.getConfiguration().addApprovedLicenseCategories(IOUtils.readLines(in, StandardCharsets.UTF_8));
}
} catch (IOException | ParseException e) {
} catch (IOException e) {
throw new ConfigurationException(e);
}
}
Expand All @@ -622,7 +627,7 @@ private static void processConfigurationArgs(final ArgumentContext context) thro
try (InputStream in = Files.newInputStream(f.toPath())) {
context.getConfiguration().removeApprovedLicenseCategories(IOUtils.readLines(in, StandardCharsets.UTF_8));
}
} catch (IOException | ParseException e) {
} catch (IOException e) {
throw new ConfigurationException(e);
}
}
Expand All @@ -634,11 +639,11 @@ private static void processConfigurationArgs(final ArgumentContext context) thro
}
if (LICENSES_APPROVED_FILE.isSelected()) {
try {
File f = context.getCommandLine().getParsedOptionValue(LICENSES_APPROVED_FILE.getSelected());
try (InputStream in = Files.newInputStream(f.toPath())) {
File file = context.getCommandLine().getParsedOptionValue(LICENSES_APPROVED_FILE.getSelected());
try (InputStream in = Files.newInputStream(file.toPath())) {
context.getConfiguration().addApprovedLicenseIds(IOUtils.readLines(in, StandardCharsets.UTF_8));
}
} catch (IOException | ParseException e) {
} catch (IOException e) {
throw new ConfigurationException(e);
}
}
Expand All @@ -649,11 +654,11 @@ private static void processConfigurationArgs(final ArgumentContext context) thro
}
if (LICENSES_DENIED_FILE.isSelected()) {
try {
File f = context.getCommandLine().getParsedOptionValue(LICENSES_DENIED_FILE.getSelected());
try (InputStream in = Files.newInputStream(f.toPath())) {
File file = context.getCommandLine().getParsedOptionValue(LICENSES_DENIED_FILE.getSelected());
try (InputStream in = Files.newInputStream(file.toPath())) {
context.getConfiguration().removeApprovedLicenseIds(IOUtils.readLines(in, StandardCharsets.UTF_8));
}
} catch (IOException | ParseException e) {
} catch (IOException e) {
throw new ConfigurationException(e);
}
}
Expand Down Expand Up @@ -795,6 +800,7 @@ public static void processLogLevel(final CommandLine commandLine) {
* @throws ConfigurationException on error
*/
public static void processArgs(final ArgumentContext context) throws ConfigurationException {
Converters.FILE_CONVERTER.setWorkingDirectory(context.getWorkingDirectory());
processOutputArgs(context);
processEditArgs(context);
processInputArgs(context);
Expand Down Expand Up @@ -843,12 +849,12 @@ private static void processOutputArgs(final ArgumentContext context) {

if (OUTPUT_FILE.isSelected()) {
try {
File f = context.getCommandLine().getParsedOptionValue(OUTPUT_FILE.getSelected());
File parent = f.getParentFile();
File file = context.getCommandLine().getParsedOptionValue(OUTPUT_FILE.getSelected());
File parent = file.getParentFile();
if (!parent.mkdirs() && !parent.isDirectory()) {
DefaultLog.getInstance().error("Could not create report parent directory " + f);
DefaultLog.getInstance().error("Could not create report parent directory " + file);
}
context.getConfiguration().setOut(f);
context.getConfiguration().setOut(file);
} catch (ParseException e) {
context.logParseException(e, OUTPUT_FILE.getSelected(), "System.out");
context.getConfiguration().setOut((IOSupplier<OutputStream>) null);
Expand All @@ -857,7 +863,7 @@ private static void processOutputArgs(final ArgumentContext context) {

if (OUTPUT_STYLE.isSelected()) {
String selected = OUTPUT_STYLE.getSelected().getKey();
if (selected.equals("x")) {
if ("x".equals(selected)) {
// display deprecated message.
context.getCommandLine().hasOption("x");
context.getConfiguration().setStyleSheet(StyleSheets.getStyleSheet("xml"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@
package org.apache.rat.commandline;

import java.io.File;
import java.io.IOException;

import org.apache.commons.cli.Converter;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.rat.ConfigurationException;
import org.apache.rat.document.DocumentName;
import org.apache.rat.report.claim.ClaimStatistic;

import static java.lang.String.format;

/**
* Customized converters for Arg processing
* Customized converters for Arg processing.
*/
public final class Converters {

Expand All @@ -37,11 +39,12 @@ private Converters() {
}

/**
* Creates a File with fully qualified name
* Creates a File with fully qualified name.
*/
public static final Converter<File, NullPointerException> FILE_CONVERTER = s -> new File(s).getAbsoluteFile();
public static final FileConverter FILE_CONVERTER = new FileConverter();

/**
* converts the Converter pattern into a Converter, count pair.
* Converts the Converter pattern into a Converter, count pair.
*/
public static final Converter<Pair<ClaimStatistic.Counter, Integer>, ConfigurationException> COUNTER_CONVERTER = arg -> {
String[] parts = arg.split(":");
Expand All @@ -55,4 +58,50 @@ private Converters() {
throw new ConfigurationException(format("'%s' is not a valid Counter", parts[0]), e);
}
};

/**
* A converter that can handle relative or absolute files.
*/
public static final class FileConverter implements Converter<File, NullPointerException> {
/** The working directory to resolve relative files against. */
private DocumentName workingDirectory;

/**
* The constructor.
*/
private FileConverter() {
// private construction only.
}

/**
* Sets the working directory for the conversion.
* @param workingDirectory current working directory
*/
public void setWorkingDirectory(final DocumentName workingDirectory) {
this.workingDirectory = workingDirectory;
}

/**
* Applies the conversion function to the specified file name.
* @param fileName the file name to create a file from.
* @return a File.
* @throws NullPointerException if {@code fileName} is null.
*/
public File apply(final String fileName) throws NullPointerException {
File file = new File(fileName);
// is this a relative file?
if (!fileName.startsWith(File.separator)) {
// check for a root provided (e.g. C:\\)"
if (!DocumentName.FSInfo.getDefault().rootFor(fileName).isPresent()) {
// no root, resolve against workingDirectory
file = new File(workingDirectory.resolve(fileName).getName()).getAbsoluteFile();
}
}
try {
return file.getCanonicalFile();
} catch (IOException e) {
return file.getAbsoluteFile();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ public boolean startsWith(final String string) {

@Override
public String toString() {
return Arrays.asList(tokenized)
.toString();
return Arrays.asList(tokenized).toString();
}

public String source() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ private MatchPatterns(final MatchPattern[] patterns) {

@Override
public String toString() {
return Arrays.stream(patterns)
.map(MatchPattern::toString)
.collect(Collectors.toList())
.toString();
return Arrays.stream(patterns).map(MatchPattern::toString).collect(Collectors.toList()).toString();
}

public String source() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,21 @@
*/
package org.apache.rat.commandline;

import java.io.IOException;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
import org.apache.rat.DeprecationReporter;
import org.apache.rat.OptionCollection;
import org.apache.rat.ReportConfiguration;
import org.apache.rat.document.DocumentName;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.io.File;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.assertj.core.api.Assertions.assertThat;

public class ArgTests {

Expand All @@ -40,23 +42,23 @@ private CommandLine createCommandLine(String[] args) throws ParseException {
.setAllowPartialMatching(true).build().parse(opts, args);
}

@ParameterizedTest
@ParameterizedTest(name = "{0}")
@ValueSource(strings = { "rat.txt", "./rat.txt", "/rat.txt", "target/rat.test" })
public void outputFleNameNoDirectoryTest(String name) throws ParseException {
public void outputFleNameNoDirectoryTest(String name) throws ParseException, IOException {
class OutputFileConfig extends ReportConfiguration {
private File actual = null;
@Override
public void setOut(File file) {
actual = file;
}
}
String fileName = name.replace("/", DocumentName.FSInfo.getDefault().dirSeparator());
File expected = new File(fileName);

File expected = new File(name);

CommandLine commandLine = createCommandLine(new String[] {"--output-file", name});
CommandLine commandLine = createCommandLine(new String[] {"--output-file", fileName});
OutputFileConfig configuration = new OutputFileConfig();
ArgumentContext ctxt = new ArgumentContext(new File("."), configuration, commandLine);
Arg.processArgs(ctxt);
assertEquals(expected.getAbsolutePath(), configuration.actual.getAbsolutePath());
assertThat(configuration.actual.getAbsolutePath()).isEqualTo(expected.getCanonicalPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,21 @@ public void setAddDefaultLicenses(final boolean addDefaultLicenses) {

/**
* Whether to add the default list of license matchers.
* @deprecated @deprecated Use specific configuration under &lt;configuration&gt;.
* @deprecated Use specific configuration under &lt;configuration&gt;.
*/
@Deprecated
@Parameter(property = "rat.addDefaultLicenseMatchers")
private boolean addDefaultLicenseMatchers;

/** The list of approved licenses
* @deprecated @deprecated Use specific configuration under &lt;configuration&gt;.
* @deprecated Use specific configuration under &lt;configuration&gt;.
*/
@Deprecated
@Parameter(required = false)
@Parameter
private String[] approvedLicenses;

/** The file of approved licenses
* @deprecated @deprecated Use specific configuration under &lt;configuration&gt;.
* @deprecated Use specific configuration under &lt;configuration&gt;.
*/
@Deprecated
@Parameter(property = "rat.approvedFile")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public AntOptionsProvider() {
super(BaseAntTask.unsupportedArgs(), testPath.toFile());
}

protected ReportConfiguration generateConfig(List<Pair<Option, String[]>> args) {
protected final ReportConfiguration generateConfig(List<Pair<Option, String[]>> args) {
BuildTask task = args.get(0).getKey() == null ? new BuildTask() : new BuildTask(args.get(0).getKey());
task.setUp(args);
task.buildRule.executeTarget(task.name);
Expand Down

0 comments on commit 34dcb05

Please sign in to comment.