diff --git a/README.md b/README.md index 9ee1ebc..581c032 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,13 @@ For continuous use across builds, include the plugin in your project’s pom.xml true + + + true + + com.example.arch.MyRules#NO_CYCLES + + @@ -166,3 +173,119 @@ For continuous use across builds, include the plugin in your project’s pom.xml | `checkers` | The list of checkers to be run. | See `CheckerFrameworkConfiguration` class in your codebase. | | `compilerArgs` | Custom compiler arguments. | `[]` | | `versions.checkerFramework` | The Checker Framework version to use. | `3.48.1` | + +### ArchUnit configuration + +Evaluates [ArchUnit](https://www.archunit.org/) rules against the compiled classes of the module and reports the +findings alongside the other analyzers, with the source file and line the violation belongs to. + +Unlike the other analyzers the checks are not built in: the rules come from the project. Running them here rather than +as `@ArchTest` JUnit tests means they also run when the build skips tests, and that their findings reach the GitLab +code quality report. A project which keeps its ArchUnit tests should be aware the rules are then evaluated twice, once +by surefire and once here. + +| Parameter | Description | Default | +|------------------------|-------------------------------------------------------------------------|---------| +| `enabled` | Whether the ArchUnit analyzer should be enabled. | `false` | +| `permissive` | Whether the execution should be permissive (not fail on violations). | `true` | +| `rules` | Explicit rule references, see below. | `[]` | +| `serviceLoaderEnabled` | Whether rule providers should be discovered from the test classpath. | `true` | +| `analyzeTestClasses` | Whether the test classes should be analyzed alongside the main classes. | `false` | +| `severity` | The severity reported for a rule without an entry in `ruleSeverities`. | `MAJOR` | +| `ruleSeverities` | Severity per rule name, overriding `severity`. | `{}` | + +#### Referencing rules explicitly + +Three forms are accepted. The referenced classes are loaded from the **test** classpath, so the rule library only has +to be a test scoped dependency: + +```xml + + true + + + com.example.arch.MyRules#NO_CYCLES + + com.example.arch.MyRules#noCycles() + + com.example.arch.MyRules + + + BLOCKER + + +``` + +A rule is reported under `SimpleClassName.member`, which is also the key `ruleSeverities` is looked up by. Neither `#` +nor the parentheses of a method reference are legal in an XML element name, hence the normalisation. An override +matching no resolved rule is warned about rather than silently ignored. + +#### Providing rules from a library + +A rule library can register itself instead, so consuming projects need no configuration beyond enabling the step. +Implement `ArchRuleProvider` and ship a service entry: + +```java +public class MyRuleProvider implements ArchRuleProvider { + @Override + public Collection rules() { + return List.of(NamedArchRule.of("NO_CYCLES", MyRules.NO_CYCLES)); + } +} +``` + +``` +META-INF/services/io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider +``` + +Provider names are chosen by the library, so keep them usable as XML element names if consumers should be able to +override their severity. + +#### Where the rules live + +Both sources load from a jar just as happily as from the module's own classes, so a shared rule library can be wired +in two ways. + +As a test scoped dependency of the analyzed module: + +```xml + + com.example + arch-rules + 1.0.0 + test + +``` + +Or as a dependency of the plugin declaration, which keeps it out of the project's own dependency tree entirely and +lets a parent POM hand the rules to every module that inherits it: + +```xml + + io.github.finoid + codequality-maven-plugin + + + com.example + arch-rules + 1.0.0 + + + +``` + +Explicit references and service loader discovery work through either. + +#### Dependency resolution scope + +The goal keeps resolving dependencies in **compile** scope. Widening it to test scope would resolve the test +dependencies of every module whether or not this step is enabled, and would fail the goal on a test dependency which +cannot be resolved. The step resolves the test classpath itself, through `ProjectDependenciesResolver`, and only when +it actually runs. + +#### Class loading + +Rules are loaded through a class loader over the test classpath of the module, delegating to the plugin's own class +loader. ArchUnit therefore always resolves to the copy the plugin was built against. Rules compiled against another +1.x release link fine against it, since the types they touch (`ArchRule`, `ArchCondition`, `DescribedPredicate`) are +stable across the line, but a project on a future 2.x release would need the plugin upgraded in step. diff --git a/pom.xml b/pom.xml index df2fb5d..76fd942 100644 --- a/pom.xml +++ b/pom.xml @@ -28,6 +28,7 @@ 21 UTF-8 + 1.4.1 0.3.2 14.1.0 4.2.3 @@ -197,6 +198,11 @@ + + com.tngtech.archunit + archunit + ${archunit.version} + de.vandermeer asciitable diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/CodeQuality.java b/src/main/java/io/github/finoid/maven/plugins/codequality/CodeQuality.java index 17b72f5..143fd9d 100644 --- a/src/main/java/io/github/finoid/maven/plugins/codequality/CodeQuality.java +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/CodeQuality.java @@ -10,6 +10,7 @@ import io.github.finoid.maven.plugins.codequality.handlers.CleanHandler; import io.github.finoid.maven.plugins.codequality.report.Severity; import io.github.finoid.maven.plugins.codequality.report.ViolationReporter; +import io.github.finoid.maven.plugins.codequality.step.ArchUnitStep; import io.github.finoid.maven.plugins.codequality.step.CheckerFrameworkStep; import io.github.finoid.maven.plugins.codequality.step.CheckstyleStep; import io.github.finoid.maven.plugins.codequality.step.ErrorProneStep; @@ -39,6 +40,7 @@ public class CodeQuality extends AbstractMojo { private final CheckstyleStep checkstyleStep; private final ErrorProneStep errorProneStep; private final CheckerFrameworkStep checkerFrameworkStep; + private final ArchUnitStep archUnitStep; private final CleanHandler cleanHandler; private final StepResultsRepository stepResultsRepository; private final ReactorCompletionTracker reactorCompletionTracker; @@ -69,6 +71,7 @@ public CodeQuality( final CheckstyleStep checkstyleStep, final ErrorProneStep errorProneStep, final CheckerFrameworkStep checkerFrameworkStep, + final ArchUnitStep archUnitStep, final CleanHandler cleanHandler, final MavenSession mavenSession, final MavenProject project, @@ -81,6 +84,7 @@ public CodeQuality( this.checkstyleStep = Precondition.nonNull(checkstyleStep, "CheckstyleStep shouldn't be null"); this.errorProneStep = Precondition.nonNull(errorProneStep, "ErrorProneStep shouldn't be null"); this.checkerFrameworkStep = Precondition.nonNull(checkerFrameworkStep, "CheckerFrameworkStep shouldn't be null"); + this.archUnitStep = Precondition.nonNull(archUnitStep, "ArchUnitStep shouldn't be null"); this.cleanHandler = Precondition.nonNull(cleanHandler, "CleanHandler shouldn't be null"); this.stepResultsRepository = Precondition.nonNull(stepResultsRepository, "StepResultsRepository shouldn't be null"); this.reactorCompletionTracker = Precondition.nonNull(reactorCompletionTracker, "ReactorCompletionTracker shouldn't be null"); @@ -126,7 +130,8 @@ private ProjectStepResults executeSteps(final ExecutionContext context) { context.getProject().getName(), executeStep(checkstyleStep, codeQualityConfiguration, codeQualityConfiguration.getCheckstyle(), context), executeStep(errorProneStep, codeQualityConfiguration, codeQualityConfiguration.getErrorProne(), context), - executeStep(checkerFrameworkStep, codeQualityConfiguration, codeQualityConfiguration.getCheckerFramework(), context) + executeStep(checkerFrameworkStep, codeQualityConfiguration, codeQualityConfiguration.getCheckerFramework(), context), + executeStep(archUnitStep, codeQualityConfiguration, codeQualityConfiguration.getArchUnit(), context) ); stepResultsRepository.store(context.getProject(), projectStepResults); diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleProvider.java b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleProvider.java new file mode 100644 index 0000000..85bbaf9 --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleProvider.java @@ -0,0 +1,23 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import java.util.Collection; + +/** + * Supplies the ArchUnit rules a library wants applied to the projects consuming it. + *

+ * Implementations are discovered through {@link java.util.ServiceLoader} from the test classpath of the analyzed + * module, so a rule library declares itself by shipping a + * {@code META-INF/services/io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider} entry. Implementations + * need a public no-args constructor. + *

+ * A library which does not want a dependency on this plugin does not have to implement anything: rules can be + * referenced directly from the plugin configuration instead, see {@code archUnit.rules}. + */ +public interface ArchRuleProvider { + /** + * The rules to apply. + * + * @return the rules, never null + */ + Collection rules(); +} diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleResolver.java b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleResolver.java new file mode 100644 index 0000000..1aa48ae --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleResolver.java @@ -0,0 +1,191 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import com.tngtech.archunit.lang.ArchRule; +import io.github.finoid.maven.plugins.codequality.ExecutionContext; +import io.github.finoid.maven.plugins.codequality.configuration.ArchUnitConfiguration; +import io.github.finoid.maven.plugins.codequality.exceptions.CodeQualityException; + +import javax.inject.Singleton; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.ServiceConfigurationError; +import java.util.ServiceLoader; + +/** + * Resolves the rules to evaluate, from the plugin configuration and from the service loader. + *

+ * Both sources are read through a class loader over the test classpath of the analyzed module, so a rule library only + * has to be a test scoped dependency of the project. That class loader delegates to the class loader of this plugin, + * which means ArchUnit itself is always the copy this plugin was built against, even when the project depends on a + * different version. Rules compiled against another 1.x release link fine against it, since the types they touch + * ({@code ArchRule}, {@code ArchCondition}, {@code DescribedPredicate}) are stable across the line, but a project on a + * 2.x release would need this plugin to be upgraded in step. + */ +@Singleton +public class ArchRuleResolver { + private static final String MEMBER_SEPARATOR = "#"; + private static final String METHOD_SUFFIX = "()"; + + /** + * Resolves every configured and discovered rule. + *

+ * Duplicates by name are collapsed, so a rule both provided through the service loader and referenced explicitly + * is evaluated once. + * + * @param configuration the step configuration + * @param classLoader the class loader over the test classpath of the analyzed module + * @param context the context of the current mojo execution + * @return the rules to evaluate, in a stable order + */ + public List resolve(final ArchUnitConfiguration configuration, final ClassLoader classLoader, + final ExecutionContext context) { + final Map byName = new LinkedHashMap<>(); + + if (configuration.isServiceLoaderEnabled()) { + fromServiceLoader(classLoader, context).forEach(rule -> byName.putIfAbsent(rule.name(), rule)); + } + + for (final String reference : configuration.getRules()) { + fromReference(reference, classLoader).forEach(rule -> byName.putIfAbsent(rule.name(), rule)); + } + + return new ArrayList<>(byName.values()); + } + + private List fromServiceLoader(final ClassLoader classLoader, final ExecutionContext context) { + final List rules = new ArrayList<>(); + + try { + for (final ArchRuleProvider provider : ServiceLoader.load(ArchRuleProvider.class, classLoader)) { + rules.addAll(provider.rules()); + } + } catch (final ServiceConfigurationError e) { + // A broken provider on the classpath must not take the build down; the explicitly configured rules are + // still worth evaluating. + context.getLog() + .warn(String.format("Failed to load an ArchRuleProvider. Cause: %s", e.getMessage())); + } + + return rules; + } + + private List fromReference(final String reference, final ClassLoader classLoader) { + final int separator = reference.indexOf(MEMBER_SEPARATOR); + + if (separator < 0) { + return fromClass(reference, classLoader); + } + + final String className = reference.substring(0, separator); + final String memberName = reference.substring(separator + 1); + final Class owner = loadClass(className, classLoader, reference); + + if (memberName.endsWith(METHOD_SUFFIX)) { + final String methodName = memberName.substring(0, memberName.length() - METHOD_SUFFIX.length()); + + return List.of(fromMethod(owner, methodName, className, reference)); + } + + return List.of(fromField(owner, memberName, className, reference)); + } + + private List fromClass(final String className, final ClassLoader classLoader) { + final Class type = loadClass(className, classLoader, className); + + if (ArchRuleProvider.class.isAssignableFrom(type)) { + return new ArrayList<>(instantiateProvider(type, className).rules()); + } + + final List rules = new ArrayList<>(); + + for (final Field field : type.getDeclaredFields()) { + if (isStaticArchRule(field)) { + rules.add(fromField(type, field.getName(), className, className + MEMBER_SEPARATOR + field.getName())); + } + } + + if (rules.isEmpty()) { + throw new CodeQualityException(String.format( + "ArchUnit rule reference [%s] resolved to a class with neither an ArchRuleProvider implementation nor a" + + " static ArchRule field", className)); + } + + return rules; + } + + private NamedArchRule fromField(final Class owner, final String fieldName, final String className, final String reference) { + try { + final Field field = owner.getDeclaredField(fieldName); + + if (!isStaticArchRule(field)) { + throw new CodeQualityException(String.format( + "ArchUnit rule reference [%s] is not a static field of type ArchRule", reference)); + } + + field.setAccessible(true); + + return NamedArchRule.of(nameOf(className, fieldName), (ArchRule) field.get(null)); + } catch (final NoSuchFieldException | IllegalAccessException e) { + throw new CodeQualityException(String.format("Failed to read ArchUnit rule [%s]. Cause: %s", reference, e.getMessage()), e); + } + } + + private NamedArchRule fromMethod(final Class owner, final String methodName, final String className, final String reference) { + try { + final Method method = owner.getDeclaredMethod(methodName); + + if (!Modifier.isStatic(method.getModifiers()) || !ArchRule.class.isAssignableFrom(method.getReturnType())) { + throw new CodeQualityException(String.format( + "ArchUnit rule reference [%s] is not a static no-args method returning an ArchRule", reference)); + } + + method.setAccessible(true); + + return NamedArchRule.of(nameOf(className, methodName), (ArchRule) method.invoke(null)); + } catch (final ReflectiveOperationException e) { + throw new CodeQualityException(String.format("Failed to invoke ArchUnit rule [%s]. Cause: %s", reference, e.getMessage()), e); + } + } + + private ArchRuleProvider instantiateProvider(final Class type, final String reference) { + try { + return (ArchRuleProvider) type.getDeclaredConstructor() + .newInstance(); + } catch (final ReflectiveOperationException e) { + throw new CodeQualityException( + String.format("Failed to instantiate ArchRuleProvider [%s]. Cause: %s", reference, e.getMessage()), e); + } + } + + private Class loadClass(final String className, final ClassLoader classLoader, final String reference) { + try { + return Class.forName(className, true, classLoader); + } catch (final ClassNotFoundException e) { + throw new CodeQualityException(String.format( + "ArchUnit rule reference [%s] could not be loaded from the test classpath of the module", reference), e); + } + } + + private static boolean isStaticArchRule(final Field field) { + return Modifier.isStatic(field.getModifiers()) && ArchRule.class.isAssignableFrom(field.getType()); + } + + /** + * The reported name of a rule, being the referenced member qualified by the simple name of its class. + *

+ * Neither {@code #} nor the parentheses of a method reference survive as an XML element name, and the severity + * overrides are keyed by rule name in the plugin configuration, so the reference is normalised to + * {@code SimpleClassName.member} - which is both a legal element name and shorter to read in a report. + */ + private static String nameOf(final String className, final String memberName) { + final int lastDot = className.lastIndexOf('.'); + final String simpleName = lastDot < 0 ? className : className.substring(lastDot + 1); + + return simpleName + '.' + memberName; + } +} diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzer.java b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzer.java new file mode 100644 index 0000000..69e0544 --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzer.java @@ -0,0 +1,150 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.domain.SourceCodeLocation; +import com.tngtech.archunit.core.domain.properties.HasSourceCodeLocation; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.lang.EvaluationResult; +import com.tngtech.archunit.lang.ViolationHandler; +import io.github.finoid.maven.plugins.codequality.ExecutionContext; +import io.github.finoid.maven.plugins.codequality.configuration.ArchUnitConfiguration; +import io.github.finoid.maven.plugins.codequality.report.Violation; +import io.github.finoid.maven.plugins.codequality.step.ViolationConverter; +import io.github.finoid.maven.plugins.codequality.util.Precondition; +import org.apache.maven.project.MavenProject; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Optional; + +/** + * Evaluates ArchUnit rules against the compiled classes of a module and turns the results into violations. + *

+ * The classes are imported once and shared by every rule, since the import is by far the expensive part. + */ +@Singleton +public class ArchUnitAnalyzer { + private final ViolationConverter violationConverter; + + @Inject + public ArchUnitAnalyzer(final ViolationConverter violationConverter) { + this.violationConverter = Precondition.nonNull(violationConverter, "ViolationConverter shouldn't be null"); + } + + /** + * Evaluates the given rules. + * + * @param rules the rules to evaluate + * @param configuration the step configuration + * @param context the context of the current mojo execution + * @return the violations found, empty when there is nothing to analyze + */ + public List analyze(final List rules, final ArchUnitConfiguration configuration, + final ExecutionContext context) { + final List classDirectories = classDirectoriesOf(context.getProject(), configuration.isAnalyzeTestClasses()); + + if (classDirectories.isEmpty()) { + context.getLog() + .debug("No compiled classes to analyze with ArchUnit. Skipping..."); + + return List.of(); + } + + final JavaClasses javaClasses = new ClassFileImporter().importPaths(classDirectories); + final SourceFileResolver sourceFileResolver = + new SourceFileResolver(context.getProject(), configuration.isAnalyzeTestClasses()); + + final List violations = new ArrayList<>(); + + for (final NamedArchRule rule : rules) { + violations.addAll(evaluate(rule, javaClasses, configuration, sourceFileResolver)); + } + + return violations; + } + + private List evaluate(final NamedArchRule namedRule, final JavaClasses javaClasses, + final ArchUnitConfiguration configuration, final SourceFileResolver sourceFileResolver) { + final EvaluationResult result = namedRule.rule() + .evaluate(javaClasses); + + if (!result.hasViolation()) { + return List.of(); + } + + final List violations = new ArrayList<>(); + + /* + * An anonymous class rather than a lambda: ArchUnit derives the type it filters the corresponding objects by + * from the reified varargs array, which a lambda does not provide. Typed as Object so every corresponding + * object is handed over, whether it is a class, a member or an access. + */ + result.handleViolations(new ViolationHandler() { + @Override + public void handle(final Collection correspondingObjects, final String message) { + violations.add(toViolation(namedRule, correspondingObjects, message, configuration, sourceFileResolver)); + } + }); + + return violations; + } + + private Violation toViolation(final NamedArchRule namedRule, final Collection correspondingObjects, + final String message, final ArchUnitConfiguration configuration, + final SourceFileResolver sourceFileResolver) { + final Optional location = sourceCodeLocation(correspondingObjects); + + final File file = location.map(sourceFileResolver::resolve) + .orElseGet(sourceFileResolver::moduleDirectory); + final int line = location.map(sourceFileResolver::lineNumber) + .orElse(1); + + return violationConverter.ofArchUnitViolation(namedRule.name(), singleLine(message), file, line, + configuration.severityOf(namedRule.name())); + } + + private static Optional sourceCodeLocation(final Collection correspondingObjects) { + return correspondingObjects.stream() + .filter(HasSourceCodeLocation.class::isInstance) + .map(HasSourceCodeLocation.class::cast) + .map(HasSourceCodeLocation::getSourceCodeLocation) + .findFirst(); + } + + /** + * ArchUnit renders some violations over several lines. The report formats expect a single line, and the console + * table renderer would break its layout on an embedded newline. + */ + private static String singleLine(final String message) { + return message.replace("\r", " ") + .replace("\n", " ") + .replaceAll("\\s{2,}", " ") + .trim(); + } + + private static List classDirectoriesOf(final MavenProject project, final boolean includeTestClasses) { + final List directories = new ArrayList<>(); + + addIfDirectory(directories, project.getBuild().getOutputDirectory()); + + if (includeTestClasses) { + addIfDirectory(directories, project.getBuild().getTestOutputDirectory()); + } + + return directories; + } + + private static void addIfDirectory(final List 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/NamedArchRule.java b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/NamedArchRule.java new file mode 100644 index 0000000..907f80b --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/NamedArchRule.java @@ -0,0 +1,22 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import com.tngtech.archunit.lang.ArchRule; +import io.github.finoid.maven.plugins.codequality.util.Precondition; + +/** + * An {@link ArchRule} together with the stable identifier it is reported under. + *

+ * The identifier ends up as the rule name of the emitted violation and as part of its fingerprint, so it must be + * stable across builds. Prefer the name of the constant declaring the rule over the rule's own description, which + * tends to be a long sentence and changes whenever the wording is improved. + */ +public record NamedArchRule(String name, ArchRule rule) { + public NamedArchRule { + Precondition.nonBlank(name, "Name shouldn't be blank"); + Precondition.nonNull(rule, "ArchRule shouldn't be null"); + } + + public static NamedArchRule of(final String name, final ArchRule rule) { + return new NamedArchRule(name, rule); + } +} 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 new file mode 100644 index 0000000..a37b843 --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/SourceFileResolver.java @@ -0,0 +1,106 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import com.tngtech.archunit.core.domain.SourceCodeLocation; +import org.apache.maven.project.MavenProject; +import org.jspecify.annotations.Nullable; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +/** + * 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. + */ +public final class SourceFileResolver { + private final List sourceRoots; + private final Path baseDirectory; + + public SourceFileResolver(final MavenProject project, final boolean includeTestSources) { + this.baseDirectory = project.getBasedir() + .toPath(); + this.sourceRoots = sourceRootsOf(project, includeTestSources); + } + + /** + * The absolute path of the source file the location points at. + * + * @param location the ArchUnit source location + * @return the resolved file, or the module directory when no source file matches + */ + public File resolve(final SourceCodeLocation location) { + final String relative = relativeSourcePath(location); + + for (final Path sourceRoot : sourceRoots) { + final Path candidate = sourceRoot.resolve(relative); + + if (Files.isRegularFile(candidate)) { + return candidate.toFile(); + } + } + + return baseDirectory.toFile(); + } + + /** + * The directory of the module, reported when a violation carries no source location at all. + * + * @return the module directory + */ + public File moduleDirectory() { + return baseDirectory.toFile(); + } + + /** + * 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. + * + * @param location the ArchUnit source location + * @return the line number, never below one + */ + public int lineNumber(final SourceCodeLocation location) { + return Math.max(location.getLineNumber(), 1); + } + + private static String relativeSourcePath(final SourceCodeLocation location) { + final String packageName = location.getSourceClass() + .getPackageName(); + + if (packageName.isEmpty()) { + return location.getSourceFileName(); + } + + return packageName.replace('.', File.separatorChar) + File.separatorChar + location.getSourceFileName(); + } + + private static List sourceRootsOf(final MavenProject project, final boolean includeTestSources) { + final List roots = new ArrayList<>(); + + addAll(roots, project.getCompileSourceRoots()); + + if (includeTestSources) { + addAll(roots, project.getTestCompileSourceRoots()); + } + + return roots; + } + + private static void addAll(final List roots, final @Nullable List sourceRoots) { + if (sourceRoots == null) { + return; + } + + sourceRoots.stream() + .map(Path::of) + .filter(Files::isDirectory) + .forEach(roots::add); + } +} diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/TestClassPathResolver.java b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/TestClassPathResolver.java new file mode 100644 index 0000000..9b30478 --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/archunit/TestClassPathResolver.java @@ -0,0 +1,105 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import io.github.finoid.maven.plugins.codequality.exceptions.CodeQualityException; +import io.github.finoid.maven.plugins.codequality.util.Precondition; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.project.DefaultDependencyResolutionRequest; +import org.apache.maven.project.DependencyResolutionException; +import org.apache.maven.project.DependencyResolutionResult; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectDependenciesResolver; +import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.util.artifact.JavaScopes; +import org.eclipse.aether.util.filter.DependencyFilterUtils; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; + +/** + * Resolves the test classpath of a module on demand. + *

+ * The goal declares {@code ResolutionScope.COMPILE}, so {@link MavenProject#getTestClasspathElements()} is not + * available: its artifacts carry no file. Widening the goal to {@code ResolutionScope.TEST} would resolve the test + * dependencies of every module whether or not the ArchUnit step is enabled, and would make every project pay for a + * step most do not use - including failing the goal on a test dependency which cannot be resolved. The test classpath + * is therefore resolved here, and only when the step actually runs. + *

+ * Only reactor wide state is read from the session, which makes the resolver safe to share between the modules of a + * parallel build. The Aether session it hands to the resolver is the one of the whole reactor. + */ +@Singleton +public class TestClassPathResolver { + private final ProjectDependenciesResolver dependenciesResolver; + private final MavenSession session; + + @Inject + public TestClassPathResolver(final ProjectDependenciesResolver dependenciesResolver, final MavenSession session) { + this.dependenciesResolver = Precondition.nonNull(dependenciesResolver, "ProjectDependenciesResolver shouldn't be null"); + this.session = Precondition.nonNull(session, "MavenSession shouldn't be null"); + } + + /** + * The test classpath of the module, being its own output directories followed by every dependency reachable in + * test scope. + * + * @param project the module to resolve the test classpath of + * @return the classpath entries, in classpath order + * @throws CodeQualityException in case the dependencies cannot be resolved + */ + public List resolve(final MavenProject project) { + final List classPath = new ArrayList<>(); + + addIfExists(classPath, project.getBuild().getOutputDirectory()); + addIfExists(classPath, project.getBuild().getTestOutputDirectory()); + + for (final Dependency dependency : resolveDependencies(project)) { + final File file = dependency.getArtifact() + .getFile(); + + if (file != null) { + classPath.add(toUrl(file)); + } + } + + return classPath; + } + + private List resolveDependencies(final MavenProject project) { + final DefaultDependencyResolutionRequest request = + new DefaultDependencyResolutionRequest(project, session.getRepositorySession()); + + request.setResolutionFilter(DependencyFilterUtils.classpathFilter(JavaScopes.TEST)); + + try { + final DependencyResolutionResult result = dependenciesResolver.resolve(request); + + return result.getResolvedDependencies(); + } catch (final DependencyResolutionException e) { + throw new CodeQualityException(String.format( + "Failed to resolve the test classpath of module [%s], which the ArchUnit step loads its rules from." + + " Cause: %s", project.getArtifactId(), e.getMessage()), e); + } + } + + private static void addIfExists(final List classPath, final String directory) { + final File file = new File(directory); + + if (file.isDirectory()) { + classPath.add(toUrl(file)); + } + } + + private static URL toUrl(final File file) { + try { + return file.toURI() + .toURL(); + } catch (final MalformedURLException e) { + throw new CodeQualityException(String.format("Failed to build a class path URL of [%s]. Cause: %s", file, e.getMessage()), e); + } + } +} 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 new file mode 100644 index 0000000..9e88f96 --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/ArchUnitConfiguration.java @@ -0,0 +1,91 @@ +package io.github.finoid.maven.plugins.codequality.configuration; + +import io.github.finoid.maven.plugins.codequality.report.Severity; +import lombok.Data; +import org.apache.maven.plugins.annotations.Parameter; + +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; + +@Data +public class ArchUnitConfiguration implements Configuration { + /** + * Whether the ArchUnit analyzer should be enabled or disabled. + */ + @Parameter(property = "cq.archunit.enabled") + private boolean enabled = false; + + /** + * Whether the execution should be permissive (allow violations without failing) or strict (fail on violations). + */ + @Parameter(property = "cq.archunit.permissive") + private boolean permissive = true; + + /** + * Explicit rule references, evaluated in addition to whatever the service loader discovers. + *

+ * Three forms are accepted: + *

    + *
  • {@code com.example.MyRules#MY_RULE} - a static field of type {@code ArchRule}
    • + *
  • {@code com.example.MyRules#myRule()} - a static no-args method returning an {@code ArchRule}
    • + *
  • {@code com.example.MyRules} - every public static {@code ArchRule} field of the class, or, when the + * class implements {@link io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider}, the rules + * it provides
    • + *
+ *

+ * The referenced classes are loaded from the test classpath of the analyzed module, so the rule library only + * needs to be a test scoped dependency. + */ + @Parameter(property = "cq.archunit.rules") + private Set rules = new LinkedHashSet<>(); + + /** + * Whether rule providers should be discovered through + * {@link java.util.ServiceLoader} from the test classpath of the analyzed module. + */ + @Parameter(property = "cq.archunit.serviceLoaderEnabled") + private boolean serviceLoaderEnabled = true; + + /** + * Whether the test classes of the module should be analyzed alongside its main classes. + *

+ * Off by default: rules describing production structure tend to report the test fixtures which deliberately + * violate them. + */ + @Parameter(property = "cq.archunit.analyzeTestClasses") + private boolean analyzeTestClasses = false; + + /** + * The severity reported for a rule without an explicit entry in {@link #ruleSeverities}. + *

+ * ArchUnit has no notion of severity of its own, so one has to be assigned here. + */ + @Parameter(property = "cq.archunit.severity") + private Severity severity = Severity.MAJOR; + + /** + * Severity per rule name, overriding {@link #severity}. + *

+ * Keyed by the name the rule is reported under, being the {@link + * io.github.finoid.maven.plugins.codequality.archunit.NamedArchRule#name()} of a provided rule or the referenced + * member for an explicitly configured one. + */ + @Parameter + private Map ruleSeverities = new HashMap<>(); + + public boolean isNotPermissive() { + return !permissive; + } + + /** + * The severity to report the given rule under. + * + * @param ruleName the name the rule is reported under + * @return the configured severity, or the step wide default + */ + public Severity severityOf(final String ruleName) { + return ruleSeverities.getOrDefault(ruleName, severity); + } +} diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/CodeQualityConfiguration.java b/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/CodeQualityConfiguration.java index 7e0ebf4..67ec1bc 100644 --- a/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/CodeQualityConfiguration.java +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/configuration/CodeQualityConfiguration.java @@ -37,6 +37,9 @@ public class CodeQualityConfiguration { @Parameter private CheckerFrameworkConfiguration checkerFramework = new CheckerFrameworkConfiguration(); + @Parameter + private ArchUnitConfiguration archUnit = new ArchUnitConfiguration(); + /** * List of annotation processor paths. */ 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 new file mode 100644 index 0000000..4f295c6 --- /dev/null +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/step/ArchUnitStep.java @@ -0,0 +1,137 @@ +package io.github.finoid.maven.plugins.codequality.step; + +import io.github.finoid.maven.plugins.codequality.ExecutionContext; +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; +import io.github.finoid.maven.plugins.codequality.archunit.TestClassPathResolver; +import io.github.finoid.maven.plugins.codequality.configuration.ArchUnitConfiguration; +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.Precondition; +import org.apache.maven.project.MavenProject; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Step which evaluates ArchUnit rules against the compiled classes of the module. + *

+ * Unlike the other analyzers, the checks this step runs are not built in: the rules come from the project, either + * referenced explicitly through {@code archUnit.rules} or discovered from the test classpath through the service + * loader. The step is therefore a no-op until a project configures at least one of the two. + *

+ * Running the rules here rather than as {@code @ArchTest} JUnit tests means they also run when the build skips tests, + * and that their findings land in the same report as the other analyzers. A project which keeps its ArchUnit tests + * should be aware the rules are then evaluated twice, once by surefire and once here. + */ +@Singleton +public class ArchUnitStep implements Step { + private final ArchRuleResolver archRuleResolver; + private final ArchUnitAnalyzer archUnitAnalyzer; + private final TestClassPathResolver testClassPathResolver; + + @Inject + public ArchUnitStep(final ArchRuleResolver archRuleResolver, final ArchUnitAnalyzer archUnitAnalyzer, + final TestClassPathResolver testClassPathResolver) { + 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"); + } + + @Override + public boolean isEnabled(final ArchUnitConfiguration configuration) { + return configuration.isEnabled(); + } + + @Override + public PrerequisiteResult hasPrerequisites(final ArchUnitConfiguration configuration, final ExecutionContext context) { + if (!configuration.isServiceLoaderEnabled() && configuration.getRules().isEmpty()) { + return PrerequisiteResult.notOK("no rules are configured and the service loader is disabled"); + } + + return PrerequisiteResult.OK; + } + + @Override + public StepType type() { + return StepType.ARCH_UNIT; + } + + @Override + public StepResult execute(final CodeQualityConfiguration codeQualityConfiguration, final ArchUnitConfiguration stepConfiguration, + final ExecutionContext context) { + return StepResult.create(StepType.ARCH_UNIT, stepConfiguration.isPermissive(), executeStep(stepConfiguration, context)); + } + + @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; + } + + private List executeStep(final ArchUnitConfiguration configuration, final ExecutionContext context) { + try (URLClassLoader classLoader = testClassLoaderOf(context.getProject())) { + final List rules = archRuleResolver.resolve(configuration, classLoader, context); + + if (rules.isEmpty()) { + context.getLog() + .info("No ArchUnit rules were resolved. Skipping..."); + + return List.of(); + } + + context.getLog() + .debug(String.format("Evaluating %d ArchUnit rule(s): %s", rules.size(), rules.stream().map(NamedArchRule::name).toList())); + + warnOnUnmatchedSeverities(configuration, rules, context); + + return archUnitAnalyzer.analyze(rules, configuration, context); + } catch (final IOException e) { + throw new CodeQualityException(String.format("Failed to close the ArchUnit class loader. Cause: %s", e.getMessage()), e); + } + } + + /** + * Warns about severity overrides which match no resolved rule. + *

+ * An override is keyed by rule name, and getting that name wrong is silent otherwise: the rule simply keeps the + * default severity, which looks exactly like the override having been applied to a rule that reports nothing. + */ + private void warnOnUnmatchedSeverities(final ArchUnitConfiguration configuration, final List rules, + final ExecutionContext context) { + final Set resolvedNames = rules.stream() + .map(NamedArchRule::name) + .collect(Collectors.toSet()); + + configuration.getRuleSeverities() + .keySet() + .stream() + .filter(name -> !resolvedNames.contains(name)) + .forEach(name -> context.getLog() + .warn(String.format("ArchUnit severity override [%s] matches no resolved rule. Known rules: %s", name, resolvedNames))); + } + + /** + * A class loader over the test classpath of the module, delegating to the class loader of this plugin. + *

+ * Parent first delegation is deliberate: ArchUnit must resolve to the copy the plugin was built against, so that + * the {@code ArchRule} instances the project hands back are of the type this plugin evaluates. A project holding + * a second copy on its own classpath would otherwise produce a {@code ClassCastException} which is hard to read. + *

+ * Delegating to the plugin's own class loader also means a rule library declared as a dependency of the plugin + * itself is found, without the analyzed project depending on it at all. + */ + private URLClassLoader testClassLoaderOf(final MavenProject project) { + final List classPath = testClassPathResolver.resolve(project); + + return new URLClassLoader(classPath.toArray(URL[]::new), getClass().getClassLoader()); + } +} diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/step/StepType.java b/src/main/java/io/github/finoid/maven/plugins/codequality/step/StepType.java index c4a5a0e..b28fb47 100644 --- a/src/main/java/io/github/finoid/maven/plugins/codequality/step/StepType.java +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/step/StepType.java @@ -3,5 +3,6 @@ public enum StepType { CHECKSTYLE, ERROR_PRONE, - CHECKER_FRAMEWORK + CHECKER_FRAMEWORK, + ARCH_UNIT } diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/step/ViolationConverter.java b/src/main/java/io/github/finoid/maven/plugins/codequality/step/ViolationConverter.java index 63854da..f376fd0 100644 --- a/src/main/java/io/github/finoid/maven/plugins/codequality/step/ViolationConverter.java +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/step/ViolationConverter.java @@ -77,6 +77,40 @@ public Violation ofErrorProneViolationMatcher(final Matcher violationMatcher) { .build(); } + /** + * Converts an ArchUnit violation into a {@link Violation}. + *

+ * Unlike the analyzers which report a position, the fingerprint deliberately leaves out the line number. An + * ArchUnit violation identifies a structural fact about a named element, and the element's message already + * contains its fully qualified name; keying on the line as well would make the whole report look new to GitLab + * whenever unrelated edits shift the code down a line. + * + * @param rule the name the rule is reported under + * @param description the violation message + * @param sourceFile the file the violation is reported against + * @param lineNumber the line the violation is reported against + * @param severity the severity configured for the rule + * @return the violation + */ + public Violation ofArchUnitViolation(final String rule, final String description, final File sourceFile, final int lineNumber, + final Severity severity) { + final File repositoryRoot = repositoryRoot(); + + final String absoluteFilePath = sourceFile.getAbsolutePath(); + + return Violation.builder() + .tool("ArchUnit") + .description(description) + .fingerprint(fingerprint(String.format("%s:%s:%s", relativePath(repositoryRoot, absoluteFilePath), rule, description))) + .severity(severity) + .relativePath(relativePath(repositoryRoot, absoluteFilePath)) + .fullPath(absoluteFilePath.replace("\\", "/")) // Windows compatibility + .line(lineNumber) + .columnNumber(0) + .rule(rule) + .build(); + } + public Violation ofCheckerFrameworkViolationMatcher(final Matcher violationMatcher) { final File repositoryRoot = repositoryRoot(); diff --git a/src/main/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtils.java b/src/main/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtils.java index 5fbfa03..a30cdcb 100644 --- a/src/main/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtils.java +++ b/src/main/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtils.java @@ -12,6 +12,7 @@ import java.io.File; import java.util.List; import java.util.Optional; +import java.util.Set; /** * Utility class for working with Maven projects, dependencies, and source directories. @@ -23,6 +24,12 @@ public final class ProjectUtils { */ public static final String PLUGIN_KEY = "io.github.finoid:codequality-maven-plugin"; + /** + * The scopes making up the compile classpath, being the classpath the analyzers compile the main sources against. + */ + private static final Set COMPILE_CLASS_PATH_SCOPES = + Set.of(Artifact.SCOPE_COMPILE, Artifact.SCOPE_PROVIDED, Artifact.SCOPE_SYSTEM); + /** * Resolves a list of files from the given source directories in the specified Maven project. * @@ -47,20 +54,29 @@ public static boolean isLombokPresentOnClassPath(final MavenProject project) { } /** - * Checks if a specific dependency is present on the classpath. + * Checks if a specific dependency is present on the compile classpath. + *

+ * Deliberately filtered by scope rather than answered from every resolved artifact. The analyzers compile the + * main sources against the compile classpath, so a test or runtime scoped Lombok or checker-qual must not look + * like it were available to them - the Checker Framework step would otherwise pass its prerequisite and then fail + * the forked compile. The goal resolves dependencies in compile scope today, which makes the filter a safeguard + * rather than a correction, and keeps the answer correct should the scope ever be widened. * * @param project the Maven project whose dependencies are checked. * @param groupId the group ID of the dependency. * @param artifactId the artifact ID of the dependency. - * @return {@code true} if the dependency is found on the classpath, {@code false} otherwise. + * @return {@code true} if the dependency is found on the compile classpath, {@code false} otherwise. */ public static boolean isPresentOnClassPath(final MavenProject project, final String groupId, final String artifactId) { return project.getArtifacts().stream() + .filter(ProjectUtils::isOnCompileClassPath) .anyMatch(it -> groupId.equals(it.getGroupId()) && artifactId.equals(it.getArtifactId())); } /** - * Retrieves the version of an optional dependency from the project's classpath. + * Retrieves the version of an optional dependency from the project's compile classpath. + *

+ * Scoped the same way as {@link #isPresentOnClassPath(MavenProject, String, String)}, and for the same reason. * * @param project the Maven project whose dependencies are checked. * @param groupId the group ID of the dependency. @@ -69,11 +85,24 @@ public static boolean isPresentOnClassPath(final MavenProject project, final Str */ public static Optional optionalArtifactVersion(final MavenProject project, final String groupId, final String artifactId) { return project.getArtifacts().stream() + .filter(ProjectUtils::isOnCompileClassPath) .filter(artifact -> groupId.equals(artifact.getGroupId()) && artifactId.equals(artifact.getArtifactId())) .map(Artifact::getVersion) .findFirst(); } + /** + * Whether the artifact ends up on the compile classpath. + *

+ * An artifact resolved without a scope is treated as compile scoped, matching how Maven defaults a dependency + * which declares none. + */ + private static boolean isOnCompileClassPath(final Artifact artifact) { + final String scope = artifact.getScope(); + + return scope == null || COMPILE_CLASS_PATH_SCOPES.contains(scope); + } + /** * Resolves the configured step log level from the Maven plugin confifiguration or falls back * to a provided default if the configuration is missing or incomplete. diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/CodeQualityArchUnitIT.java b/src/test/java/io/github/finoid/maven/plugins/codequality/CodeQualityArchUnitIT.java new file mode 100644 index 0000000..0477a41 --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/CodeQualityArchUnitIT.java @@ -0,0 +1,207 @@ +package io.github.finoid.maven.plugins.codequality; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.maven.it.VerificationException; +import org.apache.maven.it.Verifier; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +/** + * Runs the ArchUnit step against a reactor whose rules live in a library rather than in the analyzed module. + *

+ * The point of the fixture is where the rules come from. The {@code rules} module produces an ordinary jar holding an + * {@code ArchRule} constant and an {@code ArchRuleProvider}, and the {@code app} module does nothing but depend on + * that jar in test scope: one rule is referenced by name from the plugin configuration, the other is found through + * the service loader without {@code app} mentioning it at all. Both therefore have to be loaded from the dependency, + * which is what separates this step from the analyzers whose checks are built into the plugin. + *

+ * The remaining assertions cover what a unit test cannot reach: that the violations survive into the aggregated + * GitLab report with a repository relative path, that a per rule severity override keyed by the reported rule name + * applies, and that a non permissive run fails the build. + */ +class CodeQualityArchUnitIT { + private static final String FIXTURE = "it/archunit-reactor"; + + private static final String EXPLICIT_RULE = "ItRules.NO_IMPL_SUFFIX"; + private static final String PROVIDED_RULE = "PROVIDED_NO_PUBLIC_MUTABLE_STATICS"; + + private static final String IMPL_PATH = "app/src/main/java/it/app/FooImpl.java"; + private static final String COUNTERS_PATH = "app/src/main/java/it/app/Counters.java"; + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + private static String pluginVersion; + + @BeforeAll + static void beforeAll() { + pluginVersion = System.getProperty("it.plugin.version"); + + Assertions.assertNotNull(pluginVersion, "The it.plugin.version system property has to be provided by the failsafe configuration"); + } + + @Test + @DisplayName("Rules referenced from, and provided by, a dependency are both evaluated and reported") + void givenRulesInADependency_whenVerify_thenBothSourcesAreEvaluated() throws Exception { + final Path basedir = copyFixture("archunit"); + + final Verifier verifier = verifier(basedir); + verifier.executeGoal("verify"); + verifier.verifyErrorFreeLog(); + verifier.resetStreams(); + + final List violations = aggregatedViolations(basedir); + final Map byPath = new LinkedHashMap<>(); + + violations.forEach(violation -> byPath.put(violation.path(), violation)); + + Assertions.assertEquals(2, violations.size(), + () -> "Expected one violation per rule, the explicitly referenced one and the provided one, but got " + violations); + + Assertions.assertTrue(byPath.containsKey(IMPL_PATH), + () -> "The explicitly referenced rule is expected to report " + IMPL_PATH + ", got " + byPath.keySet()); + Assertions.assertTrue(byPath.containsKey(COUNTERS_PATH), + () -> "The provided rule is expected to report " + COUNTERS_PATH + ", got " + byPath.keySet()); + + violations.forEach(violation -> Assertions.assertTrue(violation.description().startsWith("ArchUnit: "), + () -> "Unexpected non ArchUnit violation: " + violation)); + + violations.forEach(violation -> Assertions.assertTrue(violation.line() >= 1, + () -> "A violation is expected to carry a line, never zero: " + violation)); + + // The severity override is keyed by the reported rule name, which only the explicitly referenced rule has one for + Assertions.assertEquals("blocker", byPath.get(IMPL_PATH).severity()); + Assertions.assertEquals("major", byPath.get(COUNTERS_PATH).severity()); + + final String log = logOf(basedir); + + Assertions.assertTrue(log.contains("[ArchUnit - " + EXPLICIT_RULE + "]"), + "The explicitly referenced rule is expected to be reported under its normalised name"); + Assertions.assertTrue(log.contains("[ArchUnit - " + PROVIDED_RULE + "]"), + "The provided rule is expected to be reported under the name its provider gave it"); + } + + @Test + @DisplayName("A non permissive run fails the build on the violations found in the dependency's rules") + void givenNonPermissiveRun_whenVerify_thenBuildFails() throws Exception { + final Path basedir = copyFixture("archunit-strict"); + + final Verifier verifier = verifier(basedir); + verifier.addCliOption("-Dcq.it.archunit.permissive=false"); + + Assertions.assertThrows(VerificationException.class, () -> verifier.executeGoal("verify"), + "The build is expected to fail once the violations are not permissive"); + + verifier.resetStreams(); + + verifier.verifyTextInLog("Severity threshold has been exceeded."); + } + + private Verifier verifier(final Path basedir) throws VerificationException { + final Verifier verifier = new Verifier(basedir.toString()); + + verifier.setForkJvm(true); + verifier.setAutoclean(false); + verifier.addCliOption("-Dcq.plugin.version=" + pluginVersion); + + return verifier; + } + + private List aggregatedViolations(final Path basedir) throws IOException { + final Path report = basedir.resolve("target/gitlab-violations.json"); + + Assertions.assertTrue(Files.isRegularFile(report), () -> "Missing aggregated report: " + report); + + final JsonNode reported = OBJECT_MAPPER.readTree(Files.readString(report, StandardCharsets.UTF_8)); + + final List violations = new ArrayList<>(); + + for (final JsonNode violation : reported) { + violations.add(new ReportedViolation( + violation.path("location").path("path").asText(), + violation.path("location").path("lines").path("begin").asInt(), + violation.path("severity").asText(), + violation.path("description").asText())); + } + + return violations; + } + + private String logOf(final Path basedir) throws IOException { + return Files.readString(basedir.resolve("log.txt"), StandardCharsets.UTF_8); + } + + private Path copyFixture(final String name) throws IOException { + final Path fixtureRoot = Paths.get(System.getProperty("basedir", ""), "target", "test-classes", FIXTURE) + .toAbsolutePath(); + + Assertions.assertTrue(Files.isDirectory(fixtureRoot), () -> "Missing integration test fixture: " + fixtureRoot); + + final Path target = Paths.get(System.getProperty("basedir", ""), "target", "it", name) + .toAbsolutePath(); + + deleteRecursively(target); + + Files.createDirectories(target); + + try (Stream sources = Files.walk(fixtureRoot)) { + sources.forEach(source -> { + final Path destination = target.resolve(fixtureRoot.relativize(source).toString()); + + try { + if (Files.isDirectory(source)) { + Files.createDirectories(destination); + } else { + Files.copy(source, destination); + } + } catch (final IOException e) { + throw new UncheckedIOException(e); + } + }); + } + + return target; + } + + private static void deleteRecursively(final Path path) throws IOException { + if (!Files.exists(path)) { + return; + } + + Files.walkFileTree(path, new SimpleFileVisitor<>() { + @Override + public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException { + Files.delete(file); + + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(final Path directory, final IOException exception) throws IOException { + Files.delete(directory); + + return FileVisitResult.CONTINUE; + } + }); + } + + private record ReportedViolation(String path, int line, String severity, String description) { + } +} diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleResolverUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleResolverUnitTest.java new file mode 100644 index 0000000..c3d8bc4 --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchRuleResolverUnitTest.java @@ -0,0 +1,133 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import io.github.finoid.maven.plugins.codequality.ExecutionContext; +import io.github.finoid.maven.plugins.codequality.archunit.fixture.TestRuleProvider; +import io.github.finoid.maven.plugins.codequality.configuration.ArchUnitConfiguration; +import io.github.finoid.maven.plugins.codequality.exceptions.CodeQualityException; +import io.github.finoid.maven.plugins.codequality.fixtures.UnitTest; +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.mockito.Mock; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +class ArchRuleResolverUnitTest extends UnitTest { + private static final String RULES_CLASS = "io.github.finoid.maven.plugins.codequality.archunit.fixture.TestRules"; + private static final String PROVIDER_CLASS = "io.github.finoid.maven.plugins.codequality.archunit.fixture.TestRuleProvider"; + + @Mock + private Log log; + + private ArchRuleResolver unit; + private ExecutionContext context; + + @BeforeEach + void beforeEach() { + unit = new ArchRuleResolver(); + context = ExecutionContext.of(new MavenProject(), log); + } + + @Test + @DisplayName("Given a static field reference, Then the rule is named SimpleClass.member so it is usable as a config key") + void resolvesAStaticFieldReference() { + final List rules = resolve(RULES_CLASS + "#A_RULE"); + + Assertions.assertEquals(1, rules.size()); + Assertions.assertEquals("TestRules.A_RULE", rules.getFirst().name()); + } + + @Test + @DisplayName("Given a static method reference, Then the rule is resolved") + void resolvesAStaticMethodReference() { + final List rules = resolve(RULES_CLASS + "#ruleFromAMethod()"); + + Assertions.assertEquals(1, rules.size()); + Assertions.assertEquals("TestRules.ruleFromAMethod", rules.getFirst().name()); + } + + @Test + @DisplayName("Given a bare class reference, Then every static rule field of the class is resolved") + void resolvesEveryStaticFieldOfAClass() { + final List names = resolve(RULES_CLASS).stream() + .map(NamedArchRule::name) + .sorted() + .toList(); + + Assertions.assertEquals(List.of("TestRules.ANOTHER_RULE", "TestRules.A_RULE"), names); + } + + @Test + @DisplayName("Given a provider class reference, Then the rules it provides are resolved under their own names") + void resolvesAProviderClassReference() { + final List rules = resolve(PROVIDER_CLASS); + + Assertions.assertEquals(1, rules.size()); + Assertions.assertEquals(TestRuleProvider.RULE_NAME, rules.getFirst().name()); + } + + @Test + @DisplayName("Given the same rule referenced twice, Then it is evaluated once") + void collapsesDuplicatesByName() { + final ArchUnitConfiguration configuration = configuration(); + configuration.setRules(new LinkedHashSet<>(List.of(RULES_CLASS + "#A_RULE", RULES_CLASS))); + + final List rules = unit.resolve(configuration, getClass().getClassLoader(), context); + + Assertions.assertEquals(2, rules.size(), "A_RULE resolved twice, ANOTHER_RULE once"); + } + + @Test + @DisplayName("Given a member which is not a rule, Then the reference is rejected with the offending reference named") + void rejectsAMemberWhichIsNotARule() { + final CodeQualityException exception = + Assertions.assertThrows(CodeQualityException.class, () -> resolve(RULES_CLASS + "#NOT_A_RULE")); + + Assertions.assertTrue(exception.getMessage().contains("NOT_A_RULE"), exception.getMessage()); + Assertions.assertTrue(exception.getMessage().contains("not a static field of type ArchRule"), exception.getMessage()); + } + + @Test + @DisplayName("Given a method returning something else, Then the reference is rejected") + void rejectsAMethodWhichDoesNotReturnARule() { + final CodeQualityException exception = + Assertions.assertThrows(CodeQualityException.class, () -> resolve(RULES_CLASS + "#notARule()")); + + Assertions.assertTrue(exception.getMessage().contains("not a static no-args method returning an ArchRule"), exception.getMessage()); + } + + @Test + @DisplayName("Given an unknown class, Then the reference is rejected") + void rejectsAnUnknownClass() { + final CodeQualityException exception = + Assertions.assertThrows(CodeQualityException.class, () -> resolve("com.example.DoesNotExist#RULE")); + + Assertions.assertTrue(exception.getMessage().contains("could not be loaded from the test classpath"), exception.getMessage()); + } + + @Test + @DisplayName("Given no rules and no service loader, Then nothing is resolved rather than failing") + void resolvesNothingWithoutConfiguration() { + Assertions.assertTrue(unit.resolve(configuration(), getClass().getClassLoader(), context).isEmpty()); + } + + private List resolve(final String... references) { + final ArchUnitConfiguration configuration = configuration(); + configuration.setRules(new LinkedHashSet<>(Set.of(references))); + + return unit.resolve(configuration, getClass().getClassLoader(), context); + } + + private static ArchUnitConfiguration configuration() { + final ArchUnitConfiguration configuration = new ArchUnitConfiguration(); + // The plugin's own test classpath registers no provider; the explicit references are what is under test. + configuration.setServiceLoaderEnabled(false); + + return configuration; + } +} 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 new file mode 100644 index 0000000..3a89e83 --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzerUnitTest.java @@ -0,0 +1,167 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +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.report.Severity; +import io.github.finoid.maven.plugins.codequality.report.Violation; +import io.github.finoid.maven.plugins.codequality.step.ViolationConverter; +import org.apache.maven.execution.MavenExecutionRequest; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Build; +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.mockito.Mock; +import org.mockito.Mockito; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.Map; + +/** + * Runs the analyzer against the compiled classes of this very module, which is the only source of real bytecode + * available to a unit test, and the only way to prove the source locations are mapped back onto real files. + */ +class ArchUnitAnalyzerUnitTest extends UnitTest { + private static final Path WORKING_DIRECTORY = Paths.get("") + .toAbsolutePath(); + + @Mock + private MavenSession session; + @Mock + private MavenExecutionRequest request; + @Mock + private Log log; + + private ArchUnitAnalyzer unit; + private ExecutionContext context; + + @BeforeEach + void beforeEach() { + // Lenient: the repository root is only read when a violation is actually converted, and two of the tests + // below deliberately produce none. + Mockito.lenient() + .when(session.getRequest()) + .thenReturn(request); + Mockito.lenient() + .when(request.getMultiModuleProjectDirectory()) + .thenReturn(WORKING_DIRECTORY.toFile()); + + unit = new ArchUnitAnalyzer(new ViolationConverter(session)); + context = ExecutionContext.of(project(), log); + } + + @Test + @DisplayName("Given a rule no class satisfies, Then every violation carries the source file and line it belongs to") + void reportsViolationsWithTheirSourceLocation() { + final ArchRule rule = ArchRuleDefinition.classes() + .that() + .haveSimpleName("ArchUnitAnalyzer") + .should() + .haveSimpleName("SomethingElse") + .allowEmptyShould(true); + + final List violations = unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration(), context); + + Assertions.assertEquals(1, violations.size()); + + final Violation violation = violations.getFirst(); + Assertions.assertEquals("ArchUnit", violation.getTool()); + Assertions.assertEquals("NAMING", violation.getRule()); + Assertions.assertEquals( + "src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzer.java", + violation.getRelativePath()); + Assertions.assertTrue(violation.getLine() >= 1, "Line should never be reported as zero"); + Assertions.assertFalse(violation.getDescription().contains("\n"), "Description should be a single line"); + } + + @Test + @DisplayName("Given a satisfied rule, Then nothing is reported") + void reportsNothingForASatisfiedRule() { + final ArchRule rule = ArchRuleDefinition.classes() + .that() + .haveSimpleName("ArchUnitAnalyzer") + .should() + .haveSimpleName("ArchUnitAnalyzer") + .allowEmptyShould(true); + + Assertions.assertTrue(unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration(), context).isEmpty()); + } + + @Test + @DisplayName("Given a per rule severity, Then it overrides the step wide default") + void appliesThePerRuleSeverity() { + final ArchUnitConfiguration configuration = configuration(); + configuration.setSeverity(Severity.MINOR); + configuration.setRuleSeverities(Map.of("NAMING", Severity.BLOCKER)); + + final ArchRule rule = ArchRuleDefinition.classes() + .that() + .haveSimpleName("ArchUnitAnalyzer") + .should() + .haveSimpleName("SomethingElse") + .allowEmptyShould(true); + + final List violations = unit.analyze(List.of(NamedArchRule.of("NAMING", rule)), configuration, context); + + Assertions.assertEquals(Severity.BLOCKER, violations.getFirst().getSeverity()); + } + + @Test + @DisplayName("Given the same violation twice, Then the fingerprint is stable") + void producesAStableFingerprint() { + final ArchRule rule = ArchRuleDefinition.classes() + .that() + .haveSimpleName("ArchUnitAnalyzer") + .should() + .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); + + Assertions.assertEquals(first.getFirst().getFingerprint(), second.getFirst().getFingerprint()); + } + + @Test + @DisplayName("Given a module without compiled classes, Then nothing is analyzed rather than failing") + void skipsAModuleWithoutCompiledClasses() { + final MavenProject emptyProject = new MavenProject(); + final Build build = new Build(); + build.setOutputDirectory(WORKING_DIRECTORY.resolve("target/does-not-exist").toString()); + build.setTestOutputDirectory(WORKING_DIRECTORY.resolve("target/does-not-exist-either").toString()); + emptyProject.setBuild(build); + emptyProject.setFile(WORKING_DIRECTORY.resolve("pom.xml").toFile()); + + final ExecutionContext emptyContext = ExecutionContext.of(emptyProject, log); + + Assertions.assertTrue(unit.analyze(List.of(), configuration(), emptyContext).isEmpty()); + } + + private static ArchUnitConfiguration configuration() { + return new ArchUnitConfiguration(); + } + + private static MavenProject project() { + final MavenProject project = new MavenProject(); + + final Build build = new Build(); + build.setOutputDirectory(WORKING_DIRECTORY.resolve("target/classes").toString()); + build.setTestOutputDirectory(WORKING_DIRECTORY.resolve("target/test-classes").toString()); + + project.setBuild(build); + 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/archunit/TestClassPathResolverUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/TestClassPathResolverUnitTest.java new file mode 100644 index 0000000..675c8d3 --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/TestClassPathResolverUnitTest.java @@ -0,0 +1,133 @@ +package io.github.finoid.maven.plugins.codequality.archunit; + +import io.github.finoid.maven.plugins.codequality.exceptions.CodeQualityException; +import io.github.finoid.maven.plugins.codequality.fixtures.UnitTest; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Build; +import org.apache.maven.project.DependencyResolutionException; +import org.apache.maven.project.DependencyResolutionRequest; +import org.apache.maven.project.DependencyResolutionResult; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectDependenciesResolver; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.util.artifact.JavaScopes; +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.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.Mockito; + +import java.io.IOException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +class TestClassPathResolverUnitTest extends UnitTest { + @Mock + private ProjectDependenciesResolver dependenciesResolver; + @Mock + private MavenSession session; + @Mock + private DependencyResolutionResult result; + + @TempDir + private Path temporaryDirectory; + + private TestClassPathResolver unit; + + @BeforeEach + void beforeEach() { + unit = new TestClassPathResolver(dependenciesResolver, session); + } + + @Test + @DisplayName("Given a resolved dependency, When resolving, Then the output directories come first and the jar follows") + void resolvesOutputDirectoriesAheadOfTheDependencies() throws Exception { + final Path classes = Files.createDirectory(temporaryDirectory.resolve("classes")); + final Path testClasses = Files.createDirectory(temporaryDirectory.resolve("test-classes")); + final Path jar = Files.createFile(temporaryDirectory.resolve("rules.jar")); + + Mockito.when(result.getResolvedDependencies()) + .thenReturn(List.of(dependency(jar))); + Mockito.when(dependenciesResolver.resolve(Mockito.any())) + .thenReturn(result); + + final List classPath = unit.resolve(project(classes, testClasses)); + + Assertions.assertEquals( + List.of(classes.toUri().toURL(), testClasses.toUri().toURL(), jar.toUri().toURL()), + classPath); + } + + @Test + @DisplayName("Given a dependency which was never resolved to a file, When resolving, Then it is left out") + void skipsDependenciesWithoutAFile() throws Exception { + final Path classes = Files.createDirectory(temporaryDirectory.resolve("classes")); + + Mockito.when(result.getResolvedDependencies()) + .thenReturn(List.of(new Dependency(new DefaultArtifact("g:a:1.0"), JavaScopes.TEST))); + Mockito.when(dependenciesResolver.resolve(Mockito.any())) + .thenReturn(result); + + final List classPath = unit.resolve(project(classes, temporaryDirectory.resolve("absent"))); + + Assertions.assertEquals(List.of(classes.toUri().toURL()), classPath); + } + + @Test + @DisplayName("When resolving, Then the dependencies are filtered down to the test classpath") + void filtersDownToTheTestClassPath() throws Exception { + Mockito.when(result.getResolvedDependencies()) + .thenReturn(List.of()); + Mockito.when(dependenciesResolver.resolve(Mockito.any())) + .thenReturn(result); + + unit.resolve(project(temporaryDirectory.resolve("absent"), temporaryDirectory.resolve("absent-too"))); + + final ArgumentCaptor request = ArgumentCaptor.forClass(DependencyResolutionRequest.class); + Mockito.verify(dependenciesResolver) + .resolve(request.capture()); + + Assertions.assertNotNull(request.getValue().getResolutionFilter(), + "The request is expected to carry a filter, so that only the test classpath is resolved"); + } + + @Test + @DisplayName("Given unresolvable dependencies, When resolving, Then the module is named in the failure") + void namesTheModuleWhenResolutionFails() throws Exception { + // Built before the stubbing: its constructor calls into the result, which Mockito would otherwise read as + // an unfinished stubbing of that mock + final DependencyResolutionException failure = new DependencyResolutionException(result, "boom", new IOException("boom")); + + Mockito.when(dependenciesResolver.resolve(Mockito.any())) + .thenThrow(failure); + + final MavenProject project = project(temporaryDirectory.resolve("absent"), temporaryDirectory.resolve("absent-too")); + project.setArtifactId("the-module"); + + final CodeQualityException exception = Assertions.assertThrows(CodeQualityException.class, () -> unit.resolve(project)); + + Assertions.assertTrue(exception.getMessage().contains("the-module"), exception.getMessage()); + } + + private static Dependency dependency(final Path file) { + return new Dependency(new DefaultArtifact("g:a:1.0").setFile(file.toFile()), JavaScopes.TEST); + } + + private static MavenProject project(final Path classes, final Path testClasses) { + final MavenProject project = new MavenProject(); + + final Build build = new Build(); + build.setOutputDirectory(classes.toString()); + build.setTestOutputDirectory(testClasses.toString()); + + project.setBuild(build); + + return project; + } +} diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/fixture/TestRuleProvider.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/fixture/TestRuleProvider.java new file mode 100644 index 0000000..771b5ae --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/fixture/TestRuleProvider.java @@ -0,0 +1,19 @@ +package io.github.finoid.maven.plugins.codequality.archunit.fixture; + +import io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider; +import io.github.finoid.maven.plugins.codequality.archunit.NamedArchRule; + +import java.util.Collection; +import java.util.List; + +/** + * A provider in the shape a rule library would ship. + */ +public class TestRuleProvider implements ArchRuleProvider { + public static final String RULE_NAME = "PROVIDED_RULE"; + + @Override + public Collection rules() { + return List.of(NamedArchRule.of(RULE_NAME, TestRules.A_RULE)); + } +} diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/fixture/TestRules.java b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/fixture/TestRules.java new file mode 100644 index 0000000..2ac23d7 --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/archunit/fixture/TestRules.java @@ -0,0 +1,32 @@ +package io.github.finoid.maven.plugins.codequality.archunit.fixture; + +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; + +/** + * Rules referenced by the resolver tests, in each of the shapes the explicit configuration accepts. + */ +public final class TestRules { + public static final ArchRule A_RULE = ArchRuleDefinition.classes() + .should() + .bePublic() + .allowEmptyShould(true); + + public static final ArchRule ANOTHER_RULE = ArchRuleDefinition.classes() + .should() + .haveSimpleNameNotEndingWith("Impl") + .allowEmptyShould(true); + + private static final String NOT_A_RULE = "ignored"; + + private TestRules() { + } + + public static ArchRule ruleFromAMethod() { + return A_RULE; + } + + public static String notARule() { + return NOT_A_RULE; + } +} diff --git a/src/test/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtilsUnitTest.java b/src/test/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtilsUnitTest.java new file mode 100644 index 0000000..a5d927b --- /dev/null +++ b/src/test/java/io/github/finoid/maven/plugins/codequality/util/ProjectUtilsUnitTest.java @@ -0,0 +1,80 @@ +package io.github.finoid.maven.plugins.codequality.util; + +import io.github.finoid.maven.plugins.codequality.fixtures.UnitTest; +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.handler.DefaultArtifactHandler; +import org.apache.maven.artifact.versioning.VersionRange; +import org.apache.maven.project.MavenProject; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Set; + +/** + * Everything answered from the resolved artifact set filters by scope, so that the answer does not depend on the + * resolution scope the goal happens to declare. A test or runtime scoped Lombok or checker-qual must never make an + * analyzer believe it is available to the compilation of the main sources, which happens against the compile + * classpath only: the Checker Framework step would pass its prerequisite and then fail the forked compile. + */ +class ProjectUtilsUnitTest extends UnitTest { + private static final String GROUP_ID = "org.checkerframework"; + private static final String ARTIFACT_ID = "checker-qual"; + + @Test + @DisplayName("Given a compile scoped dependency, When looked up, Then it is found") + void findsACompileScopedDependency() { + final MavenProject project = projectWith(artifact(Artifact.SCOPE_COMPILE)); + + Assertions.assertTrue(ProjectUtils.isPresentOnClassPath(project, GROUP_ID, ARTIFACT_ID)); + Assertions.assertEquals("1.0.0", ProjectUtils.optionalArtifactVersion(project, GROUP_ID, ARTIFACT_ID).orElse(null)); + } + + @Test + @DisplayName("Given a provided scoped dependency, When looked up, Then it is found") + void findsAProvidedScopedDependency() { + final MavenProject project = projectWith(artifact(Artifact.SCOPE_PROVIDED)); + + Assertions.assertTrue(ProjectUtils.isPresentOnClassPath(project, GROUP_ID, ARTIFACT_ID)); + } + + @Test + @DisplayName("Given a test scoped dependency, When looked up, Then it is not found") + void ignoresATestScopedDependency() { + final MavenProject project = projectWith(artifact(Artifact.SCOPE_TEST)); + + Assertions.assertFalse(ProjectUtils.isPresentOnClassPath(project, GROUP_ID, ARTIFACT_ID), + "A test scoped dependency is not on the compile classpath the analyzers compile against"); + Assertions.assertTrue(ProjectUtils.optionalArtifactVersion(project, GROUP_ID, ARTIFACT_ID).isEmpty()); + } + + @Test + @DisplayName("Given a runtime scoped dependency, When looked up, Then it is not found") + void ignoresARuntimeScopedDependency() { + final MavenProject project = projectWith(artifact(Artifact.SCOPE_RUNTIME)); + + Assertions.assertFalse(ProjectUtils.isPresentOnClassPath(project, GROUP_ID, ARTIFACT_ID)); + } + + @Test + @DisplayName("Given a dependency resolved without a scope, When looked up, Then it is treated as compile scoped") + void treatsAMissingScopeAsCompile() { + final MavenProject project = projectWith(artifact(null)); + + Assertions.assertTrue(ProjectUtils.isPresentOnClassPath(project, GROUP_ID, ARTIFACT_ID)); + } + + private static MavenProject projectWith(final Artifact artifact) { + final MavenProject project = new MavenProject(); + + project.setArtifacts(Set.of(artifact)); + + return project; + } + + private static Artifact artifact(final String scope) { + return new DefaultArtifact(GROUP_ID, ARTIFACT_ID, VersionRange.createFromVersion("1.0.0"), scope, "jar", null, + new DefaultArtifactHandler("jar")); + } +} diff --git a/src/test/resources/it/archunit-reactor/.mvn/maven.config b/src/test/resources/it/archunit-reactor/.mvn/maven.config new file mode 100644 index 0000000..ff82809 --- /dev/null +++ b/src/test/resources/it/archunit-reactor/.mvn/maven.config @@ -0,0 +1 @@ +--no-transfer-progress diff --git a/src/test/resources/it/archunit-reactor/app/pom.xml b/src/test/resources/it/archunit-reactor/app/pom.xml new file mode 100644 index 0000000..d659b33 --- /dev/null +++ b/src/test/resources/it/archunit-reactor/app/pom.xml @@ -0,0 +1,70 @@ + + + 4.0.0 + + + io.github.finoid.it + archunit-reactor + 1.0 + + + app + app + + + + + io.github.finoid.it + rules + 1.0 + test + + + + + + + io.github.finoid + codequality-maven-plugin + ${cq.plugin.version} + true + + + code-quality + verify + + code-quality + + + + + + + + false + + + false + + + false + + + true + ${cq.it.archunit.permissive} + + + it.rules.ItRules#NO_IMPL_SUFFIX + + + BLOCKER + + + + + + + + diff --git a/src/test/resources/it/archunit-reactor/app/src/main/java/it/app/Counters.java b/src/test/resources/it/archunit-reactor/app/src/main/java/it/app/Counters.java new file mode 100644 index 0000000..f2ccf7d --- /dev/null +++ b/src/test/resources/it/archunit-reactor/app/src/main/java/it/app/Counters.java @@ -0,0 +1,11 @@ +package it.app; + +/** + * Violates the rule the library registers through the service loader. + */ +public class Counters { + public static int counter; + + private Counters() { + } +} diff --git a/src/test/resources/it/archunit-reactor/app/src/main/java/it/app/FooImpl.java b/src/test/resources/it/archunit-reactor/app/src/main/java/it/app/FooImpl.java new file mode 100644 index 0000000..71b93a5 --- /dev/null +++ b/src/test/resources/it/archunit-reactor/app/src/main/java/it/app/FooImpl.java @@ -0,0 +1,10 @@ +package it.app; + +/** + * Violates the explicitly referenced rule, by way of its name. + */ +public class FooImpl { + public String value() { + return "foo"; + } +} diff --git a/src/test/resources/it/archunit-reactor/pom.xml b/src/test/resources/it/archunit-reactor/pom.xml new file mode 100644 index 0000000..76bb71d --- /dev/null +++ b/src/test/resources/it/archunit-reactor/pom.xml @@ -0,0 +1,34 @@ + + + 4.0.0 + + io.github.finoid.it + archunit-reactor + 1.0 + pom + archunit-reactor + + + 21 + 21 + 21 + 21 + UTF-8 + + 1.4.1 + + + main + + true + + + + + rules + app + + diff --git a/src/test/resources/it/archunit-reactor/rules/pom.xml b/src/test/resources/it/archunit-reactor/rules/pom.xml new file mode 100644 index 0000000..7f1e737 --- /dev/null +++ b/src/test/resources/it/archunit-reactor/rules/pom.xml @@ -0,0 +1,30 @@ + + + 4.0.0 + + + io.github.finoid.it + archunit-reactor + 1.0 + + + rules + rules + + + + com.tngtech.archunit + archunit + ${archunit.version} + + + + io.github.finoid + codequality-maven-plugin + ${cq.plugin.version} + provided + + + diff --git a/src/test/resources/it/archunit-reactor/rules/src/main/java/it/rules/ItRuleProvider.java b/src/test/resources/it/archunit-reactor/rules/src/main/java/it/rules/ItRuleProvider.java new file mode 100644 index 0000000..e03f9b0 --- /dev/null +++ b/src/test/resources/it/archunit-reactor/rules/src/main/java/it/rules/ItRuleProvider.java @@ -0,0 +1,25 @@ +package it.rules; + +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; +import io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider; +import io.github.finoid.maven.plugins.codequality.archunit.NamedArchRule; + +import java.util.Collection; +import java.util.List; + +/** + * The same library registering a rule of its own, so a consuming module picks it up without configuring anything. + */ +public class ItRuleProvider implements ArchRuleProvider { + @Override + public Collection rules() { + return List.of(NamedArchRule.of("PROVIDED_NO_PUBLIC_MUTABLE_STATICS", ArchRuleDefinition.fields() + .that() + .areStatic() + .and() + .areNotFinal() + .should() + .notBePublic() + .allowEmptyShould(true))); + } +} diff --git a/src/test/resources/it/archunit-reactor/rules/src/main/java/it/rules/ItRules.java b/src/test/resources/it/archunit-reactor/rules/src/main/java/it/rules/ItRules.java new file mode 100644 index 0000000..e622024 --- /dev/null +++ b/src/test/resources/it/archunit-reactor/rules/src/main/java/it/rules/ItRules.java @@ -0,0 +1,17 @@ +package it.rules; + +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; + +/** + * Rules shipped by a library, referenced explicitly from the plugin configuration of the consuming module. + */ +public final class ItRules { + public static final ArchRule NO_IMPL_SUFFIX = ArchRuleDefinition.classes() + .should() + .haveSimpleNameNotEndingWith("Impl") + .allowEmptyShould(true); + + private ItRules() { + } +} diff --git a/src/test/resources/it/archunit-reactor/rules/src/main/resources/META-INF/services/io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider b/src/test/resources/it/archunit-reactor/rules/src/main/resources/META-INF/services/io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider new file mode 100644 index 0000000..7ceba5a --- /dev/null +++ b/src/test/resources/it/archunit-reactor/rules/src/main/resources/META-INF/services/io.github.finoid.maven.plugins.codequality.archunit.ArchRuleProvider @@ -0,0 +1 @@ +it.rules.ItRuleProvider