Add support for violation suppression

README.md

@@ -116,6 +116,49 @@ For continuous use across builds, include the plugin in your project’s pom.xml
 | `violationReporters`       | List of violation reporters.                                              | `[CONSOLE_PLAIN,GITLAB_FILE_VIOLATION]` |
 | `violationFilters`         | List of violation filters.                                                | `[]`                                    |
 
+### Suppressing a violation
+
+`SUPPRESSION_COMMENT` is a violation filter which drops findings a comment in the source has opted out of. Enable it
+alongside any other filters:
+
+```xml
+<codeQuality>
+    <violationFilters>
+        <violationFilter>SUPPRESSION_COMMENT</violationFilter>
+    </violationFilters>
+</codeQuality>
+```
+
+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:
+
+```java
+// suppress:NullAway the framework sets this before the first call
+private String name;
+
+private String name; // suppress:NullAway same, on the line itself
+```
+
+Violations are reported against the **declaration** - of the method, field or type - rather than against the first
+statement the bytecode records, so the comment goes directly above the member it concerns.
+
+The rule has to be named exactly as the report names it, being the part after the tool in the console output - for
+example `NullAway`, `MethodName(name.invalidPattern)` or
+`TransactionRules.NO_TRANSACTIONAL_METHOD_SHOULD_START_A_SECOND_TRANSACTION`. One comment may name several rules. A
+bare `// suppress` is deliberately not honoured: it would silently swallow the next, unrelated finding on the same
+line.
+
+Suppressions apply to every analyzer, since the filter works off the reported position rather than off the source
+language. How many were honoured is logged unconditionally, so a run reporting nothing can still be told apart from
+one whose findings were all opted out of:
+
+```
+[INFO] Suppressed 1 violation(s) by comment
+```
+
+Note that a suppression removes the finding before the severity threshold is evaluated, so it also stops a non
+permissive violation failing the build.
+
 ### Checkstyle configuration
 
 | Parameter       | Description                                         | Default |
@@ -191,6 +234,7 @@ by surefire and once here.
 | `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` |
+| `compileIfMissing`     | Whether to compile the module when it has not been compiled already.    | `false` |
 | `severity`             | The severity reported for a rule without an entry in `ruleSeverities`.  | `MAJOR` |
 | `ruleSeverities`       | Severity per rule name, overriding `severity`.                          | `{}`    |
 
@@ -241,6 +285,38 @@ META-INF/services/io.github.finoid.maven.plugins.codequality.archunit.ArchRulePr
 Provider names are chosen by the library, so keep them usable as XML element names if consumers should be able to
 override their severity.
 
+#### Phase requirement
+
+The step reads the classes the build has already produced, rather than forking a compiler of its own the way Error
+Prone and the Checker Framework do. It therefore has to run at or after `compile`. Bound to an earlier phase, or
+invoked on the command line ahead of one, it reports a missing prerequisite and skips:
+
+```
+[INFO] Step ARCH_UNIT is missing prerequisites to run. Cause: the module has no compiled classes,
+       the goal has to run at or after the compile phase. Skipping...
+```
+
+Running the goal directly therefore needs a phase in front of it:
+
+```shell
+mvn clean compile io.github.finoid:codequality-maven-plugin:code-quality@maven-code-quality
+```
+
+Or let the step compile the module itself:
+
+```xml
+<archUnit>
+    <enabled>true</enabled>
+    <compileIfMissing>true</compileIfMissing>
+</archUnit>
+```
+
+The compilation goes into `target/archunit-classes`, so it neither overwrites the output of the build nor leads a
+later phase to believe the module is already built, and that directory is cleaned before each run. It reproduces the
+release level and the annotation processors of the module, which covers a Lombok using service, but not a bespoke
+compiler configuration - additional compiler arguments, generated source roots, a module path. Where that matters,
+run the goal after the compile phase instead and analyze exactly what the build produced.
+
 #### 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

src/main/java/io/github/finoid/maven/plugins/codequality/archunit/ArchUnitAnalyzer.java

@@ -1,7 +1,6 @@
 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;
@@ -11,12 +10,10 @@
 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;
@@ -40,18 +37,19 @@ public ArchUnitAnalyzer(final ViolationConverter violationConverter) {
     /**
      * Evaluates the given rules.
      *
-     * @param rules         the rules to evaluate
-     * @param configuration the step configuration
-     * @param context       the context of the current mojo execution
+     * @param classDirectories the directories holding the compiled classes to analyze
+     * @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<Violation> analyze(final List<NamedArchRule> rules, final ArchUnitConfiguration configuration,
-                                   final ExecutionContext context) {
-        final List<Path> classDirectories = classDirectoriesOf(context.getProject(), configuration.isAnalyzeTestClasses());
-
+    public List<Violation> analyze(final List<Path> classDirectories, final List<NamedArchRule> rules,
+                                   final ArchUnitConfiguration configuration, final ExecutionContext context) {
         if (classDirectories.isEmpty()) {
+            // Normally unreachable, the step declares the output directory as a prerequisite. Warned about rather
+            // than debugged, so an empty analysis is never mistaken for a clean one.
             context.getLog()
-                .debug("No compiled classes to analyze with ArchUnit. Skipping...");
+                .warn("No compiled classes to analyze with ArchUnit, every rule will report nothing. Skipping...");
 
             return List.of();
         }
@@ -98,22 +96,25 @@ public void handle(final Collection<Object> correspondingObjects, final String m
     private Violation toViolation(final NamedArchRule namedRule, final Collection<Object> correspondingObjects,
                                   final String message, final ArchUnitConfiguration configuration,
                                   final SourceFileResolver sourceFileResolver) {
-        final Optional<SourceCodeLocation> location = sourceCodeLocation(correspondingObjects);
+        final Optional<HasSourceCodeLocation> element = locatableElement(correspondingObjects);
 
-        final File file = location.map(sourceFileResolver::resolve)
+        final File file = element.map(it -> sourceFileResolver.resolve(it.getSourceCodeLocation()))
             .orElseGet(sourceFileResolver::moduleDirectory);
-        final int line = location.map(sourceFileResolver::lineNumber)
+        final int line = element.map(it -> sourceFileResolver.lineNumber(it, it.getSourceCodeLocation(), file))
             .orElse(1);
 
         return violationConverter.ofArchUnitViolation(namedRule.name(), singleLine(message), file, line,
             configuration.severityOf(namedRule.name()));
     }
 
-    private static Optional<SourceCodeLocation> sourceCodeLocation(final Collection<Object> correspondingObjects) {
+    /**
+     * The element the violation is reported against, kept rather than only its location: a member knows its own name,
+     * which is what lets the declaration be found in the source.
+     */
+    private static Optional<HasSourceCodeLocation> locatableElement(final Collection<Object> correspondingObjects) {
         return correspondingObjects.stream()
             .filter(HasSourceCodeLocation.class::isInstance)
             .map(HasSourceCodeLocation.class::cast)
-            .map(HasSourceCodeLocation::getSourceCodeLocation)
             .findFirst();
     }
 
@@ -128,23 +129,4 @@ private static String singleLine(final String message) {
             .trim();
     }
 
-    private static List<Path> classDirectoriesOf(final MavenProject project, final boolean includeTestClasses) {
-        final List<Path> directories = new ArrayList<>();
-
-        addIfDirectory(directories, project.getBuild().getOutputDirectory());
-
-        if (includeTestClasses) {
-            addIfDirectory(directories, project.getBuild().getTestOutputDirectory());
-        }
-
-        return directories;
-    }
-
-    private static void addIfDirectory(final List<Path> directories, final String directory) {
-        final Path path = Path.of(directory);
-
-        if (Files.isDirectory(path)) {
-            directories.add(path);
-        }
-    }
 }

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.
- * <p>
- * 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.
+ *
+ * <p>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.
+ *
+ * <p>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<Path> 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.
-     * <p>
-     * 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<String> 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.
+     *
+     * <p>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<Integer> declarationLineOf(final JavaMember member, final SourceCodeLocation location,
+                                                                 final List<String> 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<Integer> declarationLine(final Pattern declaration, final List<String> 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("(?<![.\\w])" + Pattern.quote(name) + "\\s*\\(");
+    }
+
+    /** A field, being its name followed by an initialiser, a terminator or a further declarator. */
+    private static Pattern fieldDeclaration(final String name) {
+        return Pattern.compile("(?<![.\\w])" + Pattern.quote(name) + "\\s*[;=,]");
+    }
+
+    private static Pattern typeDeclaration(final String simpleName) {
+        return Pattern.compile("\\b(?:class|interface|enum|record|@interface)\\s+" + Pattern.quote(simpleName) + "\\b");
+    }
+
+    private static List<String> 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) {

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.
+     * <p>
+     * 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.
+     * <p>
+     * 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.
      * <p>