From 324fc48c409de800aaed646c3b7e6e3da21ca513 Mon Sep 17 00:00:00 2001 From: "Erik C. Thauvin" Date: Wed, 28 Aug 2024 00:12:55 -0700 Subject: [PATCH] Cleaned up API to match bld operations and options APIs --- examples/lib/bld/bld-wrapper.properties | 4 +- .../rife/bld/extension/DetektOperation.java | 234 ++++++++++++++++-- .../bld/extension/DetektOperationTest.java | 165 ++++++++++++ 3 files changed, 374 insertions(+), 29 deletions(-) diff --git a/examples/lib/bld/bld-wrapper.properties b/examples/lib/bld/bld-wrapper.properties index 765309b..d6daa69 100644 --- a/examples/lib/bld/bld-wrapper.properties +++ b/examples/lib/bld/bld-wrapper.properties @@ -1,8 +1,8 @@ bld.downloadExtensionJavadoc=false bld.downloadExtensionSources=true bld.downloadLocation= -bld.extension-detekt=com.uwyn.rife2:bld-detekt:0.9.5 -bld.extension-kotlin=com.uwyn.rife2:bld-kotlin:1.0.0 +bld.extension-detekt=com.uwyn.rife2:bld-detekt:0.9.6-SNAPSHOT +bld.extension-kotlin=com.uwyn.rife2:bld-kotlin:1.0.1-SNAPSHOT bld.repositories=MAVEN_LOCAL,MAVEN_CENTRAL,RIFE2_SNAPSHOTS,RIFE2_RELEASES bld.sourceDirectories= bld.version=2.0.1 diff --git a/src/main/java/rife/bld/extension/DetektOperation.java b/src/main/java/rife/bld/extension/DetektOperation.java index 97aecc5..e0b6f21 100644 --- a/src/main/java/rife/bld/extension/DetektOperation.java +++ b/src/main/java/rife/bld/extension/DetektOperation.java @@ -24,8 +24,8 @@ import rife.bld.operations.exceptions.ExitStatusException; import java.io.File; import java.io.IOException; +import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.logging.Level; @@ -55,7 +55,7 @@ public class DetektOperation extends AbstractProcessOperation { "sarif4k-jvm-", "snakeyaml-engine-", "trove4j-"); - private static final Logger LOGGER = Logger.getLogger(Report.class.getName()); + private static final Logger LOGGER = Logger.getLogger(DetektOperation.class.getName()); private final Collection classpath_ = new ArrayList<>(); private final Collection config_ = new ArrayList<>(); private final Collection excludes_ = new ArrayList<>(); @@ -128,8 +128,28 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation basePath(File path) { - basePath_ = path.getAbsolutePath(); - return this; + return basePath(path.getAbsolutePath()); + } + + /** + * Retrieves the base path. + * + * @return the directory path + */ + public String basePath() { + return basePath_; + } + + /** + * Specifies a directory as the base path. Currently, it impacts all file + * paths in the formatted reports. File paths in console output and txt + * report are not affected and remain as absolute paths. + * + * @param path the directory path + * @return this operation instance + */ + public DetektOperation basePath(Path path) { + return basePath(path.toFile()); } /** @@ -152,8 +172,27 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation baseline(File baseline) { - baseline_ = baseline.getAbsolutePath(); - return this; + return baseline(baseline.getAbsolutePath()); + } + + /** + * If a baseline xml file is passed in, only new code smells not in the + * baseline are printed in the console. + * + * @param baseline the baseline xml file + * @return this operation instance + */ + public DetektOperation baseline(Path baseline) { + return baseline(baseline.toFile()); + } + + /** + * Retrieves the baseline xml file. + * + * @return the baseline xml file + */ + public String baseline() { + return baseline_; } /** @@ -177,8 +216,18 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation classPath(File... paths) { - classpath_.addAll(List.of(paths)); - return this; + return classPath(List.of(paths)); + } + + /** + * EXPERIMENTAL: Paths where to find user class files and jar dependencies. + * Used for type resolution. + * + * @param paths one or more files + * @return this operation instance + */ + public DetektOperation classPath(Path... paths) { + return classPathPaths(List.of(paths)); } /** @@ -189,8 +238,7 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation classPath(String... paths) { - classpath_.addAll(Arrays.stream(paths).map(File::new).toList()); - return this; + return classPathStrings(List.of(paths)); } @@ -216,13 +264,26 @@ public class DetektOperation extends AbstractProcessOperation { } /** - * Paths to the config files ({@code path/to/config.yml}). + * EXPERIMENTAL: Paths where to find user class files and jar dependencies. + * Used for type resolution. * - * @param configs one or more config files + * @param paths the paths * @return this operation instance */ - public DetektOperation config(File... configs) { - config_.addAll(List.of(configs)); + public DetektOperation classPathPaths(Collection paths) { + classpath_.addAll(paths.stream().map(Path::toFile).toList()); + return this; + } + + /** + * EXPERIMENTAL: Paths where to find user class files and jar dependencies. + * Used for type resolution. + * + * @param paths the paths + * @return this operation instance + */ + public DetektOperation classPathStrings(Collection paths) { + classpath_.addAll(paths.stream().map(File::new).toList()); return this; } @@ -232,11 +293,31 @@ public class DetektOperation extends AbstractProcessOperation { * @param configs one or more config files * @return this operation instance */ - public DetektOperation config(String... configs) { - config_.addAll(Arrays.stream(configs).map(File::new).toList()); - return this; + public DetektOperation config(File... configs) { + return config(List.of(configs)); } + /** + * Paths to the config files ({@code path/to/config.yml}). + * + * @param configs one or more config files + * @return this operation instance + */ + public DetektOperation config(Path... configs) { + return configPaths(List.of(configs)); + } + + /** + * Paths to the config files ({@code path/to/config.yml}). + * + * @param configs one or more config files + * @return this operation instance + */ + public DetektOperation config(String... configs) { + return configStrings(List.of(configs)); + } + + /** * Paths to the config files ({@code path/to/config.yml}). * @@ -257,6 +338,17 @@ public class DetektOperation extends AbstractProcessOperation { return config_; } + /** + * Paths to the config files ({@code path/to/config.yml}). + * + * @param configs the config files + * @return this operation instance + */ + public DetektOperation configPaths(Collection configs) { + config_.addAll(configs.stream().map(Path::toFile).toList()); + return this; + } + /** * Path to the config resource on detekt's classpath ({@code path/to/config.yml}). * @@ -275,7 +367,36 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation configResource(File resource) { - configResource_ = resource.getAbsolutePath(); + return configResource(resource.getAbsolutePath()); + } + + /** + * Path to the config resource on detekt's classpath ({@code path/to/config.yml}). + * + * @param resource the config resource path + * @return this operation instance + */ + public DetektOperation configResource(Path resource) { + return configResource(resource.toFile()); + } + + /** + * Retrieves the path of the config resource. + * + * @return the config resource path + */ + public String configResource() { + return configResource_; + } + + /** + * Paths to the config files ({@code path/to/config.yml}). + * + * @param configs the config files + * @return this operation instance + */ + public DetektOperation configStrings(Collection configs) { + config_.addAll(configs.stream().map(File::new).toList()); return this; } @@ -360,7 +481,7 @@ public class DetektOperation extends AbstractProcessOperation { throw new ExitStatusException(ExitStatusException.EXIT_FAILURE); } else { super.execute(); - if (successful_ && LOGGER.isLoggable(Level.INFO) && !silent()){ + if (successful_ && LOGGER.isLoggable(Level.INFO) && !silent()) { if (createBaseline_) { LOGGER.info("Detekt baseline generated successfully: " + "file://" + new File(baseline_).toURI().getPath()); @@ -627,8 +748,7 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation input(String... paths) { - input_.addAll(Arrays.stream(paths).map(File::new).toList()); - return this; + return inputStrings(List.of(paths)); } /** @@ -638,10 +758,18 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation input(File... paths) { - input_.addAll(List.of(paths)); - return this; + return input(List.of(paths)); } + /** + * Input paths to analyze. If not specified the current working directory is used. + * + * @param paths one or more paths + * @return this operation instance + */ + public DetektOperation input(Path... paths) { + return inputPaths(List.of(paths)); + } /** * Returns the input paths to analyze. @@ -652,6 +780,28 @@ public class DetektOperation extends AbstractProcessOperation { return input_; } + /** + * Input paths to analyze. If not specified the current working directory is used. + * + * @param paths the paths + * @return this operation instance + */ + public DetektOperation inputPaths(Collection paths) { + input_.addAll(paths.stream().map(Path::toFile).toList()); + return this; + } + + /** + * Input paths to analyze. If not specified the current working directory is used. + * + * @param paths the paths + * @return this operation instance + */ + public DetektOperation inputStrings(Collection paths) { + input_.addAll(paths.stream().map(File::new).toList()); + return this; + } + /* * Determines if a string is not blank. */ @@ -731,8 +881,7 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation plugins(String... jars) { - plugins_.addAll(Arrays.stream(jars).map(File::new).toList()); - return this; + return pluginsStrings(List.of(jars)); } /** @@ -742,8 +891,17 @@ public class DetektOperation extends AbstractProcessOperation { * @return this operation instance */ public DetektOperation plugins(File... jars) { - plugins_.addAll(List.of(jars)); - return this; + return plugins(List.of(jars)); + } + + /** + * Extra paths to plugin jars. + * + * @param jars one or more jars + * @return this operation instance + */ + public DetektOperation plugins(Path... jars) { + return pluginsPaths(List.of(jars)); } /** @@ -766,6 +924,28 @@ public class DetektOperation extends AbstractProcessOperation { return plugins_; } + /** + * Extra paths to plugin jars. + * + * @param jars the jars paths + * @return this operation instance + */ + public DetektOperation pluginsPaths(Collection jars) { + plugins_.addAll(jars.stream().map(Path::toFile).toList()); + return this; + } + + /** + * Extra paths to plugin jars. + * + * @param jars the jars paths + * @return this operation instance + */ + public DetektOperation pluginsStrings(Collection jars) { + plugins_.addAll(jars.stream().map(File::new).toList()); + return this; + } + /** * Generates a report for given {@link ReportId report-id} and stores it on given 'path'. * diff --git a/src/test/java/rife/bld/extension/DetektOperationTest.java b/src/test/java/rife/bld/extension/DetektOperationTest.java index c45b3fa..24e20ae 100644 --- a/src/test/java/rife/bld/extension/DetektOperationTest.java +++ b/src/test/java/rife/bld/extension/DetektOperationTest.java @@ -60,6 +60,36 @@ class DetektOperationTest { } } + @Test + void testBasePath() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().basePath(foo); + assertThat(op.basePath()).as("as file").isEqualTo(foo.getAbsolutePath()); + + op = op.basePath(bar.toPath()); + assertThat(op.basePath()).as("as path").isEqualTo(bar.getAbsolutePath()); + + op = new DetektOperation().basePath("foo"); + assertThat(op.basePath()).as("as string").isEqualTo("foo"); + } + + @Test + void testBaseline() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().baseline(foo); + assertThat(op.baseline()).as("as file").isEqualTo(foo.getAbsolutePath()); + + op = op.baseline(bar.toPath()); + assertThat(op.baseline()).as("as path").isEqualTo(bar.getAbsolutePath()); + + op = new DetektOperation().baseline("foo"); + assertThat(op.baseline()).as("as string").isEqualTo("foo"); + } + @Test @SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops") void testCheckAllParameters() throws IOException { @@ -129,6 +159,81 @@ class DetektOperationTest { } } + @Test + void testClassPath() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().classPath("foo", "bar"); + assertThat(op.classPath()).as("String...").contains(foo, bar); + op.classPath().clear(); + + op = op.classPath(foo, bar); + assertThat(op.classPath()).as("File...").contains(foo, bar); + op.classPath().clear(); + + op = op.classPath(foo.toPath(), bar.toPath()); + assertThat(op.classPath()).as("Path...").contains(foo, bar); + op.classPath().clear(); + + op = op.classPathStrings(List.of("foo", "bar")); + assertThat(op.classPath()).as("List(String...)").contains(foo, bar); + op.classPath().clear(); + + op = op.classPath(List.of(foo, bar)); + assertThat(op.classPath()).as("File...").contains(foo, bar); + op.classPath().clear(); + + op = op.classPathPaths(List.of(foo.toPath(), bar.toPath())); + assertThat(op.classPath()).as("Path...").contains(foo, bar); + op.classPath().clear(); + } + + @Test + void testConfig() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().config("foo", "bar"); + assertThat(op.config()).as("String...").contains(foo, bar); + op.config().clear(); + + op = op.config(foo, bar); + assertThat(op.config()).as("File...").contains(foo, bar); + op.config().clear(); + + op = op.config(foo.toPath(), bar.toPath()); + assertThat(op.config()).as("Path...").contains(foo, bar); + op.config().clear(); + + op = op.configStrings(List.of("foo", "bar")); + assertThat(op.config()).as("List(String...)").contains(foo, bar); + op.config().clear(); + + op = op.config(List.of(foo, bar)); + assertThat(op.config()).as("File...").contains(foo, bar); + op.config().clear(); + + op = op.configPaths(List.of(foo.toPath(), bar.toPath())); + assertThat(op.config()).as("Path...").contains(foo, bar); + op.config().clear(); + } + + @Test + void testConfigResource() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().configResource(foo); + assertThat(op.configResource()).as("as file").isEqualTo(foo.getAbsolutePath()); + + op = op.configResource(bar.toPath()); + assertThat(op.configResource()).as("as path").isEqualTo(bar.getAbsolutePath()); + + op = new DetektOperation().configResource("foo"); + assertThat(op.configResource()).as("as string").isEqualTo("foo"); + } + @Test void testExampleBaseline() throws IOException, ExitStatusException, InterruptedException { var tmpDir = Files.createTempDirectory("bld-detekt-").toFile(); @@ -197,4 +302,64 @@ class DetektOperationTest { var op = new DetektOperation(); assertThatCode(op::execute).isInstanceOf(ExitStatusException.class); } + + @Test + void testInput() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().input("foo", "bar"); + assertThat(op.input()).as("String...").contains(foo, bar); + op.input().clear(); + + op = op.input(foo, bar); + assertThat(op.input()).as("File...").contains(foo, bar); + op.input().clear(); + + op = op.input(foo.toPath(), bar.toPath()); + assertThat(op.input()).as("Path...").contains(foo, bar); + op.input().clear(); + + op = op.inputStrings(List.of("foo", "bar")); + assertThat(op.input()).as("List(String...)").contains(foo, bar); + op.input().clear(); + + op = op.input(List.of(foo, bar)); + assertThat(op.input()).as("File...").contains(foo, bar); + op.input().clear(); + + op = op.inputPaths(List.of(foo.toPath(), bar.toPath())); + assertThat(op.input()).as("Path...").contains(foo, bar); + op.input().clear(); + } + + @Test + void testPlugins() { + var foo = new File("foo"); + var bar = new File("bar"); + + var op = new DetektOperation().plugins("foo", "bar"); + assertThat(op.plugins()).as("String...").contains(foo, bar); + op.plugins().clear(); + + op = op.plugins(foo, bar); + assertThat(op.plugins()).as("File...").contains(foo, bar); + op.plugins().clear(); + + op = op.plugins(foo.toPath(), bar.toPath()); + assertThat(op.plugins()).as("Path...").contains(foo, bar); + op.plugins().clear(); + + op = op.pluginsStrings(List.of("foo", "bar")); + assertThat(op.plugins()).as("List(String...)").contains(foo, bar); + op.plugins().clear(); + + op = op.plugins(List.of(foo, bar)); + assertThat(op.plugins()).as("File...").contains(foo, bar); + op.plugins().clear(); + + op = op.pluginsPaths(List.of(foo.toPath(), bar.toPath())); + assertThat(op.plugins()).as("Path...").contains(foo, bar); + op.plugins().clear(); + } }