directories, final String directory) {
- final Path path = Path.of(directory);
-
- if (Files.isDirectory(path)) {
- directories.add(path);
- }
- }
}
diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolver.java b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolver.java
index a37b843..7516109 100644
--- a/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolver.java
+++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolver.java
@@ -1,22 +1,38 @@
package io.github.finoid.maven.plugins.codequality.archunit;
+import com.tngtech.archunit.core.domain.JavaConstructor;
+import com.tngtech.archunit.core.domain.JavaField;
+import com.tngtech.archunit.core.domain.JavaMember;
import com.tngtech.archunit.core.domain.SourceCodeLocation;
+import com.tngtech.archunit.core.domain.properties.HasSourceCodeLocation;
import org.apache.maven.project.MavenProject;
import org.jspecify.annotations.Nullable;
import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
+import java.util.Optional;
+import java.util.regex.Pattern;
/**
- * Maps an ArchUnit source location back onto the source file it came from.
- *
- * ArchUnit reads its locations from the bytecode, which carries the simple file name and the line number but not the
- * path, so the path is rebuilt from the package of the owning class and looked up under the source roots of the
- * module. A class whose file cannot be found - typically generated code, which has no source root - is reported
- * against the module directory rather than dropped, so the violation is still visible.
+ * Maps an ArchUnit source location back onto the source file and line it came from.
+ *
+ *
ArchUnit reads its locations from the bytecode, which carries the simple file name but not the path, so the path
+ * is rebuilt from the package of the owning class and looked up under the source roots of the module. A class whose
+ * file cannot be found - typically generated code, which has no source root - is reported against the module
+ * directory rather than dropped, so the violation is still visible.
+ *
+ *
Line numbers need the same treatment for a different reason. The bytecode records the line of the first
+ * statement of a method, and nothing at all for a class or a field, whereas a violation reads far better - and a
+ * suppression comment can only sensibly be written - against the declaration. The declaration is therefore located in
+ * the source, by scanning it. That is a heuristic, and a deliberate one: the class file simply does not record where
+ * a declaration was, and parsing the source to find out would buy an accuracy nobody would notice at a cost nobody
+ * wants. Every step falls back to what the bytecode said, so a scan which finds nothing never makes the location
+ * worse than it would have been.
*/
public final class SourceFileResolver {
private final List sourceRoots;
@@ -58,16 +74,101 @@ public File moduleDirectory() {
}
/**
- * The line to report, being the line of the location, or the first line when the bytecode carried none.
- *
- * A location without a line number is normal for a violation reported against a class rather than a member.
- * Reporting line zero renders badly in the GitLab code quality widget, hence the fallback.
+ * The line to report for the given element.
*
- * @param location the ArchUnit source location
+ * @param element the element the violation was reported against, a class or a member
+ * @param location the ArchUnit source location of that element
+ * @param sourceFile the file the location was resolved to
* @return the line number, never below one
*/
- public int lineNumber(final SourceCodeLocation location) {
- return Math.max(location.getLineNumber(), 1);
+ public int lineNumber(final HasSourceCodeLocation element, final SourceCodeLocation location, final File sourceFile) {
+ final int reported = Math.max(location.getLineNumber(), 1);
+ final List lines = linesOf(sourceFile);
+
+ if (lines.isEmpty()) {
+ return reported;
+ }
+
+ if (element instanceof JavaMember member) {
+ return declarationLineOf(member, location, lines).orElse(reported);
+ }
+
+ return declarationLine(typeDeclaration(location.getSourceClass().getSimpleName()), lines, 0).orElse(reported);
+ }
+
+ /**
+ * The line a member is declared on.
+ *
+ * Searched backwards from the line the bytecode reported, being the first statement of the body, so the
+ * nearest preceding declaration wins. Anything further up the file - an earlier overload, an unrelated call - is
+ * therefore never reached. A field carries no line at all, so its whole file is searched instead.
+ */
+ private static Optional declarationLineOf(final JavaMember member, final SourceCodeLocation location,
+ final List lines) {
+ if (member instanceof JavaField field) {
+ return declarationLine(fieldDeclaration(field.getName()), lines, 0);
+ }
+
+ final String name = member instanceof JavaConstructor
+ ? member.getOwner().getSimpleName()
+ : member.getName();
+
+ return declarationLine(callableDeclaration(name), lines, Math.max(location.getLineNumber(), 1));
+ }
+
+ /**
+ * The first line matching the given declaration.
+ *
+ * @param declaration the pattern identifying the declaration
+ * @param lines the lines of the source file
+ * @param upperBound the line to search backwards from, or zero to search the whole file forwards
+ * @return the line, or empty when the source holds no such declaration
+ */
+ private static Optional declarationLine(final Pattern declaration, final List lines, final int upperBound) {
+ if (upperBound > 0) {
+ for (int line = Math.min(upperBound, lines.size()); line >= 1; line--) {
+ if (declaration.matcher(lines.get(line - 1)).find()) {
+ return Optional.of(line);
+ }
+ }
+
+ return Optional.empty();
+ }
+
+ for (int line = 1; line <= lines.size(); line++) {
+ if (declaration.matcher(lines.get(line - 1)).find()) {
+ return Optional.of(line);
+ }
+ }
+
+ return Optional.empty();
+ }
+
+ /** A method or constructor, excluding calls to it, which are preceded by a dot. */
+ private static Pattern callableDeclaration(final String name) {
+ return Pattern.compile("(? linesOf(final File sourceFile) {
+ if (!sourceFile.isFile()) {
+ return List.of();
+ }
+
+ try {
+ return Files.readAllLines(sourceFile.toPath(), StandardCharsets.UTF_8);
+ } catch (final IOException e) {
+ // The location is best effort; an unreadable source must not fail the analysis
+ return List.of();
+ }
}
private static String relativeSourcePath(final SourceCodeLocation location) {
diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/ArchUnitConfiguration.java b/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/ArchUnitConfiguration.java
index 9e88f96..61dba53 100644
--- a/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/ArchUnitConfiguration.java
+++ b/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/ArchUnitConfiguration.java
@@ -48,6 +48,21 @@ public class ArchUnitConfiguration implements Configuration {
@Parameter(property = "cq.archunit.serviceLoaderEnabled")
private boolean serviceLoaderEnabled = true;
+ /**
+ * Whether the module should be compiled when it has not been already.
+ *
+ * Off by default. In the ordinary binding the goal runs at {@code verify}, where the classes are long since
+ * built, and a code quality goal quietly compiling the module is a side effect nobody asked for - a compilation
+ * failure would surface as a code quality failure. Switch it on to invoke the goal directly, without putting a
+ * phase in front of it.
+ *
+ * The compilation is a separate one, into {@code target/archunit-classes}, so it neither overwrites nor satisfies
+ * the output of the build itself. It reproduces the release level and the annotation processors of the module,
+ * but not a bespoke compiler configuration, so what is analyzed may differ from what the build produces.
+ */
+ @Parameter(property = "cq.archunit.compileIfMissing")
+ private boolean compileIfMissing = false;
+
/**
* Whether the test classes of the module should be analyzed alongside its main classes.
*
diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/filter/SuppressionCommentFilter.java b/src/main/java/io/github/finoid/maven/plugins/codequality/filter/SuppressionCommentFilter.java
new file mode 100644
index 0000000..51d7901
--- /dev/null
+++ b/src/main/java/io/github/finoid/maven/plugins/codequality/filter/SuppressionCommentFilter.java
@@ -0,0 +1,159 @@
+package io.github.finoid.maven.plugins.codequality.filter;
+
+import io.github.finoid.maven.plugins.codequality.report.Violation;
+import org.jspecify.annotations.Nullable;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Drops violations a suppression comment in the source has opted out of.
+ *
+ * A comment naming the rule, on the line the violation is reported against or on the line above it, suppresses that
+ * one rule at that one place:
+ *
+ * // suppress:NullAway the framework guarantees this is set before the first call
+ * private String name;
+ *
+ * private String name; // suppress:NullAway same, on the line itself
+ *
+ *
+ * The rule has to be named, and named exactly as the report does. A bare {@code // suppress} is deliberately not
+ * honoured: it would silently swallow the next, unrelated finding on the same line, which is the failure mode a
+ * suppression mechanism most needs to avoid.
+ *
+ * The filter works off the reported position rather than off the source language, so it applies to every analyzer -
+ * Checkstyle, Error Prone, the Checker Framework and ArchUnit alike. ArchUnit is the reason it cannot be done the
+ * other way around: it reads bytecode, which carries no comments, so a rule can never see the suppression itself.
+ */
+@Named("suppression-comment")
+@Singleton
+public class SuppressionCommentFilter implements ViolationFilter {
+ public static final String NAME = "SUPPRESSION_COMMENT";
+
+ /**
+ * Captures every rule named on a line, so several suppressions can share one comment.
+ */
+ private static final Pattern SUPPRESSION = Pattern.compile("suppress:(\\S+)");
+
+ @Override
+ public Violations filter(final Violations violations, final Context context) {
+ final SourceLines sourceLines = new SourceLines(context);
+
+ final List permissive = new ArrayList<>();
+ final List nonPermissive = new ArrayList<>();
+ final List suppressed = new ArrayList<>();
+
+ for (final Violation violation : violations.getPermissiveViolations()) {
+ (isSuppressed(violation, sourceLines) ? suppressed : permissive).add(violation);
+ }
+
+ for (final Violation violation : violations.getNonPermissiveViolations()) {
+ (isSuppressed(violation, sourceLines) ? suppressed : nonPermissive).add(violation);
+ }
+
+ report(suppressed, context);
+
+ return new Violations(permissive, nonPermissive);
+ }
+
+ @Override
+ public String name() {
+ return NAME;
+ }
+
+ /**
+ * A suppression is never silent: how many were honoured is logged unconditionally, so a build which reports
+ * nothing can still be told apart from one whose findings were all opted out of.
+ */
+ private static void report(final List suppressed, final Context context) {
+ if (suppressed.isEmpty()) {
+ return;
+ }
+
+ context.getLog()
+ .info(String.format("Suppressed %d violation(s) by comment", suppressed.size()));
+
+ suppressed.forEach(violation -> context.getLog()
+ .debug(String.format("Suppressed [%s] at %s:%d", violation.getRule(), violation.getRelativePath(), violation.getLine())));
+ }
+
+ private static boolean isSuppressed(final Violation violation, final SourceLines sourceLines) {
+ final int line = violation.getLine() == null ? 0 : violation.getLine();
+
+ // The line the violation sits on, and the line above it, which is where an own line comment goes
+ return namesRule(sourceLines.at(violation.getFullPath(), line), violation.getRule())
+ || namesRule(sourceLines.at(violation.getFullPath(), line - 1), violation.getRule());
+ }
+
+ private static boolean namesRule(final @Nullable String line, final @Nullable String rule) {
+ if (line == null || rule == null) {
+ return false;
+ }
+
+ final Matcher matcher = SUPPRESSION.matcher(line);
+
+ while (matcher.find()) {
+ if (rule.equals(matcher.group(1))) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ /**
+ * Reads the source files once each, since a file tends to carry more than one violation.
+ */
+ private static final class SourceLines {
+ private final Map> linesByPath = new HashMap<>();
+ private final Context context;
+
+ private SourceLines(final Context context) {
+ this.context = context;
+ }
+
+ @Nullable
+ private String at(final @Nullable String path, final int line) {
+ if (path == null || line < 1) {
+ return null;
+ }
+
+ final List lines = linesOf(path);
+
+ return line > lines.size() ? null : lines.get(line - 1);
+ }
+
+ private List linesOf(final String path) {
+ return linesByPath.computeIfAbsent(path, this::readLines);
+ }
+
+ private List readLines(final String path) {
+ final Path file = Path.of(path);
+
+ if (!Files.isRegularFile(file)) {
+ return List.of();
+ }
+
+ try {
+ return Files.readAllLines(file, StandardCharsets.UTF_8);
+ } catch (final IOException e) {
+ // An unreadable source is not a reason to fail the build; the violation simply stays unsuppressed
+ context.getLog()
+ .warn(String.format("Could not read [%s] to look for suppression comments. Cause: %s", path, e.getMessage()));
+
+ return List.of();
+ }
+ }
+ }
+}
diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/step/ArchUnitStep.java b/src/main/java/io/github/finoid/maven/plugins/codequality/step/ArchUnitStep.java
index 4f295c6..e36a77b 100644
--- a/src/main/java/io/github/finoid/maven/plugins/codequality/step/ArchUnitStep.java
+++ b/src/main/java/io/github/finoid/maven/plugins/codequality/step/ArchUnitStep.java
@@ -1,6 +1,7 @@
package io.github.finoid.maven.plugins.codequality.step;
import io.github.finoid.maven.plugins.codequality.ExecutionContext;
+import io.github.finoid.maven.plugins.codequality.MavenAnnotationProcessorsManager;
import io.github.finoid.maven.plugins.codequality.archunit.ArchRuleResolver;
import io.github.finoid.maven.plugins.codequality.archunit.ArchUnitAnalyzer;
import io.github.finoid.maven.plugins.codequality.archunit.NamedArchRule;
@@ -9,18 +10,37 @@
import io.github.finoid.maven.plugins.codequality.configuration.CodeQualityConfiguration;
import io.github.finoid.maven.plugins.codequality.exceptions.CodeQualityException;
import io.github.finoid.maven.plugins.codequality.report.Violation;
+import io.github.finoid.maven.plugins.codequality.util.CollectorUtils;
+import io.github.finoid.maven.plugins.codequality.util.MojoUtils.ElementUtils;
+import io.github.finoid.maven.plugins.codequality.util.MojoUtils.PluginUtils;
import io.github.finoid.maven.plugins.codequality.util.Precondition;
+import io.github.finoid.maven.plugins.codequality.util.PropertyUtils;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.BuildPluginManager;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.plugin.descriptor.PluginDescriptor;
import org.apache.maven.project.MavenProject;
+import org.twdata.maven.mojoexecutor.MojoExecutor;
import javax.inject.Inject;
import javax.inject.Singleton;
+import java.io.File;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.net.URL;
import java.net.URLClassLoader;
+import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
+import static org.twdata.maven.mojoexecutor.MojoExecutor.configuration;
+import static org.twdata.maven.mojoexecutor.MojoExecutor.element;
+import static org.twdata.maven.mojoexecutor.MojoExecutor.executeMojo;
+import static org.twdata.maven.mojoexecutor.MojoExecutor.executionEnvironment;
+import static org.twdata.maven.mojoexecutor.MojoExecutor.goal;
+
/**
* Step which evaluates ArchUnit rules against the compiled classes of the module.
*
@@ -34,16 +54,25 @@
*/
@Singleton
public class ArchUnitStep implements Step {
+ private static final String ARCH_UNIT_CLASSES = "archunit-classes";
+
private final ArchRuleResolver archRuleResolver;
private final ArchUnitAnalyzer archUnitAnalyzer;
private final TestClassPathResolver testClassPathResolver;
+ private final CodeQualityConfiguration codeQualityConfiguration;
+ private final MavenSession mavenSession;
+ private final BuildPluginManager pluginManager;
@Inject
public ArchUnitStep(final ArchRuleResolver archRuleResolver, final ArchUnitAnalyzer archUnitAnalyzer,
- final TestClassPathResolver testClassPathResolver) {
+ final TestClassPathResolver testClassPathResolver, final CodeQualityConfiguration codeQualityConfiguration,
+ final MavenSession mavenSession, final BuildPluginManager pluginManager) {
this.archRuleResolver = Precondition.nonNull(archRuleResolver, "ArchRuleResolver shouldn't be null");
this.archUnitAnalyzer = Precondition.nonNull(archUnitAnalyzer, "ArchUnitAnalyzer shouldn't be null");
this.testClassPathResolver = Precondition.nonNull(testClassPathResolver, "TestClassPathResolver shouldn't be null");
+ this.codeQualityConfiguration = Precondition.nonNull(codeQualityConfiguration, "CodeQualityConfiguration shouldn't be null");
+ this.mavenSession = Precondition.nonNull(mavenSession, "MavenSession shouldn't be null");
+ this.pluginManager = Precondition.nonNull(pluginManager, "BuildPluginManager shouldn't be null");
}
@Override
@@ -57,6 +86,18 @@ public PrerequisiteResult hasPrerequisites(final ArchUnitConfiguration configura
return PrerequisiteResult.notOK("no rules are configured and the service loader is disabled");
}
+ /*
+ * Unlike the analyzers which fork a compiler of their own, this step reads the classes the build has already
+ * produced. Bound to a phase before compile - or invoked directly on the command line ahead of one - there is
+ * nothing to read, and every rule would pass for the wrong reason. Reported as a missing prerequisite rather
+ * than as an empty result, so a run which cannot find anything never looks like a clean one.
+ */
+ if (!configuration.isCompileIfMissing() && !Files.isDirectory(outputDirectoryOf(context.getProject()))) {
+ return PrerequisiteResult.notOK(
+ "the module has no compiled classes, the goal has to run at or after the compile phase."
+ + " Set archUnit.compileIfMissing to compile it instead");
+ }
+
return PrerequisiteResult.OK;
}
@@ -73,8 +114,12 @@ public StepResult execute(final CodeQualityConfiguration codeQualityConfiguratio
@Override
public CleanContext getCleanContext() {
- // Nothing is written between runs: the classes are re-imported and the rules re-evaluated on every execution.
- return CleanContext.DO_NOTHING;
+ /*
+ * Only the output of an own compilation is cleaned, and only that. The classes of the build itself are left
+ * alone, but a stale class of ours - from a source file since deleted or renamed - would otherwise be
+ * analyzed forever, which is the sort of finding nobody can explain.
+ */
+ return new CleanContext(CleanContext.CleanType.DIRECTORY, ARCH_UNIT_CLASSES, "**/*");
}
private List executeStep(final ArchUnitConfiguration configuration, final ExecutionContext context) {
@@ -93,7 +138,7 @@ private List executeStep(final ArchUnitConfiguration configuration, f
warnOnUnmatchedSeverities(configuration, rules, context);
- return archUnitAnalyzer.analyze(rules, configuration, context);
+ return archUnitAnalyzer.analyze(classDirectoriesOf(configuration, context), rules, configuration, context);
} catch (final IOException e) {
throw new CodeQualityException(String.format("Failed to close the ArchUnit class loader. Cause: %s", e.getMessage()), e);
}
@@ -119,6 +164,95 @@ private void warnOnUnmatchedSeverities(final ArchUnitConfiguration configuration
.warn(String.format("ArchUnit severity override [%s] matches no resolved rule. Known rules: %s", name, resolvedNames)));
}
+ /**
+ * The directories holding the classes to analyze.
+ *
+ * Ordinarily the output of the build. When the module has not been compiled and {@code compileIfMissing} is set,
+ * a compilation of its own is run first, into a directory of its own so that neither the output of the build is
+ * overwritten nor a later phase led to believe the module is already built.
+ */
+ private List classDirectoriesOf(final ArchUnitConfiguration configuration, final ExecutionContext context) {
+ final MavenProject project = context.getProject();
+
+ final List directories = new ArrayList<>();
+
+ if (Files.isDirectory(outputDirectoryOf(project))) {
+ directories.add(outputDirectoryOf(project));
+ } else if (configuration.isCompileIfMissing()) {
+ directories.add(compile(context));
+ }
+
+ if (configuration.isAnalyzeTestClasses() && Files.isDirectory(testOutputDirectoryOf(project))) {
+ directories.add(testOutputDirectoryOf(project));
+ }
+
+ return directories;
+ }
+
+ /**
+ * Compiles the main sources of the module into {@code target/archunit-classes}.
+ *
+ * The release level and the annotation processors of the module are reproduced, which covers the common case
+ * of a Lombok using service. A module with a bespoke compiler configuration - additional compiler arguments,
+ * generated source roots, a module path - is not fully reproduced, so what is analyzed can differ from what the
+ * build itself produces. Running the goal after the compile phase avoids the question entirely.
+ */
+ private Path compile(final ExecutionContext context) {
+ final MavenProject project = context.getProject();
+
+ final PluginDescriptor descriptor = PluginUtils.pluginDescriptor("org.apache.maven.plugins", "maven-compiler-plugin",
+ codeQualityConfiguration.getVersions().getMavenCompiler());
+
+ final String javaVersion = PropertyUtils.valueOrFallback(project.getProperties(), "java.version", "21");
+ final Path outputDirectory = Path.of(project.getBuild().getDirectory(), ARCH_UNIT_CLASSES);
+
+ // The forked compile assigns an artifact file to the project, which would later be reported as
+ // 'The packaging for this project did not assign a file to the build artifact.'
+ final File originalArtifactFile = project.getArtifact()
+ .getFile();
+
+ context.getLog()
+ .info(String.format("Compiling %s for ArchUnit, no compiled classes were found", project.getArtifactId()));
+
+ try {
+ executeMojo(
+ PluginUtils.pluginOfDescriptor(descriptor),
+ goal("compile"),
+ configuration(
+ element(MojoExecutor.name("source"), javaVersion),
+ element(MojoExecutor.name("target"), javaVersion),
+ element(MojoExecutor.name("release"), javaVersion),
+ element(MojoExecutor.name("outputDirectory"), outputDirectory.toString()),
+ element(MojoExecutor.name("annotationProcessorPaths"), annotationProcessorPathsOf(project)
+ .toArray(MojoExecutor.Element[]::new))
+ ),
+ executionEnvironment(project, mavenSession, pluginManager));
+
+ return outputDirectory;
+ } catch (final MojoExecutionException e) {
+ throw new CodeQualityException(
+ String.format("Failed to compile module [%s] for ArchUnit. Cause: %s", project.getArtifactId(), e.getMessage()), e);
+ } finally {
+ project.getArtifact()
+ .setFile(originalArtifactFile);
+ }
+ }
+
+ private List annotationProcessorPathsOf(final MavenProject project) {
+ return new MavenAnnotationProcessorsManager(project, codeQualityConfiguration).annotationPaths()
+ .stream()
+ .map(path -> ElementUtils.annotationProcessor(path.getGroupId(), path.getArtifactId(), path.getVersion()))
+ .collect(CollectorUtils.toMutableList());
+ }
+
+ private static Path outputDirectoryOf(final MavenProject project) {
+ return Path.of(project.getBuild().getOutputDirectory());
+ }
+
+ private static Path testOutputDirectoryOf(final MavenProject project) {
+ return Path.of(project.getBuild().getTestOutputDirectory());
+ }
+
/**
* A class loader over the test classpath of the module, delegating to the class loader of this plugin.
*
diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzerUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzerUnitTest.java
index 3a89e83..281f973 100644
--- a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzerUnitTest.java
+++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzerUnitTest.java
@@ -68,7 +68,7 @@ void reportsViolationsWithTheirSourceLocation() {
.haveSimpleName("SomethingElse")
.allowEmptyShould(true);
- final List violations = unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration(), context);
+ final List violations = unit.analyze(classDirectories(), List.of(NamedArchRule.of("NAMING", rule)), configuration(), context);
Assertions.assertEquals(1, violations.size());
@@ -92,7 +92,7 @@ void reportsNothingForASatisfiedRule() {
.haveSimpleName("ArchUnitAnalyzer")
.allowEmptyShould(true);
- Assertions.assertTrue(unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration(), context).isEmpty());
+ Assertions.assertTrue(unit.analyze(classDirectories(), List.of(NamedArchRule.of("NAMING", rule)), configuration(), context).isEmpty());
}
@Test
@@ -109,7 +109,7 @@ void appliesThePerRuleSeverity() {
.haveSimpleName("SomethingElse")
.allowEmptyShould(true);
- final List violations = unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration, context);
+ final List violations = unit.analyze(classDirectories(), List.of(NamedArchRule.of("NAMING", rule)), configuration, context);
Assertions.assertEquals(Severity.BLOCKER, violations.getFirst().getSeverity());
}
@@ -124,8 +124,8 @@ void producesAStableFingerprint() {
.haveSimpleName("SomethingElse")
.allowEmptyShould(true);
- final List first = unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration(), context);
- final List second = unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration(), context);
+ final List first = unit.analyze(classDirectories(), List.of(NamedArchRule.of("NAMING", rule)), configuration(), context);
+ final List second = unit.analyze(classDirectories(), List.of(NamedArchRule.of("NAMING", rule)), configuration(), context);
Assertions.assertEquals(first.getFirst().getFingerprint(), second.getFirst().getFingerprint());
}
@@ -142,7 +142,11 @@ void skipsAModuleWithoutCompiledClasses() {
final ExecutionContext emptyContext = ExecutionContext.of(emptyProject, log);
- Assertions.assertTrue(unit.analyze(List.of(), configuration(), emptyContext).isEmpty());
+ Assertions.assertTrue(unit.analyze(List.of(), List.of(), configuration(), emptyContext).isEmpty());
+ }
+
+ private static List classDirectories() {
+ return List.of(WORKING_DIRECTORY.resolve("target/classes"));
}
private static ArchUnitConfiguration configuration() {
diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitStepUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitStepUnitTest.java
new file mode 100644
index 0000000..b7a5f10
--- /dev/null
+++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitStepUnitTest.java
@@ -0,0 +1,114 @@
+package io.github.finoid.maven.plugins.codequality.archunit;
+
+import io.github.finoid.maven.plugins.codequality.ExecutionContext;
+import io.github.finoid.maven.plugins.codequality.configuration.ArchUnitConfiguration;
+import io.github.finoid.maven.plugins.codequality.fixtures.UnitTest;
+import io.github.finoid.maven.plugins.codequality.step.ArchUnitStep;
+import io.github.finoid.maven.plugins.codequality.step.Step;
+import io.github.finoid.maven.plugins.codequality.configuration.CodeQualityConfiguration;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Build;
+import org.apache.maven.plugin.BuildPluginManager;
+import org.apache.maven.plugin.logging.Log;
+import org.apache.maven.project.MavenProject;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Mock;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Set;
+
+class ArchUnitStepUnitTest extends UnitTest {
+ @Mock
+ private ArchRuleResolver archRuleResolver;
+ @Mock
+ private ArchUnitAnalyzer archUnitAnalyzer;
+ @Mock
+ private TestClassPathResolver testClassPathResolver;
+ @Mock
+ private MavenSession mavenSession;
+ @Mock
+ private BuildPluginManager pluginManager;
+ @Mock
+ private Log log;
+
+ @TempDir
+ private Path temporaryDirectory;
+
+ private ArchUnitStep unit;
+
+ @BeforeEach
+ void beforeEach() {
+ unit = new ArchUnitStep(archRuleResolver, archUnitAnalyzer, testClassPathResolver,
+ new CodeQualityConfiguration(), mavenSession, pluginManager);
+ }
+
+ @Test
+ @DisplayName("Given a module which has not been compiled, Then the step reports a missing prerequisite")
+ void reportsMissingPrerequisiteWithoutCompiledClasses() {
+ final ArchUnitConfiguration configuration = new ArchUnitConfiguration();
+
+ final Step.PrerequisiteResult result =
+ unit.hasPrerequisites(configuration, context(temporaryDirectory.resolve("never-compiled")));
+
+ Assertions.assertFalse(result.hasAllPrerequisites());
+ Assertions.assertNotNull(result.cause());
+ Assertions.assertTrue(result.cause().contains("compile"), result.cause());
+ }
+
+ @Test
+ @DisplayName("Given compiled classes, Then the prerequisites are met")
+ void acceptsACompiledModule() throws IOException {
+ final Path classes = Files.createDirectory(temporaryDirectory.resolve("classes"));
+
+ Assertions.assertTrue(unit.hasPrerequisites(new ArchUnitConfiguration(), context(classes)).hasAllPrerequisites());
+ }
+
+ @Test
+ @DisplayName("Given an uncompiled module and compileIfMissing, Then the prerequisites are met")
+ void acceptsAnUncompiledModuleWhenAllowedToCompileIt() {
+ final ArchUnitConfiguration configuration = new ArchUnitConfiguration();
+ configuration.setCompileIfMissing(true);
+
+ Assertions.assertTrue(
+ unit.hasPrerequisites(configuration, context(temporaryDirectory.resolve("never-compiled"))).hasAllPrerequisites());
+ }
+
+ @Test
+ @DisplayName("Given an uncompiled module, Then the cause names the option which would compile it")
+ void namesTheCompileOptionInTheCause() {
+ final Step.PrerequisiteResult result =
+ unit.hasPrerequisites(new ArchUnitConfiguration(), context(temporaryDirectory.resolve("never-compiled")));
+
+ Assertions.assertTrue(result.cause().contains("compileIfMissing"), result.cause());
+ }
+
+ @Test
+ @DisplayName("Given no rules and no service loader, Then the step reports a missing prerequisite")
+ void reportsMissingPrerequisiteWithoutRules() throws IOException {
+ final Path classes = Files.createDirectory(temporaryDirectory.resolve("classes"));
+
+ final ArchUnitConfiguration configuration = new ArchUnitConfiguration();
+ configuration.setServiceLoaderEnabled(false);
+ configuration.setRules(Set.of());
+
+ final Step.PrerequisiteResult result = unit.hasPrerequisites(configuration, context(classes));
+
+ Assertions.assertFalse(result.hasAllPrerequisites());
+ }
+
+ private ExecutionContext context(final Path outputDirectory) {
+ final MavenProject project = new MavenProject();
+
+ final Build build = new Build();
+ build.setOutputDirectory(outputDirectory.toString());
+ project.setBuild(build);
+
+ return ExecutionContext.of(project, log);
+ }
+}
diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolverUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolverUnitTest.java
new file mode 100644
index 0000000..40cfe1b
--- /dev/null
+++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolverUnitTest.java
@@ -0,0 +1,119 @@
+package io.github.finoid.maven.plugins.codequality.archunit;
+
+import com.tngtech.archunit.core.domain.JavaClass;
+import com.tngtech.archunit.core.domain.JavaClasses;
+import com.tngtech.archunit.core.domain.JavaField;
+import com.tngtech.archunit.core.domain.JavaMethod;
+import com.tngtech.archunit.core.importer.ClassFileImporter;
+import io.github.finoid.maven.plugins.codequality.fixtures.UnitTest;
+import org.apache.maven.project.MavenProject;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+/**
+ * Resolved against the compiled classes and the sources of this very module, which is the only place a unit test can
+ * get real bytecode and the source it came from to agree with each other.
+ */
+class SourceFileResolverUnitTest extends UnitTest {
+ private static final Path WORKING_DIRECTORY = Paths.get("")
+ .toAbsolutePath();
+
+ private static final JavaClasses CLASSES = new ClassFileImporter().importClasses(SourceFileResolver.class, NamedArchRule.class);
+
+ private SourceFileResolver unit;
+
+ @BeforeEach
+ void beforeEach() {
+ unit = new SourceFileResolver(project(), false);
+ }
+
+ @Test
+ @DisplayName("Given a method, When resolving its line, Then the declaration is reported rather than the first statement")
+ void resolvesAMethodToItsDeclaration() {
+ final JavaClass javaClass = CLASSES.get(SourceFileResolver.class);
+ final JavaMethod method = javaClass.getMethod("moduleDirectory");
+
+ final File file = unit.resolve(method.getSourceCodeLocation());
+ final int line = unit.lineNumber(method, method.getSourceCodeLocation(), file);
+
+ Assertions.assertTrue(sourceLineOf(file, line).contains("File moduleDirectory()"),
+ () -> "Expected the declaration, got line " + line + ": " + sourceLineOf(file, line));
+ Assertions.assertTrue(line < method.getSourceCodeLocation().getLineNumber(),
+ "The declaration is expected to precede the first statement the bytecode reports");
+ }
+
+ @Test
+ @DisplayName("Given an overloaded method, When resolving its line, Then the nearest preceding declaration is used")
+ void resolvesTheRightOverload() {
+ final JavaClass javaClass = CLASSES.get(SourceFileResolver.class);
+ final JavaMethod method = javaClass.getMethod("resolve", com.tngtech.archunit.core.domain.SourceCodeLocation.class);
+
+ final File file = unit.resolve(method.getSourceCodeLocation());
+ final int line = unit.lineNumber(method, method.getSourceCodeLocation(), file);
+
+ Assertions.assertTrue(sourceLineOf(file, line).contains("File resolve("),
+ () -> "Expected the declaration, got line " + line + ": " + sourceLineOf(file, line));
+ }
+
+ @Test
+ @DisplayName("Given a class, When resolving its line, Then the type declaration is reported rather than line one")
+ void resolvesAClassToItsDeclaration() {
+ final JavaClass javaClass = CLASSES.get(SourceFileResolver.class);
+
+ final File file = unit.resolve(javaClass.getSourceCodeLocation());
+ final int line = unit.lineNumber(javaClass, javaClass.getSourceCodeLocation(), file);
+
+ Assertions.assertTrue(sourceLineOf(file, line).contains("class SourceFileResolver"),
+ () -> "Expected the declaration, got line " + line + ": " + sourceLineOf(file, line));
+ }
+
+ @Test
+ @DisplayName("Given a field, When resolving its line, Then the field declaration is reported")
+ void resolvesAFieldToItsDeclaration() {
+ final JavaClass javaClass = CLASSES.get(SourceFileResolver.class);
+ final JavaField field = javaClass.getField("sourceRoots");
+
+ final File file = unit.resolve(field.getSourceCodeLocation());
+ final int line = unit.lineNumber(field, field.getSourceCodeLocation(), file);
+
+ Assertions.assertTrue(sourceLineOf(file, line).contains("sourceRoots;"),
+ () -> "Expected the declaration, got line " + line + ": " + sourceLineOf(file, line));
+ }
+
+ @Test
+ @DisplayName("Given a source which cannot be read, When resolving, Then the reported line is kept")
+ void fallsBackToTheReportedLine() {
+ final JavaClass javaClass = CLASSES.get(NamedArchRule.class);
+ final JavaMethod method = javaClass.getMethod("name");
+
+ final int reported = Math.max(method.getSourceCodeLocation().getLineNumber(), 1);
+ final int line = unit.lineNumber(method, method.getSourceCodeLocation(), new File("does-not-exist.java"));
+
+ Assertions.assertEquals(reported, line);
+ }
+
+ private static String sourceLineOf(final File file, final int line) {
+ try {
+ return java.nio.file.Files.readAllLines(file.toPath()).get(line - 1);
+ } catch (final Exception e) {
+ return "";
+ }
+ }
+
+ private static MavenProject project() {
+ final MavenProject project = new MavenProject();
+
+ project.setFile(WORKING_DIRECTORY.resolve("pom.xml").toFile());
+ project.getCompileSourceRoots()
+ .clear();
+ project.addCompileSourceRoot(WORKING_DIRECTORY.resolve("src/main/java").toString());
+
+ return project;
+ }
+}
diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/filter/SuppressionCommentFilterUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/filter/SuppressionCommentFilterUnitTest.java
new file mode 100644
index 0000000..405f977
--- /dev/null
+++ b/src/test/java/io/github/finoid/maven/plugins/codequality/filter/SuppressionCommentFilterUnitTest.java
@@ -0,0 +1,167 @@
+package io.github.finoid.maven.plugins.codequality.filter;
+
+import io.github.finoid.maven.plugins.codequality.fixtures.UnitTest;
+import io.github.finoid.maven.plugins.codequality.report.Severity;
+import io.github.finoid.maven.plugins.codequality.report.Violation;
+import org.apache.maven.plugin.logging.Log;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Mock;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+class SuppressionCommentFilterUnitTest extends UnitTest {
+ private static final String RULE = "TransactionRules.NO_TRANSACTIONAL_METHOD_SHOULD_START_A_SECOND_TRANSACTION";
+ private static final String OTHER_RULE = "NullAway";
+
+ @Mock
+ private Log log;
+
+ @TempDir
+ private Path temporaryDirectory;
+
+ private SuppressionCommentFilter unit;
+
+ @BeforeEach
+ void beforeEach() {
+ unit = new SuppressionCommentFilter();
+ }
+
+ @Test
+ @DisplayName("Given a suppression on the line above, When filtered, Then the violation is dropped")
+ void suppressesFromTheLineAbove() throws IOException {
+ final Path source = source(
+ "package demo;",
+ "class A {",
+ " // suppress:" + RULE + " provisioning is already atomic",
+ " void a() {",
+ " }",
+ "}");
+
+ final Violations result = filter(violation(source, 4, RULE));
+
+ Assertions.assertEquals(0, result.total());
+ }
+
+ @Test
+ @DisplayName("Given a suppression on the reported line itself, When filtered, Then the violation is dropped")
+ void suppressesFromTheLineItself() throws IOException {
+ final Path source = source(
+ "package demo;",
+ "class A {",
+ " void a() { // suppress:" + RULE,
+ " }",
+ "}");
+
+ Assertions.assertEquals(0, filter(violation(source, 3, RULE)).total());
+ }
+
+ @Test
+ @DisplayName("Given a suppression naming another rule, When filtered, Then the violation is kept")
+ void keepsAViolationSuppressedUnderAnotherRule() throws IOException {
+ final Path source = source(
+ "package demo;",
+ "class A {",
+ " // suppress:" + OTHER_RULE,
+ " void a() {",
+ " }",
+ "}");
+
+ Assertions.assertEquals(1, filter(violation(source, 4, RULE)).total());
+ }
+
+ @Test
+ @DisplayName("Given a bare suppression without a rule, When filtered, Then the violation is kept")
+ void keepsAViolationUnderABareSuppression() throws IOException {
+ final Path source = source(
+ "package demo;",
+ "class A {",
+ " // suppress",
+ " void a() {",
+ " }",
+ "}");
+
+ Assertions.assertEquals(1, filter(violation(source, 4, RULE)).total());
+ }
+
+ @Test
+ @DisplayName("Given one comment naming two rules, When filtered, Then both violations are dropped")
+ void suppressesSeveralRulesFromOneComment() throws IOException {
+ final Path source = source(
+ "package demo;",
+ "class A {",
+ " // suppress:" + RULE + " suppress:" + OTHER_RULE,
+ " void a() {",
+ " }",
+ "}");
+
+ final Violations result = filter(violation(source, 4, RULE), violation(source, 4, OTHER_RULE));
+
+ Assertions.assertEquals(0, result.total());
+ }
+
+ @Test
+ @DisplayName("Given a suppression two lines above, When filtered, Then the violation is kept")
+ void doesNotReachBeyondTheLineAbove() throws IOException {
+ final Path source = source(
+ "package demo;",
+ " // suppress:" + RULE,
+ "class A {",
+ " void a() {",
+ " }",
+ "}");
+
+ Assertions.assertEquals(1, filter(violation(source, 4, RULE)).total());
+ }
+
+ @Test
+ @DisplayName("Given a non permissive violation, When suppressed, Then it no longer fails the build")
+ void suppressesNonPermissiveViolationsToo() throws IOException {
+ final Path source = source(
+ "package demo;",
+ "// suppress:" + RULE,
+ "class A {",
+ "}");
+
+ final Violations result =
+ unit.filter(new Violations(List.of(), List.of(violation(source, 3, RULE))), new ViolationFilter.Context(log));
+
+ Assertions.assertTrue(result.getNonPermissiveViolations().isEmpty());
+ }
+
+ @Test
+ @DisplayName("Given a violation whose source is missing, When filtered, Then it is kept rather than dropped")
+ void keepsAViolationWithoutAReadableSource() {
+ final Violation violation = violation(temporaryDirectory.resolve("Gone.java"), 3, RULE);
+
+ Assertions.assertEquals(1, filter(violation).total());
+ }
+
+ private Violations filter(final Violation... violations) {
+ return unit.filter(new Violations(List.of(violations), List.of()), new ViolationFilter.Context(log));
+ }
+
+ private Path source(final String... lines) throws IOException {
+ return Files.write(temporaryDirectory.resolve("A.java"), List.of(lines));
+ }
+
+ private static Violation violation(final Path source, final int line, final String rule) {
+ return Violation.builder()
+ .tool("ArchUnit")
+ .description("something")
+ .fingerprint("fingerprint")
+ .severity(Severity.MAJOR)
+ .relativePath("src/main/java/demo/A.java")
+ .fullPath(source.toString())
+ .line(line)
+ .columnNumber(0)
+ .rule(rule)
+ .build();
+ }
+}