Skip to content

Report const cycle diagnostic per node to match class convention#1681

Merged
TwitchBronBron merged 1 commit intomasterfrom
fix/const-cycle-diagnostic-per-node
Apr 27, 2026
Merged

Report const cycle diagnostic per node to match class convention#1681
TwitchBronBron merged 1 commit intomasterfrom
fix/const-cycle-diagnostic-per-node

Conversation

@chrisdp
Copy link
Copy Markdown
Contributor

@chrisdp chrisdp commented Apr 27, 2026

Summary

Follow-up to #1680. The const cycle diagnostic added there reported each cycle once (deduped via the lexicographically-smallest fullName). The class hierarchy diagnostic in ClassValidator.detectCircularReferences reports an N-cycle as N diagnostics — one rooted at each member, with the chain rotated so the diagnostic's class is at the head. See BrsFile.Class.spec.ts:1215-1217 for the existing 3-cycle assertion.

This PR drops the dedup so consts emit the same shape, giving users a consistent diagnostic experience across the two cycle types.

Changes

  • ScopeValidator.detectCircularConstReferences: removed the reportedCycleStarts and fullyValidated sets. Each const starts a fresh DFS; cycles are reported on every member with the chain rotated.
  • Updated three const cycle tests + one Enum test that incidentally uses a circular const setup.

Test plan

  • npx mocha — 2633/2633 pass, 0 regressions
  • npx eslint clean on touched files

🤖 Generated with Claude Code

Drop the per-cycle dedup so an N-cycle of consts produces N diagnostics,
each rooted at a different cycle member with the chain rotated to start
at that member. This matches the existing class-hierarchy circular
reference detection (see ClassValidator.detectCircularReferences and
BrsFile.Class.spec.ts), so users see a uniform diagnostic shape across
the two cycle types.
@TwitchBronBron TwitchBronBron merged commit 381956c into master Apr 27, 2026
5 checks passed
@TwitchBronBron TwitchBronBron deleted the fix/const-cycle-diagnostic-per-node branch April 27, 2026 18: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.

2 participants