Skip to content

refactor(view): clear the Sonar findings in the draw.io writer and reader - #1707

Merged
HuiJun merged 1 commit into
developfrom
refactor/view-drawio-sonar
Oct 30, 2026
Merged

HuiJun merged 1 commit into
developfrom
refactor/view-drawio-sonar

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

What and why

SonarCloud flagged the draw.io export/import code that landed this week (#1668, #1679, #1695) on develop head 0c1fd6dbd. This clears every finding in internal/ir/view/drawio.go, drawio_import.go, drawio_import_diff.go and cmd/sysml/import_diagram_test.go by refactoring only: no emitted byte, parsed value or diagnostic changes, and no test or golden moved. The sibling files (drawio_route.go, route/, form.go, tree.go) are untouched.

Rule develop file:line Change
go:S1192 drawio.go:654, 655, 664, 668, 684, 691, 692 The writer owns its indentation once: drawioCellIndent / drawioCellBodyIndent / drawioWrappedBodyIndent and the openCell/closeCell, openObject/closeObject, openGeometry/closeGeometry helpers; the plain, wrapped, port and edge paths call them instead of repeating the literals.
go:S978 drawio.go:279 any → found (gopls rename).
godre:S8193 drawio.go:1105 if d := cx - box.X; d < best → if cx-box.X < best (checked by eye; the snippet analyzer does not run godre:*).
go:S3776 drawio_import.go:160 (46) drawioPages keeps the decoder loop; the per-token state machine is drawioPageScanner with token/start/end/chars/encode.
go:S3776 drawio_import.go:286 (66) readDrawioModel keeps the decoder loop; drawioModelReader has start/end per token kind and openCell/geometry/rectangle/point per element.
go:S1192 drawio_import.go:178 One constructor, drawioMalformedXML(err), for the five "malformed XML" errors.
go:S978 drawio_import_diff.go:819 real → realPath.
go:S3776 drawio_import_diff.go:217 (33) drawioMatcher.match indexes the baseline through baselineIndex.
go:S978 import_diagram_test.go:125 real → realPath.

No dispositions; everything is fixed in code. No changelog fragment: refactor only.

How it was verified

Pull requests get no Sonar analysis; the proof that the findings are gone is the post-merge branch analysis of develop. Pre-verified locally with the project's own quality profile (analyze_code_snippet, language go) on each develop file as a control and on this branch's file:

develop internal/ir/view/drawio.go            go:S1192 ×7 (654, 655, 664, 668, 684, 691, 692), go:S978 (279)
branch  internal/ir/view/drawio.go            (no issues)
develop internal/ir/view/drawio_import.go     go:S1192 (178), go:S3776 (160), go:S3776 (286)
branch  internal/ir/view/drawio_import.go     (no issues)
develop internal/ir/view/drawio_import_diff.go  go:S3776 (217), go:S978 (819)
branch  internal/ir/view/drawio_import_diff.go  (no issues)
develop cmd/sysml/import_diagram_test.go      go:S978 (125)
branch  cmd/sysml/import_diagram_test.go      (no issues)
$ gocognit -over 30 internal/ir/view/drawio.go internal/ir/view/drawio_import.go internal/ir/view/drawio_import_diff.go
(nothing)

The draw.io goldens and TestDrawioRoundTrip pass unchanged (run without -race locally; CI runs the race suite):

$ go test -count=1 ./internal/ir/view/... ./cmd/sysml/...
ok  	github.com/Open-MBEE/OpenSysML/internal/ir/view	61.721s
ok  	github.com/Open-MBEE/OpenSysML/internal/ir/view/route	34.976s
ok  	github.com/Open-MBEE/OpenSysML/cmd/sysml	314.037s

Gate:

$ go build ./...     (clean)
$ go vet ./...       (clean)
$ gofmt -l .         (empty)
$ go test ./...      FAIL: cmd/sysml, internal/exec/runtime and tests/codegen hit Go's default
                     10-minute per-package cap while the whole suite ran 8-wide on this machine
                     (`panic: test timed out after 10m0s`); every other package ok
$ go test -count=1 -timeout 45m ./cmd/sysml/ ./internal/exec/runtime/ ./tests/codegen/
ok  	github.com/Open-MBEE/OpenSysML/cmd/sysml	499.384s
ok  	github.com/Open-MBEE/OpenSysML/internal/exec/runtime	316.740s
ok  	github.com/Open-MBEE/OpenSysML/tests/codegen	562.172s

Checklist

  • make test and make lint pass locally (go build, go vet, gofmt -l, go test above)
  • Tests added or updated for the change — none needed: the existing goldens and round-trip tests pin the behaviour
  • Documentation extended where it already covers the surface — no user-facing surface changed
  • Changelog entry — none: refactor only
  • 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

…ader

The draw.io writer owns its cell indentation through openCell/closeCell,
openObject/closeObject and openGeometry/closeGeometry instead of repeating
the indentation literals; the page scanner and model reader of the import
are a struct each with one method per token kind and per element, so no
function passes the cognitive-complexity threshold; the matcher indexes the
baseline in a helper of its own; identifiers that shadowed the predeclared
any and real are renamed. The emitted bytes and every golden are unchanged.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 30, 2026 00:24

@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

@HuiJun
HuiJun merged commit 42f7639 into develop Oct 30, 2026
33 of 54 checks passed
@HuiJun
HuiJun deleted the refactor/view-drawio-sonar branch October 30, 2026 02:12
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