Enable Error Prone, fix two dead compiler checks, and cut test-suite overhead

de.peeeq.wurstscript/build.gradle

@@ -13,6 +13,7 @@ plugins {
     id 'eclipse'
     id 'idea'
     id 'jacoco'
+    id 'net.ltgt.errorprone' version '5.1.1'
     id 'maven-publish'
     id 'com.gradleup.shadow' version '9.2.2'
     id 'de.undercouch.download' version '5.6.0'
@@ -41,7 +42,43 @@ tasks.withType(JavaExec).configureEach {
     jvmArgs('-XX:+UnlockExperimentalVMOptions', '-XX:+UseCompactObjectHeaders')
 }
 
-tasks.withType(JavaCompile).configureEach { options.release = 25 }
+/** -------- Compilation and static analysis --------
+ * Both javac lint and Error Prone are ADVISORY: they report at warning severity and
+ * never fail the build. Unit tests and coverage remain the primary quality gate; this
+ * is a supplementary signal, not a merge blocker.
+ * Skip Error Prone entirely with: ./gradlew <task> -PskipErrorProne
+ */
+def errorProneEnabled = !project.hasProperty('skipErrorProne')
+
+tasks.withType(JavaCompile).configureEach {
+    options.release = 25
+    // 'serial'/'this-escape' are noisy against this codebase's AST and visitor style.
+    // 'auxiliaryclass' fires ~86 times purely on the multi-class-per-file layout - structural, not a defect.
+    options.compilerArgs += ['-Xlint:all', '-Xlint:-serial', '-Xlint:-this-escape',
+                             '-Xlint:-processing', '-Xlint:-auxiliaryclass']
+    // javac silently caps reporting at 100 warnings. Without this, lint noise exhausts the
+    // budget and every Error Prone finding is dropped as overflow - the analysis looks clean
+    // while reporting nothing. Do not remove.
+    options.compilerArgs += ['-Xmaxwarns', '10000']
+
+    options.errorprone {
+        enabled = errorProneEnabled
+        // src-gen holds ~700 generated AST classes - analysing them is pure noise.
+        excludedPaths = '.*src-gen.*'
+        disableWarningsInGeneratedCode = true
+        // Advisory only: demote every check to a warning so the build never fails on it.
+        // Requires the -Xmaxwarns bump above to actually be visible.
+        allErrorsAsWarnings = true
+        // Upstream bug: NonCanonicalType NPEs in SuggestedFixes.qualifyType on anonymous
+        // subclasses of nested types, which this codebase's visitors use everywhere
+        // (e.g. `new LuaModel.DefaultVisitor() {}`). A plugin crash is a hard javac error,
+        // so allErrorsAsWarnings cannot absorb it. Purely stylistic check; safe to drop.
+        disable('NonCanonicalType')
+    }
+}
+
+// Keep the test compile loop fast - tests are the primary gate, not the audit target.
+tasks.named('compileTestJava') { options.errorprone.enabled = false }
 
 jacoco {
     toolVersion = "0.8.13"
@@ -90,6 +127,8 @@ configurations {
 }
 
 dependencies {
+    errorprone 'com.google.errorprone:error_prone_core:2.50.0'
+
     implementation 'org.jetbrains:annotations:23.0.0'
 
     // Antlr

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/CompiletimeFunctionRunner.java

@@ -120,7 +120,7 @@ public void run() {
             collectCompiletimeFunctions(toExecute);
             long tCollected = System.nanoTime();
 
-            toExecute.sort(Comparator.comparing(this::getOrderIndex));
+            toExecute.sort(Comparator.comparingInt(this::getOrderIndex));
             long tSorted = System.nanoTime();
 
             execute(toExecute);

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrExprType.java

@@ -74,7 +74,12 @@ public static WurstType calculate(ExprVarAccess term) {
             return WurstTypeUnknown.instance();
         }
         if (!(varDef instanceof OtherLink) && varDef.getDef() instanceof VarDef) {
-            if (Utils.getParentVarDef(Optional.of(term)) == Optional.of((VarDef) varDef.getDef())) {
+            // Compare the enclosing VarDef to the one this access resolves to. Both sides used
+            // to be wrapped in fresh Optionals and compared with ==, which is never true, so
+            // this check silently did nothing. getParentVarDef returns null (not empty) when
+            // there is no enclosing VarDef, hence the explicit null guard.
+            Optional<VarDef> enclosingVarDef = Utils.getParentVarDef(Optional.of(term));
+            if (enclosingVarDef != null && enclosingVarDef.orElse(null) == varDef.getDef()) {
                 term.addError("Recursive variable definition is not allowed.");
                 return WurstTypeUnknown.instance();
             }

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrFuncDef.java

@@ -197,9 +197,8 @@ private ToStringConversionResolution(@Nullable FuncLink conversion, @Nullable St
         var raw = NameResolution.lookupMemberFuncs(node, recvT, node.getFuncName(), /*showErrors=*/false);
 
         java.util.ArrayList<FuncLink> visible = new java.util.ArrayList<>(raw.size());
-        java.util.ArrayList<FuncLink> hidden  = new java.util.ArrayList<>(raw.size());
         for (var f : raw) {
-            if (isVisible(f)) visible.add(f); else hidden.add(f);
+            if (isVisible(f)) visible.add(f);
         }
 
         if (!raw.isEmpty() && visible.isEmpty()) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/OverloadingResolver.java

@@ -5,7 +5,6 @@
 import de.peeeq.wurstscript.types.WurstType;
 import de.peeeq.wurstscript.types.WurstTypeTypeParam;
 import de.peeeq.wurstscript.types.WurstTypeVararg;
-import de.peeeq.wurstscript.utils.NotNullList;
 import de.peeeq.wurstscript.utils.Utils;
 import org.eclipse.jdt.annotation.Nullable;
 
@@ -53,8 +52,6 @@ Optional<F> resolve(Iterable<F> alternativeFunctions, C caller) {
         if (size == 1) {
             return Optional.of(Utils.getFirst(alternativeFunctions));
         }
-        List<String> hints = new NotNullList<>();
-
         Map<F, Integer> numMatches = new HashMap<>();
         for (F f : alternativeFunctions) {
             if (!hasValidParameterCount(f, caller)) {
@@ -68,8 +65,6 @@ Optional<F> resolve(Iterable<F> alternativeFunctions, C caller) {
                         && expectedParamType instanceof WurstTypeTypeParam) {
                     // should be ok!
                 } else if (!getArgumentType(caller, i).isSubtypeOf(expectedParamType, f)) {
-                    hints.add("Expected " + expectedParamType
-                            + " as parameter " + i + " ,but found " + getArgumentType(caller, i) + ".");
                     continue;
                 }
                 matches++;

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/NameLinks.java

@@ -57,7 +57,9 @@ public static ImmutableMultimap<String, DefLink> calculate(ClassOrModuleOrModule
 
     @NotNull
     private static Map<String, Map<FuncLink, OverrideCheckResult>> initOverrideMap(Multimap<String, DefLink> result) {
-        Map<String, Map<FuncLink, OverrideCheckResult>> overrideCheckResults = new Hashtable<>();
+        // LinkedHashMap, not Hashtable: nothing here is concurrent, and reportOverrideErrors
+        // iterates this map, so insertion order beats hash order for reproducible diagnostics.
+        Map<String, Map<FuncLink, OverrideCheckResult>> overrideCheckResults = new LinkedHashMap<>();
         for (DefLink link : result.values()) {
             if (link instanceof FuncLink) {
                 Map<FuncLink, OverrideCheckResult> map = overrideCheckResults.computeIfAbsent(link.getName(),

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/prettyPrint/PrettyUtils.java

@@ -22,21 +22,48 @@ public class PrettyUtils {
     /**
      * @param args
      */
-    public static void pretty(List<String> args) throws IOException {
-        if (args.size() == 0) {
-            return;
+    /**
+     * What {@link #pretty(List)} does with a given argument list.
+     *
+     * <p>Split out from the dispatch so it can be asserted directly: the alternative is running the
+     * real thing, and both the directory walk and the single-file branch print to stdout while
+     * readFile swallows its own exceptions, so neither outcome is distinguishable from the other.
+     */
+    public enum PrettyAction {
+        /** No arguments; nothing to do. */
+        NONE,
+        /** "..." - format every .wurst file below the root. */
+        ALL,
+        /** "tree <file>" - dump the parse tree. */
+        TREE,
+        /** Anything else is taken as a file name. */
+        SINGLE_FILE
+    }
+
+    public static PrettyAction selectAction(List<String> args) {
+        if (args.isEmpty()) {
+            return PrettyAction.NONE;
         }
         String arg = args.get(0);
-        if (args.equals("...")) {
-            prettyAll(".");
+        // This used to read args.equals("..."), comparing the List itself to a String, which is
+        // never true - so "..." fell through and was treated as a file name.
+        if (arg.equals("...")) {
+            return PrettyAction.ALL;
         }
         if (arg.equals("tree") && args.size() >= 2) {
-            debug(args.get(1));
-            return;
+            return PrettyAction.TREE;
         }
+        return PrettyAction.SINGLE_FILE;
+    }
 
-        String clean = pretty(new File(arg));
-        System.out.println(clean);
+    public static void pretty(List<String> args) throws IOException {
+        switch (selectAction(args)) {
+            case NONE -> {
+            }
+            case ALL -> prettyAll(".");
+            case TREE -> debug(args.get(1));
+            case SINGLE_FILE -> System.out.println(pretty(new File(args.get(0))));
+        }
     }
 
     public static String pretty(String source, String ending) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/interpreter/TimerMockHandler.java

@@ -10,7 +10,7 @@
  */
 public class TimerMockHandler {
     private float virtualTime = 0;
-    private final PriorityQueue<RunTask> nextRunnable = new PriorityQueue<>(Comparator.comparing(r -> r.time));
+    private final PriorityQueue<RunTask> nextRunnable = new PriorityQueue<>(Comparator.comparingDouble(r -> r.time));
 
     public void cancelTask(RunTask runTask) {
         nextRunnable.remove(runTask);

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtojass/ImToJassTranslator.java

@@ -67,8 +67,8 @@ private <T extends JassImElementWithName> void makeNamesUnique(List<T> list) {
         List<T> sorted = new ArrayList<>(list);
         sorted.sort(Comparator.comparing(JassImElementWithName::getName)
             .thenComparing(v -> v.getTrace().attrSource().getFile())
-            .thenComparing(v -> v.getTrace().attrSource().getLine())
-            .thenComparing(v -> v.getTrace().attrSource().getStartColumn()));
+            .thenComparingInt(v -> v.getTrace().attrSource().getLine())
+            .thenComparingInt(v -> v.getTrace().attrSource().getStartColumn()));
 
         Set<String> used = new HashSet<>(sorted.size() * 2);
         Map<String, Integer> nextSuffix = new HashMap<>();

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java

@@ -1162,9 +1162,6 @@ private void dbgMethodsByName(String phase) {
     }
 
     private String checkDanglingMethodRefs(String phase) {
-        IdentityHashMap<ImMethod, Boolean> inProg = new IdentityHashMap<>();
-        for (ImMethod m : prog.getMethods()) inProg.put(m, Boolean.TRUE);
-
         final int[] dangling = {0};
 
         prog.accept(new Element.DefaultVisitor() {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java

@@ -812,12 +812,10 @@ private void translateFunc(ImFunction f) {
             }
 
             // translate local variables
-            List<LuaVariable> functionLocals = new ArrayList<>();
             for (ImVar local : f.getLocals()) {
                 LuaVariable luaLocal = luaVar.getFor(local);
                 luaLocal.setInitialValue(defaultValue(local.getType()));
                 lf.getBody().add(luaLocal);
-                functionLocals.add(luaLocal);
             }
 
             // translate body:

de.peeeq.wurstscript/src/test/java/tests/prettyprint/PrettyUtilsArgsTest.java

@@ -0,0 +1,49 @@
+package tests.prettyprint;
+
+import de.peeeq.wurstscript.attributes.prettyPrint.PrettyUtils;
+import org.testng.annotations.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import static org.testng.AssertJUnit.assertEquals;
+
+/**
+ * Argument dispatch for the -prettyPrint CLI entry point.
+ *
+ * <p>The "..." branch used to compare the argument List itself to a String, so it was never taken
+ * and "..." was treated as a file name instead; readFile then swallowed the resulting
+ * FileNotFoundException, so the mistake produced no visible failure.
+ */
+public class PrettyUtilsArgsTest {
+
+    @Test
+    public void tripleDotSelectsDirectoryFormatting() {
+        assertEquals(PrettyUtils.PrettyAction.ALL,
+            PrettyUtils.selectAction(Collections.singletonList("...")));
+    }
+
+    @Test
+    public void aFileNameIsNotTreatedAsDirectoryFormatting() {
+        assertEquals(PrettyUtils.PrettyAction.SINGLE_FILE,
+            PrettyUtils.selectAction(Collections.singletonList("some/file.wurst")));
+    }
+
+    @Test
+    public void treeWithAFileSelectsTreeDump() {
+        assertEquals(PrettyUtils.PrettyAction.TREE,
+            PrettyUtils.selectAction(Arrays.asList("tree", "some/file.wurst")));
+    }
+
+    @Test
+    public void treeWithoutAFileIsTreatedAsAFileName() {
+        assertEquals(PrettyUtils.PrettyAction.SINGLE_FILE,
+            PrettyUtils.selectAction(Collections.singletonList("tree")));
+    }
+
+    @Test
+    public void noArgumentsDoesNothing() {
+        assertEquals(PrettyUtils.PrettyAction.NONE,
+            PrettyUtils.selectAction(Collections.emptyList()));
+    }
+}

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CompilerFuzzTestsSC.java

@@ -27,16 +27,24 @@ public void generatedProgramsAreCrashFree(@From(RandomProgram.class) Program pro
 
     @Property(maxInvocations = 64)
     public void generatedProgramsCompileForBothBackends(@From(RandomProgram.class) Program program) {
-        assertCompilesForBothBackends(program);
+        assertCompilesForBothBackends(program, "generatedProgramsCompileForBothBackends");
     }
 
     @Test
     public void generatedCorpusCompilesForBothBackends() {
-        new RandomProgram().generate(0).forEach(this::assertCompilesForBothBackends);
+        new RandomProgram().generate(0)
+            .forEach(p -> assertCompilesForBothBackends(p, "generatedCorpusCompilesForBothBackends"));
     }
 
-    private void assertCompilesForBothBackends(Program program) {
-        CompilationResult result = test()
+    /**
+     * Both callers used to let test() name the output after this helper, so they shared one file
+     * under ./test-output/. They run in different Gradle forks - the @Test under TestNG, the
+     * @Property under SmallCheckViaJUnitCoreTestNG - so the two JVMs raced on that file and pjass
+     * intermittently parsed a spliced result, reporting word fragments as undefined types. Naming
+     * the output after the calling test keeps them apart.
+     */
+    private void assertCompilesForBothBackends(Program program, String testName) {
+        CompilationResult result = testNamed(testName)
             .setStopOnFirstError(false)
             .executeProg(false)
             .testLua(true)

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ScopingTests.java

@@ -95,5 +95,31 @@ public void privateCode() {
                 "endpackage");
     }
 
+    /**
+     * A class field whose initializer refers to itself must be rejected.
+     * Globals are caught earlier by "must be declared before it is used" and locals by
+     * flow analysis, so class fields are the only shape that reaches the check in
+     * AttrExprType.calculate(ExprVarAccess). That check compared two freshly allocated
+     * Optionals with ==, so it was unreachable and this compiled silently.
+     */
+    @Test
+    public void test_recursive_class_field_def() {
+        testAssertErrorsLines(false, "Recursive variable definition is not allowed",
+                "package test",
+                "	class C",
+                "		int x = x + 1",
+                "endpackage");
+    }
+
+    /** Guard against the check over-triggering: a field initialized from another field is fine. */
+    @Test
+    public void test_non_recursive_class_field_def_ok() {
+        testAssertOkLines(false,
+                "package test",
+                "	class C",
+                "		int a = 1",
+                "		int b = a + 1",
+                "endpackage");
+    }
 
 }