Skip to content

Add forLoopTerminator option and fix bogus next handling#130

Open
chrisdp wants to merge 2 commits into
masterfrom
feature/for-loop-terminator-rule
Open

Add forLoopTerminator option and fix bogus next handling#130
chrisdp wants to merge 2 commits into
masterfrom
feature/for-loop-terminator-rule

Conversation

@chrisdp

@chrisdp chrisdp commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a new forLoopTerminator formatting option ('next' | 'endfor' | 'original', default 'original') that converts validly-placed for/for each loop terminators to a consistent style. The split/combine choice for end for continues to be governed by the existing compositeKeywords option.
  • Fixes a bug where a stray next token inside a non-loop block (e.g. while, function, try) caused the formatter to silently dedent. next is now only treated as a block terminator when its enclosing block is a for or for each.
  • Adds tests covering the 5 valid loop-terminator forms, nested/mixed forms, bogus parents (while, function, sub, if, try, namespace), and all forLoopTerminator modes including interaction with compositeKeywords and keywordCase.

chrisdp added 2 commits May 3, 2026 08:28
Adds a new `forLoopTerminator` formatting option (`'next' | 'endfor' |
'original'`, default `'original'`) that normalizes `for`/`for each`
loop terminators. The split/combine choice for `end for` continues to
be governed by the existing `compositeKeywords` option.

Also fixes a bug where a stray `next` token inside a non-loop block
(e.g. `while`, `function`, `try`) caused the formatter to silently
dedent. `next` is now only treated as a block terminator when its
enclosing block is a `for` or `for each`.
The early-return guard was unreachable because Formatter.ts already
gates the call when `forLoopTerminator` is `'original'` or unset, and
the truthy checks on `endForToken`/`tokens.endFor` were unreachable
because brighterscript's types declare them non-optional. Removing
brings coverage back to 100%.

@TwitchBronBron TwitchBronBron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just need some readme entries for this one. Nice!

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