Skip to content

refactor: resolve SonarCloud findings on develop - #377

Merged
HuiJun merged 7 commits into
developfrom
fix/sonarcloud-develop-findings
Sep 19, 2026
Merged

HuiJun merged 7 commits into
developfrom
fix/sonarcloud-develop-findings

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

What and why

Resolves the SonarCloud findings reported against develop, in kind and without changing behavior. Quality-gate breakers first (the nightly workflow's shell quoting, a non-volatile shared field in the fUML driver), then the plain findings across Go, shell, Python, Java, GitHub Actions and the VS Code extension, and last the cognitive-complexity findings. The branch carries current develop, so the finding set it addresses is the one reported on the latest develop scan.

Plain findings, by rule family:

  • Duplicated string literals become named constants (internal/translate/migrate, internal/exec/runtime, internal/frontend/grpc, internal/doc/docrender, tools/gen/snapshot, tools/referee/exec, tools/referee/fuml, scripts, and the earlier internal/fuml, internal/pssm, internal/stressmodel, internal/errata sites).
  • Identifiers shadowing Go builtins or reserved words (copy, real, min, any, go_, ...) are renamed.
  • Unused parameters, useless assignments, redundant != nil guards and index-then-compare patterns are dropped (strings.ContainsAny, .at(-1), optional chaining, classList over the deprecated className, Quantity | int | float over Union[...]).
  • Consecutive same-type parameters are grouped; the single-method kit.Union interface is renamed kit.Regatherer after its method; intentionally empty functions say why they are empty.
  • tools/oracle/repo.DevelopCommit resolves git with exec.LookPath before running it; the nightly VS Code packaging step runs npm ci --ignore-scripts; the shell fixtures copy "$@" into a local array before iterating.
  • queryexec.Error.Error() splits its message switch into columnMessage / sessionMessage so no switch exceeds the case limit.

One finding is deliberately left: typescript:S7747 on editors/vscode/src/diagram.ts (for (const old of Array.from(this.panels.values()))). The loop body disposes and re-creates panels in this.panels, so iterating the live Map iterator would visit the entries it inserts; the array snapshot is load-bearing and the comment above it says so.

Cognitive-complexity findings (threshold 30) are addressed by extracting focused helpers, keeping every loop and error path in place:

Was Now
lower.ToActionGraphWith (73) collectActionNodes + actionEdgeLowerer{member, initial, successionEdge, controlFlowEdge, transition, objectFlowEdge, usage, successionUsage}
lower.ToStateGraphWithEndpoints (33) StateGraph.collectRegions, StateGraph.recordRegionInitials, StateGraph.noInitialState
lower.(*stateFootprintBuilder).pseudostate fork case (32) stateFootprintBuilder.fork
runtime.(*StateExecutor).reachable (40) reachSet{add, stop, visit, settled}
runtime.(*Context).carriedVerdicts (31) carriedVerdictsOf per type
queryexec.(*executor).valueConforms (34) quantityConforms, scalarConforms, declaredByConforms
queryexec.(*executor).evaluateColumnExpression (45) rowPropertyValues, objectRowValues, carrierRowValuesverdictRowValues / stateRowValues / eventRowValues
view.(*Renderer).actionNode (35) actionEdges, successionLabel
lsp.(*debugSession).locate (34) locateStates, locateActions, nest
migrate.(*migration).visibleFrom visibility{members, imported}
migrate.(*migration).association (37) associationEnd
migrate.(*migration).feature (56) featureVisibility, featureModifiers, portPayload, featureTyping, featureRedefinitions, featureShadow, featureDefault, portPayloadLine
migrate.(*stateRegion).transition (32) triggerAccept, writeTransitionEffect
migrate.(*activity).objectFlow (31) objectFlowTarget, objectFlowSource
passes.VariableFeaturePass.Run (43) variableFeatureCheck{symbol, crossFeature, usage}
FumlExpected.main / structuredValue (Java, 36 / 33) ActivityRunner.run with an Outcome enum; structuredKind

The fUML driver source moves from scripts/fuml-driver/FumlExpected.java to scripts/fuml-driver/io/opensysml/fuml/FumlExpected.java so the path matches the io.opensysml.fuml package it declares; scripts/fuml-expected.sh and docs/project/fuml-referee.md follow.

How it was verified

  • go build ./..., go vet ./..., gofmt -l . (empty), make lint (staticcheck + gosec) and go test ./... with OPENSYSML_REQUIRE_TRAINING_CORPUS=1 OPENSYSML_REQUIRE_PILOT_CORPORA=1 pass on the branch merged with current develop.
  • gocognit -over 30 reports none of the functions SonarCloud flagged.
  • FUML_EXPECTED_OUT=... ./scripts/fuml-expected.sh regenerates the referee record from the relocated driver: 55 activities, 51 executed, byte-identical to docs/project/fuml-referee-expected.json.
  • client/python: pytest tests/test_document.py passes against a locally built sysml-grpc.
  • editors/vscode: npm run typecheck and npm test pass.
  • python3 scripts/changelog.py check and python3 scripts/check-doc-ids.py pass.

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change — pure refactor; existing tests are the contract and none changed in behavior
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved (no count moved)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Desktop check of the refactored diagram and debug surfaces, on the rebuilt extension and sysml-lsp from this branch in VS Code:

  • Action succession rendering, including the [ready] guard label (view.actionEdges / successionLabel).
  • Nested regions and fork branches; a region with no entry transition shows the "has no initial state" notice and recovers live once one is added (StateGraph.collectRegions / recordRegionInitials, stateFootprintBuilder.fork).
  • Two diagram panels follow a document rename and folder move with live edits afterwards; Shift-drag reparenting and its undo (diagram.ts, edits.ts, views.ts).
  • Debug start/step over stdio LSP: action tokens start → a → b → c → done reach completion; a state fork's active work/left/wrapper/leaf and right/checking map to drawn node IDs (debugSession.locateStates / locateActions / nest, runtime.reachSet).

The extension ships no debug adapter, so runtime highlighting in the diagram itself was not exercised.

Action successions Nested regions and fork
Action diagram with guarded succession Nested state regions and fork branches
Missing initial transition Entry transition restored
Missing initial state notice Region renders after adding entry

@HuiJun
HuiJun marked this pull request as ready for review September 18, 2026 01:11

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@devin-ai-integration
devin-ai-integration Bot force-pushed the fix/sonarcloud-develop-findings branch 8 times, most recently from 7c08602 to 881c3e2 Compare September 19, 2026 04:01
devin-ai-integration Bot and others added 2 commits September 19, 2026 04:11
Address the SonarCloud issues reported against develop without changing
behavior: duplicated string literals become named constants, shadowed
builtins are renamed, unused parameters and useless assignments are
dropped, and shell, Python and TypeScript findings are fixed in kind.

Methods over the cognitive-complexity threshold are split into focused
helpers: action-graph edge lowering (actionEdgeLowerer), state-graph
region collection and initial validation, fork footprints, reachable-set
traversal (reachSet), carried verdict collection, query value conformance,
diagram action edges, LSP debug node location, and the fUML driver's
activity runner and structured-value serialisation.

The fUML driver source moves under scripts/fuml-driver/io/opensysml/fuml/
to match its package; the committed expected record is byte-identical.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the fix/sonarcloud-develop-findings branch from 881c3e2 to 0cca971 Compare September 19, 2026 04:17
devin-ai-integration Bot and others added 5 commits September 19, 2026 21:25
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
… python client

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
@HuiJun
HuiJun merged commit f389cde into develop Sep 19, 2026
12 checks passed
@HuiJun
HuiJun deleted the fix/sonarcloud-develop-findings branch September 19, 2026 22:47
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