Skip to content

fix(spotbugs): eliminate 12 SpotBugs findings (RAN-23)#71

Merged
aksOps merged 1 commit into
mainfrom
fix/ran-23-spotbugs-cleanup
Apr 24, 2026
Merged

fix(spotbugs): eliminate 12 SpotBugs findings (RAN-23)#71
aksOps merged 1 commit into
mainfrom
fix/ran-23-spotbugs-cleanup

Conversation

@aksOps
Copy link
Copy Markdown
Contributor

@aksOps aksOps commented Apr 24, 2026

Summary

Resolves RAN-23. Eliminates all 12 outstanding SpotBugs findings on main (2 × CT_CONSTRUCTOR_THROW, 6 × DLS_DEAD_LOCAL_STORE, 3 × UC_USELESS_OBJECT, 1 × REC_CATCH_EXCEPTION) and one pre-existing DE_MIGHT_IGNORE that SpotBugs surfaced once the original 12 cleared.

No changes to spotbugs-exclude.xml — every bug is fixed in place, none silenced. Unblocks RAN-24 (binding spotbugs:check to mvn verify).

Fixes per file

File Finding Fix
CodeIqApplication.java:57-58 DLS_DEAD_LOCAL_STORE isIndex, isEnrich Remove unused locals; only isServe drives profile selection.
analyzer/Analyzer.java:279 DLS_DEAD_LOCAL_STORE fileInventory Drop the never-read buildFileInventory(...) call and its now-orphan helper + unused imports.
analyzer/Analyzer.java:388-389 DLS_DEAD_LOCAL_STORE flushed, recoveredEdges Drop the locals but keep flush() / flushDeferred() calls for their side effects (provenance stamping, dropped-edge counter).
cache/AnalysisCache.java:114, 151 CT_CONSTRUCTOR_THROW x2 Mark class final to neutralize finalizer-subclass attack on partially constructed instance. conn is already final.
cli/EnrichCommand.java:153-154 DLS_DEAD_LOCAL_STORE flushed, recoveredEdges Same as Analyzer -- drop locals, keep calls.
cli/EnrichCommand.java:425 REC_CATCH_EXCEPTION Narrow catch (Exception) to catch (IOException | RuntimeException).
detector/frontend/ReactComponentDetector.java:113 UC_USELESS_OBJECT allDetected Delete -- never queried.
detector/go/GoStructuresDetector.java:142 UC_USELESS_OBJECT methodPositions Delete -- never read; methodStarts is built from scratch lower down.
query/TopologyService.java:198 UC_USELESS_OBJECT edgeKinds Delete the map and its put -- blast-radius never uses it.
analyzer/FileDiscovery.java:167 DE_MIGHT_IGNORE (pre-existing, masked by the 12 above) Log at debug before returning instead of silently swallowing Files.size() IOException.

Test plan

  • mvn -B -DskipTests=true generate-sources resources:resources compiler:compile spotbugs:checkBugInstance size is 0, BUILD SUCCESS on this branch.
  • Same command on main (cab71a4) → fails with 12 bugs. Confirms the branch is the thing that closes the gap.
  • Pure refactor: no behavioral change — side effects of builder.flush() / flushDeferred() preserved (comments in-diff explain why the calls stay).
  • spotbugs-exclude.xml untouched.

Follow-up: once merged, RAN-24 can bind spotbugs:check to mvn verify so CI enforces this going forward.

🤖 Generated with Claude Code

Resolves RAN-23.

| File | Finding | Fix |
|---|---|---|
| CodeIqApplication.java:57-58 | DLS_DEAD_LOCAL_STORE (isIndex, isEnrich) | Remove unused locals; only isServe drives profile selection. |
| analyzer/Analyzer.java:279 | DLS_DEAD_LOCAL_STORE (fileInventory) | Drop the never-read buildFileInventory call and its now-orphan helper + unused imports. |
| analyzer/Analyzer.java:388-389 | DLS_DEAD_LOCAL_STORE (flushed, recoveredEdges) | Drop the locals but keep flush()/flushDeferred() calls for their side effects (provenance stamping, dropped-edge counter). |
| cache/AnalysisCache.java:114,151 | CT_CONSTRUCTOR_THROW x2 | Mark class final to neutralize finalizer-subclass attack on partially constructed instance. |
| cli/EnrichCommand.java:153-154 | DLS_DEAD_LOCAL_STORE (flushed, recoveredEdges) | Same as Analyzer -- drop locals, keep calls. |
| cli/EnrichCommand.java:425 | REC_CATCH_EXCEPTION | Narrow catch(Exception) to catch(IOException \| RuntimeException). |
| detector/frontend/ReactComponentDetector.java:113 | UC_USELESS_OBJECT (allDetected) | Delete -- never queried. |
| detector/go/GoStructuresDetector.java:142 | UC_USELESS_OBJECT (methodPositions) | Delete -- never read; methodStarts is built from scratch lower down. |
| query/TopologyService.java:198 | UC_USELESS_OBJECT (edgeKinds) | Delete the map and its put -- blast-radius never uses it. |

Also fixes a pre-existing DE_MIGHT_IGNORE at FileDiscovery.java:167 that
SpotBugs only surfaced once the original 12 cleared (empty catch on
Files.size() IOException).

No changes to spotbugs-exclude.xml.

Verification:
- mvn spotbugs:check -- BugInstance size is 0 (BUILD SUCCESS).
- Unblocks RAN-24 (binding spotbugs:check to mvn verify).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@aksOps
Copy link
Copy Markdown
Contributor Author

aksOps commented Apr 24, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@aksOps aksOps merged commit 8f1ce18 into main Apr 24, 2026
7 of 8 checks passed
@aksOps aksOps deleted the fix/ran-23-spotbugs-cleanup branch April 24, 2026 21:31
aksOps added a commit that referenced this pull request Apr 25, 2026
… (RAN-41) (#73)

The 313-line enrichFromCache method scored cognitive complexity 94 on
SonarCloud (S3776, allowed 15) when PR #71 (RAN-23) re-classified it as
new code. Even small future edits in this file would re-trip the
new-code quality gate.

Extracted phases into private helpers, preserving exact ordering,
side effects, log lines, exit codes, and determinism:

  - applyGraphDirOverride        — --graph option handling
  - runLinkerPhase               — GraphBuilder + cross-file linkers
  - runClassifierAndEnrichers    — layer / lexical / language enrichment
  - runServiceDetection          — ServiceDetector + node/edge merge
  - bulkLoadIntoNeo4j            — top-level Neo4j orchestration
    - clearGraph                 — batched DETACH DELETE
    - bulkLoadNodes              — UNWIND CREATE in NODE_BATCH_SIZE batches
    - createPrimaryIndex         — id index + awaitIndexes
    - createStubNodesForExternalRefs / flushStubBatches / writeStubBatch
    - buildValidEdgeMaps / toEdgeProps
    - bulkLoadEdges              — UNWIND MATCH/CREATE batches
    - createSecondaryIndexes     — kind/layer/module/path/fulltext indexes
    - printCompletionSummary
    - shutdownQuietly            — finally-block dbms shutdown
  - logStepDone                  — shared "Done in Xms" line

Also lifted magic numbers (batch sizes, await timeout, progress interval,
stub-merge Cypher) into named constants and replaced fully-qualified
references (java.util.HashSet, ServiceDetector, ObjectMapper) with
imports for readability.

After refactor each method's cognitive complexity is well under 15
(orchestrator at ~1, largest helper at ~11).

Verification:
- mvn test -Dskip.npm=true → 3395 tests, 0 failures, 0 errors
- mvn spotbugs:check       → 0 findings
- EnrichCommandTest (4)    → still green

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant