phase a/fixups spotbugs

docs/superpowers/baselines/2026-04-17/BASELINE.md

@@ -230,6 +230,7 @@ Ordered by severity. Each item cites the raw artifact it was derived from.
 
 - **SpotBugs: 8 HIGH-priority findings (priority=1) + 1,484 at priority=2.** Total 1,492. HIGH findings must be triaged individually (read `raw/spotbugs.xml`). Noise-dominant rules (`NM_METHOD_NAMING_CONVENTION`=730, `SF_SWITCH_NO_DEFAULT`=448) should be filtered via a SpotBugs exclude file so real signal surfaces; real-concern patterns that deserve review now: `NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE` (26), `BC_UNCONFIRMED_CAST` (55), `UL_UNRELEASED_LOCK_EXCEPTION_PATH` (1), `WMI_WRONG_MAP_ITERATOR` (2), `ES_COMPARING_STRINGS_WITH_EQ` (2), `MT_CORRECTNESS` category (1).
   - Raw: `raw/spotbugs.xml`, `raw/spotbugs-summary.json`.
+  - **RESOLVED (2026-04-17, branch `phase-a/fixups-spotbugs`)**: Added `spotbugs-exclude.xml` covering ANTLR-generated parsers and global noise rules (`NM_METHOD_NAMING_CONVENTION`, `SF_SWITCH_NO_DEFAULT`, `EI_EXPOSE_REP`/`EI_EXPOSE_REP2`, `MS_PKGPROTECT`/`MS_FINAL_PKGPROTECT`), wired via `pom.xml`. Fixed all 8 priority-1 findings in codeiq code (UTF-8 in `Analyzer.getGitHead`, narrowed catch in `IndexCommand`, dead-store removed in `PluginsCommand`, `.equals()` in `AntlrParserFactory` + `CSharpPreprocessorParserBase`, try-finally unlock in `AnalysisCache.removeFile`, merged duplicate branches in `CodeIqApplication`, removed dead `BundleCommand.writeEntry` overload, `entrySet()` iteration in `PluginsCommand` + `GitLabCiDetector`, narrowed `VersionCommand` catch). **Final: 1,492 → 38 (-97.5%); priority-1: 8 → 0.** Remaining 38 are priority-2 STYLE/BAD_PRACTICE; no CORRECTNESS/MT_CORRECTNESS/SECURITY left. Next-pass candidates: 26 `NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE`. Post-triage summary: `raw/spotbugs-summary-after-triage.json`.
 
 ### Medium
 

pom.xml

@@ -323,6 +323,9 @@
                 <groupId>com.github.spotbugs</groupId>
                 <artifactId>spotbugs-maven-plugin</artifactId>
                 <version>${spotbugs.version}</version>
+                <configuration>
+                    <excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
+                </configuration>
             </plugin>
 
             <plugin>

spotbugs-exclude.xml

@@ -0,0 +1,93 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  SpotBugs exclude filter for code-iq.
+
+  Philosophy: fail CI on real bugs, not on stylistic or generated-code noise.
+  Each entry MUST have a rationale comment. If a rule here ever catches a
+  genuine bug in new code, narrow or remove it — never silently live with it.
+
+  Docs: https://spotbugs.readthedocs.io/en/latest/filter.html
+-->
+<FindBugsFilter>
+
+    <!--
+      ANTLR-generated parser sources.
+      Regenerated on every `mvn generate-sources` from .g4 files in
+      src/main/antlr4/. Fixing findings here is futile. Hand-written ANTLR
+      support classes (name ends in `Base`, e.g. CSharpParserBase) are NOT
+      excluded — those are ours to fix.
+    -->
+    <Match>
+        <Class name="~io\.github\.randomcodespace\.iq\.grammar\..*(Parser|Lexer|Listener|Visitor)$"/>
+    </Match>
+
+    <!--
+      NM_METHOD_NAMING_CONVENTION — camelCase/PascalCase warnings.
+      730 findings, nearly all from generated parsers and overridden ANTLR
+      hooks (e.g. `OnPreprocessorExpressionConditionalEq`). Not actionable
+      for new hand-written code; the compiler + review catch real typos.
+    -->
+    <Match>
+        <Bug pattern="NM_METHOD_NAMING_CONVENTION"/>
+    </Match>
+
+    <!--
+      SF_SWITCH_NO_DEFAULT — stylistic.
+      448 findings, dominated by generated ANTLR rule dispatch. Real
+      fall-through/dead-store bugs are caught separately by
+      SF_SWITCH_FALLTHROUGH and SF_DEAD_STORE_DUE_TO_SWITCH_FALLTHROUGH,
+      which we DO enforce.
+    -->
+    <Match>
+        <Bug pattern="SF_SWITCH_NO_DEFAULT"/>
+    </Match>
+
+    <!--
+      EI_EXPOSE_REP / EI_EXPOSE_REP2 — "constructor/getter stores or returns
+      internal mutable state". 123 findings across detector result DTOs and
+      graph-model records. No trust boundary is crossed here: these objects
+      live inside a single JVM and are consumed by trusted pipeline code.
+      Defensive copies would add GC cost for no security benefit. If we ever
+      expose these across a security boundary, tighten here.
+    -->
+    <Match>
+        <Bug pattern="EI_EXPOSE_REP"/>
+    </Match>
+    <Match>
+        <Bug pattern="EI_EXPOSE_REP2"/>
+    </Match>
+
+    <!--
+      MS_PKGPROTECT / MS_FINAL_PKGPROTECT — "mutable non-final static field
+      could be overwritten by a malicious subclass in the same package".
+      80 findings. This JVM runs no untrusted bytecode; there is no
+      same-package attacker model. Pure noise.
+    -->
+    <Match>
+        <Bug pattern="MS_PKGPROTECT"/>
+    </Match>
+    <Match>
+        <Bug pattern="MS_FINAL_PKGPROTECT"/>
+    </Match>
+
+    <!--
+      RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE in hand-written ANTLR parser
+      support classes (CSharpParserBase, GoParserBase). Defensive null checks
+      carried over verbatim from antlr/grammars-v4 grammar action files. Harmless;
+      divergence risk if we "fix" them because upstream may re-sync.
+    -->
+    <Match>
+        <Class name="~io\.github\.randomcodespace\.iq\.grammar\.(csharp\.CSharpParserBase|golang\.GoParserBase)"/>
+        <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
+    </Match>
+
+    <!--
+      BX_UNBOXING_IMMEDIATELY_REBOXED in CSharpPreprocessorParserBase: grammar
+      action code adapted from upstream. Micro-perf, not a correctness issue.
+    -->
+    <Match>
+        <Class name="io.github.randomcodespace.iq.grammar.csharp.CSharpPreprocessorParserBase"/>
+        <Bug pattern="BX_UNBOXING_IMMEDIATELY_REBOXED"/>
+    </Match>
+
+</FindBugsFilter>

src/main/java/io/github/randomcodespace/iq/CodeIqApplication.java

@@ -54,8 +54,8 @@
                 .findFirst()
                 .orElse("");
         boolean isServe = "serve".equalsIgnoreCase(command);
         boolean isIndex = "index".equalsIgnoreCase(command);
         boolean isEnrich = "enrich".equalsIgnoreCase(command);
 
         if (isServe) {
             app.setAdditionalProfiles("serving");
@@ -96,17 +96,12 @@
             // Point Neo4j config to the graph path (enriched or new empty db).
             // GraphBootstrapper will auto-load from H2 cache if no enriched graph exists.
             System.setProperty("codeiq.graph.path", graphDbPath.toString());
-        } else if (isIndex) {
-            app.setAdditionalProfiles("indexing");
-            // Index command: no web server, no Neo4j
-            app.setWebApplicationType(org.springframework.boot.WebApplicationType.NONE);
-        } else if (isEnrich) {
-            // Enrich command: no web server, Neo4j started programmatically
-            app.setAdditionalProfiles("indexing");
-            app.setWebApplicationType(org.springframework.boot.WebApplicationType.NONE);
         } else {
+            // All non-serve commands (index, enrich, analyze, stats, ...) share the same
+            // Spring setup: "indexing" profile, no web server. index/enrich open Neo4j
+            // programmatically when needed. Previously split into three identical
+            // branches — SpotBugs DB_DUPLICATE_BRANCHES.
             app.setAdditionalProfiles("indexing");
-            // Disable web server for non-serve commands
             app.setWebApplicationType(org.springframework.boot.WebApplicationType.NONE);
         }
 

src/main/java/io/github/randomcodespace/iq/analyzer/Analyzer.java

@@ -1,5 +1,6 @@
 package io.github.randomcodespace.iq.analyzer;
 
+import java.nio.charset.StandardCharsets;
 import io.github.randomcodespace.iq.analyzer.linker.Linker;
 import io.github.randomcodespace.iq.cache.AnalysisCache;
 import io.github.randomcodespace.iq.cache.FileHasher;
@@ -1322,7 +1323,7 @@
      * shutdown — prevents the default close() from hanging up to 24 hours on stuck
      * ANTLR threads.
      */
     private record BoundedExecutor(java.util.concurrent.ExecutorService delegate) implements AutoCloseable {
         <T> Future<T> submit(java.util.concurrent.Callable<T> task) { return delegate.submit(task); }
 
         @Override
@@ -1608,7 +1609,7 @@
                     .directory(repoPath.toFile())
                     .redirectErrorStream(true);
             Process proc = pb.start();
-            String sha = new String(proc.getInputStream().readAllBytes()).trim();
+            String sha = new String(proc.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim();
             int exitCode = proc.waitFor();
             if (exitCode == 0 && sha.length() >= 7) {
                 return sha;
@@ -1630,7 +1631,7 @@
      * Ensure a node's properties map is mutable. Some nodes are created with
      * immutable Map.of() which throws UnsupportedOperationException on put().
      */
     private static Map<String, Object> ensureMutableProperties(CodeNode node) {
         Map<String, Object> props = node.getProperties();
         try {
             // Test mutability — HashMap/LinkedHashMap will not throw

src/main/java/io/github/randomcodespace/iq/cache/AnalysisCache.java

@@ -372,10 +372,16 @@ public void storeResults(String contentHash, String filePath, String language,
             log.warn("Failed to store cached results for hash {}", contentHash, e);
         } finally {
             try {
-                conn.setAutoCommit(true);
-            } catch (SQLException ignored) {
+                try {
+                    conn.setAutoCommit(true);
+                } catch (SQLException ignored) {
+                    // best-effort restore; the INSERTs have already been committed or rolled back.
+                }
+            } finally {
+                // Guarantee unlock even if conn.setAutoCommit throws a non-SQLException
+                // (RuntimeException / Error). Fixes SpotBugs UL_UNRELEASED_LOCK_EXCEPTION_PATH.
+                rwLock.writeLock().unlock();
             }
-            rwLock.writeLock().unlock();
         }
     }
 
@@ -453,10 +459,16 @@ public void removeFile(String contentHash) {
             log.warn("Failed to remove cached file {}", contentHash, e);
         } finally {
             try {
-                conn.setAutoCommit(true);
-            } catch (SQLException ignored) {
+                try {
+                    conn.setAutoCommit(true);
+                } catch (SQLException ignored) {
+                    // best-effort restore; the DELETEs have already been committed or rolled back.
+                }
+            } finally {
+                // Guarantee unlock even if conn.setAutoCommit throws a non-SQLException
+                // (RuntimeException / Error). Fixes SpotBugs UL_UNRELEASED_LOCK_EXCEPTION_PATH.
+                rwLock.writeLock().unlock();
             }
-            rwLock.writeLock().unlock();
         }
     }
 
@@ -599,10 +611,16 @@ public void replaceAll(List<CodeNode> nodes, List<CodeEdge> edges) {
             log.warn("Failed to replace cache with enriched data", e);
         } finally {
             try {
-                conn.setAutoCommit(true);
-            } catch (SQLException ignored) {
+                try {
+                    conn.setAutoCommit(true);
+                } catch (SQLException ignored) {
+                    // best-effort restore; the INSERTs have already been committed or rolled back.
+                }
+            } finally {
+                // Guarantee unlock even if conn.setAutoCommit throws a non-SQLException
+                // (RuntimeException / Error). Fixes SpotBugs UL_UNRELEASED_LOCK_EXCEPTION_PATH.
+                rwLock.writeLock().unlock();
             }
-            rwLock.writeLock().unlock();
         }
     }
 

src/main/java/io/github/randomcodespace/iq/cli/BundleCommand.java

@@ -464,9 +464,4 @@ private void writeEntry(ZipOutputStream zos, String name, String content) throws
         zos.closeEntry();
     }
 
-    private void writeEntry(ZipOutputStream zos, String name, String content, String lineEnding)
-            throws IOException {
-        writeEntry(zos, name, content);
-    }
-
 }

src/main/java/io/github/randomcodespace/iq/cli/IndexCommand.java

@@ -85,8 +85,15 @@ public Integer call() {
             if (java.nio.file.Files.exists(cacheDir)) {
                 try {
                     try (var walk = java.nio.file.Files.walk(cacheDir)) {
+                        var logger = org.slf4j.LoggerFactory.getLogger(IndexCommand.class);
                         walk.sorted(java.util.Comparator.reverseOrder())
-                                .forEach(p -> { try { java.nio.file.Files.deleteIfExists(p); } catch (Exception ignored) {} });
+                                .forEach(p -> {
+                                    try {
+                                        java.nio.file.Files.deleteIfExists(p);
+                                    } catch (java.io.IOException ex) {
+                                        logger.debug("Could not delete cache entry {}: {}", p, ex.getMessage());
+                                    }
+                                });
                     }
                     CliOutput.info("  Deleted existing cache at " + cacheDir);
                 } catch (Exception e) {

src/main/java/io/github/randomcodespace/iq/cli/PluginsCommand.java

@@ -100,8 +100,6 @@ public Integer call() {
          * or falls back to a sensible default.
          */
         static String categoryDescription(String category, List<Detector> detectors) {
-            // Collect unique frameworks from DetectorInfo if available
-            Set<String> frameworks = new TreeSet<>();
             for (Detector d : detectors) {
                 DetectorInfo info = d.getClass().getAnnotation(DetectorInfo.class);
                 if (info != null && info.description() != null && !info.description().isEmpty()) {
@@ -406,8 +404,9 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) {
             yaml.append("# Optimized for this project's detected languages\n\n");
 
             yaml.append("languages:\n");
-            for (String lang : languageCounts.keySet()) {
-                yaml.append("  - ").append(lang).append("  # ").append(languageCounts.get(lang)).append(" files\n");
+            for (Map.Entry<String, Integer> entry : languageCounts.entrySet()) {
+                yaml.append("  - ").append(entry.getKey())
+                        .append("  # ").append(entry.getValue()).append(" files\n");
             }
 
             yaml.append("\ndetectors:\n");

src/main/java/io/github/randomcodespace/iq/cli/VersionCommand.java

@@ -38,8 +38,8 @@ private static String resolveVersion() {
                 String v = props.getProperty("build.version");
                 if (v != null && !v.isBlank()) return v;
             }
-        } catch (Exception ignored) {
-            // intentionally empty
+        } catch (java.io.IOException ignored) {
+            // build-info.properties is optional; fall through to manifest lookup.
         }
         // Fallback: Implementation-Version from JAR manifest
         String v = VersionCommand.class.getPackage().getImplementationVersion();

src/main/java/io/github/randomcodespace/iq/detector/config/GitLabCiDetector.java

@@ -135,9 +135,10 @@ public DetectorResult detect(DetectorContext ctx) {
 
         // Collect job names
         List<String> jobNames = new ArrayList<>();
-        for (String key : data.keySet()) {
+        for (Map.Entry<String, Object> entry : data.entrySet()) {
+            String key = entry.getKey();
             if (GITLAB_CI_KEYWORDS.contains(key)) continue;
-            if (data.get(key) instanceof Map<?, ?>) {
+            if (entry.getValue() instanceof Map<?, ?>) {
                 jobNames.add(key);
             }
         }

src/main/java/io/github/randomcodespace/iq/grammar/AntlrParserFactory.java

@@ -113,9 +113,11 @@ public static ParseTree parse(String language, String content) {
             return null;
         }
 
-        // Check thread-local cache — same content object means same file
+        // Check thread-local cache. Using .equals() is correct because the parse tree
+        // is a deterministic function of content — equal content yields an equivalent tree.
+        // (Previously used == for identity fast-path; SpotBugs ES_COMPARING_PARAMETER_STRING_WITH_EQ.)
         var cached = PARSE_CACHE.get();
-        if (cached != null && cached.getKey() == content) {
+        if (cached != null && content.equals(cached.getKey())) {
             return cached.getValue();
         }
 

src/main/java/io/github/randomcodespace/iq/grammar/csharp/CSharpPreprocessorParserBase.java

@@ -179,14 +179,14 @@ protected void OnPreprocessorExpressionConditionalEq()
     {
         ParserRuleContext c = this._ctx;
         CSharpPreprocessorParser.Preprocessor_expressionContext d = (CSharpPreprocessorParser.Preprocessor_expressionContext)c;
-        d.value = (d.expr1.value == d.expr2.value ? "true" : "false");
+        d.value = (java.util.Objects.equals(d.expr1.value, d.expr2.value) ? "true" : "false");
     }
 
     protected void OnPreprocessorExpressionConditionalNe()
     {
         ParserRuleContext c = this._ctx;
         CSharpPreprocessorParser.Preprocessor_expressionContext d = (CSharpPreprocessorParser.Preprocessor_expressionContext)c;
-        d.value = (d.expr1.value != d.expr2.value ? "true" : "false");
+        d.value = (!java.util.Objects.equals(d.expr1.value, d.expr2.value) ? "true" : "false");
     }
 
     protected void OnPreprocessorExpressionConditionalAnd()