From b771cc1c980b291f19aa8f47f57c1a3b6e2407b3 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 6 Oct 2026 13:50:25 +0200 Subject: [PATCH] build: add ArchUnit, JaCoCo, Spotless, SpotBugs, Error Prone/NullAway, forbidden-apis, CPD, dependency-analysis --- .github/workflows/build.yml | 13 +- build.gradle.kts | 117 +++++++++++++++++- .../loki/loparkour/arch/ArchitectureTest.java | 52 ++++++++ .../loparkour/arch/StructureRulesTest.java | 75 +++++++++++ src/test/resources/archunit.properties | 4 + .../339ab3ea-28aa-4f4b-a1e4-6c69e3ff0a0a | 7 ++ .../58784f63-3f60-4edb-9733-fa0d7e11b877 | 0 .../d1d8a9d7-476c-4f86-adda-8531054aea1f | 0 .../f1e3c34b-9b02-40e6-83a6-0269e173eb83 | 3 + .../resources/archunit_store/stored.rules | 6 + 10 files changed, 275 insertions(+), 2 deletions(-) create mode 100644 src/test/java/dev/loki/loparkour/arch/ArchitectureTest.java create mode 100644 src/test/java/dev/loki/loparkour/arch/StructureRulesTest.java create mode 100644 src/test/resources/archunit.properties create mode 100644 src/test/resources/archunit_store/339ab3ea-28aa-4f4b-a1e4-6c69e3ff0a0a create mode 100644 src/test/resources/archunit_store/58784f63-3f60-4edb-9733-fa0d7e11b877 create mode 100644 src/test/resources/archunit_store/d1d8a9d7-476c-4f86-adda-8531054aea1f create mode 100644 src/test/resources/archunit_store/f1e3c34b-9b02-40e6-83a6-0269e173eb83 create mode 100644 src/test/resources/archunit_store/stored.rules diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 60a216a..0636df0 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -55,8 +55,19 @@ jobs: with: distribution: temurin java-version: 21 - - name: Full build + - name: Full build (checkstyle, PMD, spotless, spotbugs, forbidden-apis, CPD, tests, coverage) run: ./gradlew --no-daemon build + - name: Upload quality reports + if: always() + uses: actions/upload-artifact@v5 + with: + name: quality-reports + path: | + build/reports/spotbugs + build/reports/cpd + build/reports/jacoco + build/reports/checkstyle + build/reports/pmd - name: Upload default jar uses: actions/upload-artifact@v5 with: diff --git a/build.gradle.kts b/build.gradle.kts index 331ef3a..0b2e711 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,3 +1,4 @@ +import net.ltgt.gradle.errorprone.errorprone import java.io.File import java.net.URI import java.util.zip.ZipFile @@ -15,11 +16,18 @@ buildscript { } plugins { - id("java") + id("java-library") id("com.gradleup.shadow") version("9.6.1") id("xyz.jpenilla.run-paper") version("3.0.2") checkstyle pmd + jacoco + id("net.ltgt.errorprone") version("4.3.0") + id("com.diffplug.spotless") version("7.0.4") + id("com.github.spotbugs") version("6.1.13") + id("de.thetaphi.forbiddenapis") version("3.9") + id("com.autonomousapps.dependency-analysis") version("2.19.0") + id("org.owasp.dependencycheck") version("12.1.3") } group = "dev.loki" @@ -144,6 +152,8 @@ val remapNms by tasks.registering { } tasks.withType().configureEach { dependsOn(remapNms) } +// Analysis tasks (dependency-analysis, spotbugs, ...) read the remapped NMS jars too. +tasks.matching { it.name != "remapNms" && it.name != "clean" }.configureEach { mustRunAfter(remapNms) } // Resolve the org.bukkit:bukkit capability conflict that newer paper-api artifacts // introduce (they both bundle the bukkit classes and declare the capability). @@ -201,11 +211,16 @@ dependencies { compileOnly("me.filoghost.holographicdisplays:holographicdisplays-api:3.0.0-SNAPSHOT") compileOnly("org.geysermc.floodgate:api:2.0-SNAPSHOT") + // Error Prone + NullAway (compile-time bug/null-safety checks) + errorprone("com.google.errorprone:error_prone_core:2.41.0") + errorprone("com.uber.nullaway:nullaway:0.12.10") + // Annotations compileOnly("org.jetbrains:annotations:24.1.0") // Testing testImplementation("org.junit.jupiter:junit-jupiter:5.11.0-M1") + testImplementation("com.tngtech.archunit:archunit-junit5:1.4.1") testImplementation("org.mockito:mockito-core:5.10.0") testImplementation("org.mockito:mockito-junit-jupiter:5.10.0") testImplementation("org.spigotmc:spigot-api:1.20.4-R0.1-SNAPSHOT") @@ -246,6 +261,17 @@ tasks.withType { tasks.withType { options.encoding = "UTF-8" options.release = javaRelease + options.errorprone { + disableWarningsInGeneratedCode = true + // NullAway: only code annotated @NullMarked-style packages is checked once adopted; + // start as warnings so the baseline is visible but does not block. + check("NullAway", net.ltgt.gradle.errorprone.CheckSeverity.WARN) + option("NullAway:AnnotatedPackages", "dev.loki.loparkour") + option("NullAway:ExcludedFieldAnnotations", "org.jetbrains.annotations.Nullable") + } + if (name == "compileTestJava") { + options.errorprone.isEnabled.set(false) + } } tasks.withType { @@ -299,5 +325,94 @@ tasks.withType().configureEach { exclude("**/build/generated/**") } +// --------------------------------------------------------------------------- +// Extra quality gates (see TODO.md, task 2). Existing debt is recorded, new +// violations fail the build; tighten thresholds as the refactor progresses. +// --------------------------------------------------------------------------- + +jacoco { + toolVersion = "0.8.13" +} + +tasks.test { + finalizedBy(tasks.jacocoTestReport) +} + +tasks.jacocoTestReport { + dependsOn(tasks.test) + reports { + xml.required.set(true) + html.required.set(true) + } +} + +tasks.jacocoTestCoverageVerification { + violationRules { + rule { + limit { + counter = "LINE" + minimum = "0.05".toBigDecimal() // baseline; raise toward 0.5 (core 0.7) + } + } + } +} + +tasks.check { + dependsOn(tasks.jacocoTestCoverageVerification, "cpdCheck") +} + +spotless { + java { + target("src/**/*.java") + removeUnusedImports() + trimTrailingWhitespace() + endWithNewline() + } +} + +spotbugs { + toolVersion = "4.9.3" + effort = com.github.spotbugs.snom.Effort.MAX + reportLevel = com.github.spotbugs.snom.Confidence.MEDIUM + ignoreFailures = true // report-only until the baseline is triaged +} + +tasks.withType().configureEach { + reports.create("html") { required = true } +} + +forbiddenApis { + bundledSignatures = setOf("jdk-unsafe-21", "jdk-deprecated-21", "jdk-system-out", "jdk-non-portable") + ignoreFailures = true // report-only until the baseline is triaged +} + +// Copy/paste detector (DRY). The de.aaschmid.cpd plugin does not support PMD 7, so run PMD's CPD directly. +val cpdTool: Configuration by configurations.creating +dependencies { + cpdTool("net.sourceforge.pmd:pmd-cli:7.0.0") + cpdTool("net.sourceforge.pmd:pmd-java:7.0.0") +} + +val cpdCheck by tasks.registering(JavaExec::class) { + group = "verification" + description = "Finds duplicated code (min 100 tokens). Report-only until duplicates are triaged." + classpath = cpdTool + mainClass.set("net.sourceforge.pmd.cli.PmdCli") + args("cpd", "--minimum-tokens", "100", "--language", "java", "--dir", "src/main/java", + "--format", "text") + isIgnoreExitValue = true // exit code 4 = duplicates found + doFirst { + val dir = layout.buildDirectory.dir("reports/cpd").get().asFile + dir.mkdirs() + standardOutput = File(dir, "cpd.txt").outputStream() + } +} + +dependencyCheck { + // Run explicitly: ./gradlew dependencyCheckAnalyze (set NVD_API_KEY for speed). + failBuildOnCVSS = 7.0f + nvd.apiKey = System.getenv("NVD_API_KEY") +} + // Default task defaultTasks("clean", "build") diff --git a/src/test/java/dev/loki/loparkour/arch/ArchitectureTest.java b/src/test/java/dev/loki/loparkour/arch/ArchitectureTest.java new file mode 100644 index 0000000..ffc276e --- /dev/null +++ b/src/test/java/dev/loki/loparkour/arch/ArchitectureTest.java @@ -0,0 +1,52 @@ +package dev.loki.loparkour.arch; + +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchIgnore; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.lang.ArchRule; +import org.bukkit.scheduler.BukkitRunnable; + +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; +import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices; +import static com.tngtech.archunit.library.freeze.FreezingArchRule.freeze; + +/** + * Enforces the dependency rules from docs/ARCHITECTURE.md. Rules wrapped in {@code freeze} record + * existing violations in {@code src/test/resources/archunit_store}; only NEW violations fail the build. + */ +@AnalyzeClasses(packages = "dev.loki.loparkour", importOptions = ImportOption.DoNotIncludeTests.class) +class ArchitectureTest { + + private static final String ROOT = "dev.loki.loparkour"; + + // Disabled: the codebase has 100+ package cycles and freeze() cannot track cycle violations + // stably (their ids change between runs). Enable once cycles are reduced (TODO task 2). + @ArchIgnore + @ArchTest + static final ArchRule NO_PACKAGE_CYCLES = freeze( + slices().matching(ROOT + ".(*)..").should().beFreeOfCycles()); + + @ArchTest + static final ArchRule API_IS_INDEPENDENT_OF_PLATFORM = freeze( + noClasses().that().resideInAPackage(ROOT + ".api..") + .should().dependOnClassesThat().resideInAnyPackage( + ROOT + ".menu..", ROOT + ".command..", ROOT + ".storage..", + ROOT + ".listener..", ROOT + ".hook..")); + + @ArchTest + static final ArchRule NO_BUKKIT_RUNNABLE_OUTSIDE_SCHEDULER = freeze( + noClasses().that().resideOutsideOfPackage(ROOT + ".scheduler..") + .should().dependOnClassesThat().areAssignableTo(BukkitRunnable.class) + .because("all tasks go through scheduler/ so they work on Folia")); + + @ArchTest + static final ArchRule NO_BUKKIT_SCHEDULER_OUTSIDE_SCHEDULER = freeze( + noClasses().that().resideOutsideOfPackage(ROOT + ".scheduler..") + .should().callMethod(org.bukkit.Bukkit.class, "getScheduler") + .because("Bukkit.getScheduler() does not exist on Folia")); + + @ArchTest + static final ArchRule NO_LEGACY_LOLIB = + noClasses().should().dependOnClassesThat().resideInAPackage("dev.lolib.."); +} diff --git a/src/test/java/dev/loki/loparkour/arch/StructureRulesTest.java b/src/test/java/dev/loki/loparkour/arch/StructureRulesTest.java new file mode 100644 index 0000000..d8c9b4a --- /dev/null +++ b/src/test/java/dev/loki/loparkour/arch/StructureRulesTest.java @@ -0,0 +1,75 @@ +package dev.loki.loparkour.arch; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Hard structure limits: at most 4 files per package folder, at most 200 lines per file. + * Existing debt is listed explicitly and must only shrink; new violations fail the build. + */ +class StructureRulesTest { + + private static final Path MAIN = Path.of("src/main/java"); + private static final int MAX_FILES_PER_FOLDER = 4; + private static final int MAX_LINES_PER_FILE = 200; + + /** Known debt (docs/TODO.md, task 2). Remove entries as folders get split. */ + private static final Set FOLDER_DEBT = Set.of( + "dev/loki/loparkour/command/schematic", + "dev/loki/loparkour/elytra/generator", + "dev/loki/loparkour/elytra/mode", + "dev/loki/loparkour/generator/jump/calculation", + "dev/loki/loparkour/generator/jump/placement", + "dev/loki/loparkour/player/core"); + + @Test + void foldersHaveAtMostFourFiles() throws IOException { + final Map perFolder = new TreeMap<>(); + for (final Path file : javaFiles()) { + perFolder.merge(MAIN.relativize(file.getParent()).toString(), 1L, Long::sum); + } + final List violations = perFolder.entrySet().stream() + .filter(e -> e.getValue() > MAX_FILES_PER_FOLDER) + .map(Map.Entry::getKey) + .filter(folder -> !FOLDER_DEBT.contains(folder)) + .collect(Collectors.toList()); + assertTrue(violations.isEmpty(), "Folders with more than " + MAX_FILES_PER_FOLDER + " files: " + violations); + + final List fixed = FOLDER_DEBT.stream() + .filter(folder -> perFolder.getOrDefault(folder, 0L) <= MAX_FILES_PER_FOLDER) + .collect(Collectors.toList()); + assertTrue(fixed.isEmpty(), "Folders now within the limit — remove them from FOLDER_DEBT: " + fixed); + } + + @Test + void filesHaveAtMost200Lines() throws IOException { + final List violations = new java.util.ArrayList<>(); + for (final Path file : javaFiles()) { + final long lines; + try (Stream content = Files.lines(file)) { + lines = content.count(); + } + if (lines > MAX_LINES_PER_FILE) { + violations.add(MAIN.relativize(file) + " (" + lines + ")"); + } + } + assertTrue(violations.isEmpty(), "Files longer than " + MAX_LINES_PER_FILE + " lines: " + violations); + } + + private static List javaFiles() throws IOException { + try (Stream walk = Files.walk(MAIN)) { + return walk.filter(p -> p.toString().endsWith(".java")).collect(Collectors.toList()); + } + } +} diff --git a/src/test/resources/archunit.properties b/src/test/resources/archunit.properties new file mode 100644 index 0000000..7323a2b --- /dev/null +++ b/src/test/resources/archunit.properties @@ -0,0 +1,4 @@ +# Violation store for FreezingArchRule: existing debt is recorded, only new violations fail. +freeze.store.default.path=src/test/resources/archunit_store +freeze.store.default.allowStoreCreation=true +freeze.refreeze=false diff --git a/src/test/resources/archunit_store/339ab3ea-28aa-4f4b-a1e4-6c69e3ff0a0a b/src/test/resources/archunit_store/339ab3ea-28aa-4f4b-a1e4-6c69e3ff0a0a new file mode 100644 index 0000000..a502e5a --- /dev/null +++ b/src/test/resources/archunit_store/339ab3ea-28aa-4f4b-a1e4-6c69e3ff0a0a @@ -0,0 +1,7 @@ +Class extends class in (MetricsCacheStore.java:0) +Constructor (dev.loki.loparkour.adaptive.core.metrics.MetricsCacheStore)> calls constructor ()> in (MetricsCacheStore.java:84) +Field has type in (MetricsCacheStore.java:0) +Method calls constructor (dev.loki.loparkour.adaptive.core.metrics.MetricsCacheStore)> in (MetricsCacheStore.java:84) +Method calls method in (MetricsCacheStore.java:81) +Method calls method in (MetricsCacheStore.java:91) +Method calls method in (MetricsCacheStore.java:96) diff --git a/src/test/resources/archunit_store/58784f63-3f60-4edb-9733-fa0d7e11b877 b/src/test/resources/archunit_store/58784f63-3f60-4edb-9733-fa0d7e11b877 new file mode 100644 index 0000000..e69de29 diff --git a/src/test/resources/archunit_store/d1d8a9d7-476c-4f86-adda-8531054aea1f b/src/test/resources/archunit_store/d1d8a9d7-476c-4f86-adda-8531054aea1f new file mode 100644 index 0000000..e69de29 diff --git a/src/test/resources/archunit_store/f1e3c34b-9b02-40e6-83a6-0269e173eb83 b/src/test/resources/archunit_store/f1e3c34b-9b02-40e6-83a6-0269e173eb83 new file mode 100644 index 0000000..ffce91d --- /dev/null +++ b/src/test/resources/archunit_store/f1e3c34b-9b02-40e6-83a6-0269e173eb83 @@ -0,0 +1,3 @@ +Method calls method in (DuelsCountdown.java:45) +Method calls method in (DuelsCountdown.java:55) +Method calls method in (DuelsGenerator.java:137) diff --git a/src/test/resources/archunit_store/stored.rules b/src/test/resources/archunit_store/stored.rules new file mode 100644 index 0000000..62e7a72 --- /dev/null +++ b/src/test/resources/archunit_store/stored.rules @@ -0,0 +1,6 @@ +# +#Tue Oct 06 13:42:44 CEST 2026 +no\ classes\ that\ reside\ in\ a\ package\ 'dev.loki.loparkour.api..'\ should\ depend\ on\ classes\ that\ reside\ in\ any\ package\ ['dev.loki.loparkour.menu..',\ 'dev.loki.loparkour.command..',\ 'dev.loki.loparkour.storage..',\ 'dev.loki.loparkour.listener..',\ 'dev.loki.loparkour.hook..']=d1d8a9d7-476c-4f86-adda-8531054aea1f +no\ classes\ that\ reside\ outside\ of\ package\ 'dev.loki.loparkour.scheduler..'\ should\ call\ method\ Bukkit.getScheduler(),\ because\ Bukkit.getScheduler()\ does\ not\ exist\ on\ Folia=f1e3c34b-9b02-40e6-83a6-0269e173eb83 +no\ classes\ that\ reside\ outside\ of\ package\ 'dev.loki.loparkour.scheduler..'\ should\ depend\ on\ classes\ that\ are\ assignable\ to\ org.bukkit.scheduler.BukkitRunnable,\ because\ all\ tasks\ go\ through\ scheduler/\ so\ they\ work\ on\ Folia=339ab3ea-28aa-4f4b-a1e4-6c69e3ff0a0a +slices\ matching\ 'dev.loki.loparkour.(*)..'\ should\ be\ free\ of\ cycles=58784f63-3f60-4edb-9733-fa0d7e11b877