From aaf94efc6e21b40657b21cae029a3ff9d5bc06af Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Sun, 3 May 2026 09:35:20 -0300 Subject: [PATCH 1/2] Recover from mismatched loop terminators with quick fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a `while` block has no matching `end while` and the next dangling terminator is a `next`, treat it as a malformed `end while`. Symmetric treatment for `for`/`for each` blocks closed with a stray `end while`. The previous behavior was a cascade of two unhelpful diagnostics (`Unexpected token` + `Could not find matching end `) per case. Now each case produces one targeted diagnostic: - 1147 `Expected 'end while' but found 'next'` - 1148 `Expected 'end for' or 'next' but found 'end while'` Quick fixes are attached to each diagnostic (with fix-all variants when the same mistake appears multiple times in the file): - `while ... next` → `Convert 'next' to 'end while'` - `for ... end while` → `Convert 'end while' to 'end for'` (preferred) or `Convert 'end while' to 'next'` - `for each ... end while` → same two fixes as `for` --- src/DiagnosticMessages.ts | 10 ++ .../codeActions/CodeActionsProcessor.spec.ts | 140 ++++++++++++++++++ .../codeActions/CodeActionsProcessor.ts | 59 +++++++- src/parser/Parser.ts | 47 ++++-- src/parser/tests/controlFlow/For.spec.ts | 50 +++++++ src/parser/tests/controlFlow/ForEach.spec.ts | 39 +++++ src/parser/tests/controlFlow/While.spec.ts | 53 +++++++ 7 files changed, 385 insertions(+), 13 deletions(-) diff --git a/src/DiagnosticMessages.ts b/src/DiagnosticMessages.ts index 12357a6a3..147aae89d 100644 --- a/src/DiagnosticMessages.ts +++ b/src/DiagnosticMessages.ts @@ -770,6 +770,16 @@ export let DiagnosticMessages = { message: `'${featureName}' requires Roku firmware version ${minimumVersion} or higher (current target is ${configuredVersion})`, code: 1146, severity: DiagnosticSeverity.Error + }), + whileLoopTerminatedWithNext: () => ({ + message: `Expected 'end while' but found 'next'`, + code: 1147, + severity: DiagnosticSeverity.Error + }), + forLoopTerminatedWithEndWhile: () => ({ + message: `Expected 'end for' or 'next' but found 'end while'`, + code: 1148, + severity: DiagnosticSeverity.Error }) }; diff --git a/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts b/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts index 6a237bd5c..2c97851ad 100644 --- a/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts +++ b/src/bscPlugin/codeActions/CodeActionsProcessor.spec.ts @@ -1000,4 +1000,144 @@ describe('CodeActionsProcessor', () => { expect(changes[0].range.start.character).to.equal(changes[0].range.end.character - 9); }); }); + + describe('mismatched loop terminator', () => { + it('offers a single quick fix for `while ... next`', () => { + const file = program.setFile('source/main.bs', ` + sub a() + while n <= 3 + n = n + 1 + next + end sub + `); + // cursor on the bogus `next` (line 4) + testGetCodeActions(file, util.createRange(4, 20, 4, 20), [`Convert 'next' to 'end while'`]); + }); + + it('replaces only the bogus `next` token, preserving indentation', () => { + const file = program.setFile('source/main.bs', ` + sub a() + while n <= 3 + n = n + 1 + next + end sub + `); + program.validate(); + const actions = program.getCodeActions(file.srcPath, util.createRange(4, 20, 4, 20)); + const fix = actions.find(a => a.title === `Convert 'next' to 'end while'`); + const changes = Object.values(fix!.edit!.changes!)[0]; + expect(changes).to.be.lengthOf(1); + expect(changes[0].newText).to.equal('end while'); + //the replace range is exactly the 4-character `next` token + expect(changes[0].range.end.character - changes[0].range.start.character).to.equal(4); + }); + + it('marks the `end while` quick fix as preferred for `while ... next`', () => { + const file = program.setFile('source/main.bs', ` + sub a() + while n <= 3 + n = n + 1 + next + end sub + `); + program.validate(); + const actions = program.getCodeActions(file.srcPath, util.createRange(4, 20, 4, 20)); + const fix = actions.find(a => a.title === `Convert 'next' to 'end while'`); + expect(fix!.isPreferred).to.equal(true); + }); + + it('offers a fix-all for multiple `while ... next` instances in the same file', () => { + const file = program.setFile('source/main.bs', ` + sub a() + while x + x = false + next + while y + y = false + next + end sub + `); + //cursor on the first bogus `next` + testGetCodeActions(file, util.createRange(4, 20, 4, 20), [ + `Convert 'next' to 'end while'`, + `Fix all: Convert 'next' to 'end while'` + ]); + }); + + it('offers two quick fixes (end for preferred, next) for `for ... end while`', () => { + const file = program.setFile('source/main.bs', ` + sub a() + for i = 0 to 3 + print i + end while + end sub + `); + testGetCodeActions(file, util.createRange(4, 20, 4, 20), [ + `Convert 'end while' to 'end for'`, + `Convert 'end while' to 'next'` + ]); + }); + + it('marks the `end for` quick fix as preferred and `next` as non-preferred', () => { + const file = program.setFile('source/main.bs', ` + sub a() + for i = 0 to 3 + print i + end while + end sub + `); + program.validate(); + const actions = program.getCodeActions(file.srcPath, util.createRange(4, 20, 4, 20)); + const endForFix = actions.find(a => a.title === `Convert 'end while' to 'end for'`); + const nextFix = actions.find(a => a.title === `Convert 'end while' to 'next'`); + expect(endForFix!.isPreferred).to.equal(true); + expect(nextFix!.isPreferred).to.not.equal(true); + }); + + it('offers two quick fixes for `for each ... end while`', () => { + const file = program.setFile('source/main.bs', ` + sub a() + for each x in arr + print x + end while + end sub + `); + testGetCodeActions(file, util.createRange(4, 20, 4, 20), [ + `Convert 'end while' to 'end for'`, + `Convert 'end while' to 'next'` + ]); + }); + + it('offers fix-all variants for both end for and next when there are multiple `for ... end while` instances', () => { + const file = program.setFile('source/main.bs', ` + sub a() + for i = 0 to 3 + print i + end while + for j = 0 to 3 + print j + end while + end sub + `); + //cursor on the first bogus `end while` + testGetCodeActions(file, util.createRange(4, 20, 4, 20), [ + `Convert 'end while' to 'end for'`, + `Convert 'end while' to 'next'`, + `Fix all: Convert 'end while' to 'end for'`, + `Fix all: Convert 'end while' to 'next'` + ]); + }); + + it('does not offer the quick fix for a valid `for ... next`', () => { + const file = program.setFile('source/main.bs', ` + sub a() + for i = 0 to 3 + print i + next + end sub + `); + //cursor on the valid `next` + testGetCodeActions(file, util.createRange(4, 20, 4, 20), []); + }); + }); }); diff --git a/src/bscPlugin/codeActions/CodeActionsProcessor.ts b/src/bscPlugin/codeActions/CodeActionsProcessor.ts index 0ab7dbdad..2acd84e09 100644 --- a/src/bscPlugin/codeActions/CodeActionsProcessor.ts +++ b/src/bscPlugin/codeActions/CodeActionsProcessor.ts @@ -51,6 +51,10 @@ export class CodeActionsProcessor { this.suggestMissingOverrideQuickFixes([diagnostic]); } else if (diagnostic.code === DiagnosticCodeMap.cannotUseOverrideKeywordOnConstructorFunction) { this.suggestRemoveOverrideFromConstructorQuickFixes([diagnostic]); + } else if (diagnostic.code === DiagnosticCodeMap.whileLoopTerminatedWithNext) { + this.suggestWhileTerminatorQuickFixes([diagnostic]); + } else if (diagnostic.code === DiagnosticCodeMap.forLoopTerminatedWithEndWhile) { + this.suggestForTerminatorQuickFixes([diagnostic]); } } @@ -78,6 +82,10 @@ export class CodeActionsProcessor { this.suggestScriptImportCasingQuickFixes(allInFile as DiagnosticMessageType<'scriptImportCaseMismatch'>[]); } else if (code === DiagnosticCodeMap.missingOverrideKeyword) { this.suggestMissingOverrideQuickFixes(allInFile); + } else if (code === DiagnosticCodeMap.whileLoopTerminatedWithNext) { + this.suggestWhileTerminatorQuickFixes(allInFile); + } else if (code === DiagnosticCodeMap.forLoopTerminatedWithEndWhile) { + this.suggestForTerminatorQuickFixes(allInFile); } } } @@ -560,6 +568,52 @@ export class CodeActionsProcessor { ); } + /** + * Builds a list of replace-changes that replace each diagnostic's range with `newText`. + */ + private buildTerminatorReplaceChanges(diagnostics: Diagnostic[], newText: string): ReplaceChange[] { + return diagnostics.map(d => ({ + type: 'replace', + filePath: this.event.file.srcPath, + range: d.range, + newText: newText + })); + } + + /** + * Adds a code action to convert a bogus `next` (closing a `while`) into `end while`. + * `end while` is the only valid terminator for a `while` loop, so this offers a single fix. + */ + private suggestWhileTerminatorQuickFixes(diagnostics: Diagnostic[]) { + this.emitOrFixAll( + `Convert 'next' to 'end while'`, + `Fix all: Convert 'next' to 'end while'`, + this.buildTerminatorReplaceChanges(diagnostics, 'end while'), + diagnostics[0], + true + ); + } + + /** + * Adds two code actions to convert a bogus `end while` (closing a `for`/`for each`) into either + * `end for` (preferred) or `next`. Both are valid terminators for a `for`/`for each` loop. + */ + private suggestForTerminatorQuickFixes(diagnostics: Diagnostic[]) { + this.emitOrFixAll( + `Convert 'end while' to 'end for'`, + `Fix all: Convert 'end while' to 'end for'`, + this.buildTerminatorReplaceChanges(diagnostics, 'end for'), + diagnostics[0], + true + ); + this.emitOrFixAll( + `Convert 'end while' to 'next'`, + `Fix all: Convert 'end while' to 'next'`, + this.buildTerminatorReplaceChanges(diagnostics, 'next'), + diagnostics[0] + ); + } + /** * Adds code actions to remove the invalid `override` keyword from a constructor method. */ @@ -629,7 +683,8 @@ export class CodeActionsProcessor { singleTitle: string, fixAllTitle: string, changes: Array, - diagnostic: Diagnostic + diagnostic: Diagnostic, + isPreferred?: boolean ) { if (changes.length === 0) { return; @@ -639,6 +694,7 @@ export class CodeActionsProcessor { codeActionUtil.createCodeAction({ title: singleTitle, diagnostics: [diagnostic], + ...(isPreferred ? { isPreferred: true } : {}), kind: CodeActionKind.QuickFix, changes: changes }) @@ -647,6 +703,7 @@ export class CodeActionsProcessor { this.event.codeActions.push( codeActionUtil.createCodeAction({ title: fixAllTitle, + ...(isPreferred ? { isPreferred: true } : {}), kind: CodeActionKind.QuickFix, changes: changes }) diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index 75ba37549..9476c8904 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -1308,9 +1308,19 @@ export class Parser { this.consumeStatementSeparators(); - const whileBlock = this.block(TokenKind.EndWhile); + const whileBlock = this.block(TokenKind.EndWhile, TokenKind.Next); let endWhile: Token; - if (!whileBlock || this.peek().kind !== TokenKind.EndWhile) { + if (whileBlock && this.peek().kind === TokenKind.EndWhile) { + endWhile = this.advance(); + } else if (whileBlock && this.peek().kind === TokenKind.Next) { + //recover: a stray `next` is a common mistake when the user means `end while`. + //emit a targeted diagnostic and consume the `next` so the rest of the file parses cleanly. + this.diagnostics.push({ + ...DiagnosticMessages.whileLoopTerminatedWithNext(), + range: this.peek().range + }); + endWhile = this.advance(); + } else { this.diagnostics.push({ ...DiagnosticMessages.couldNotFindMatchingEndKeyword('while'), range: this.peek().range @@ -1318,8 +1328,6 @@ export class Parser { if (!whileBlock) { throw this.lastDiagnosticAsError(); } - } else { - endWhile = this.advance(); } return new WhileStatement( @@ -1355,9 +1363,18 @@ export class Parser { this.consumeStatementSeparators(); - let body = this.block(TokenKind.EndFor, TokenKind.Next); + let body = this.block(TokenKind.EndFor, TokenKind.Next, TokenKind.EndWhile); let endForToken: Token; - if (!body || !this.checkAny(TokenKind.EndFor, TokenKind.Next)) { + if (body && this.checkAny(TokenKind.EndFor, TokenKind.Next)) { + endForToken = this.advance(); + } else if (body && this.peek().kind === TokenKind.EndWhile) { + //recover: a stray `end while` is a common mistake when the user means `end for`. + this.diagnostics.push({ + ...DiagnosticMessages.forLoopTerminatedWithEndWhile(), + range: this.peek().range + }); + endForToken = this.advance(); + } else { this.diagnostics.push({ ...DiagnosticMessages.expectedEndForOrNextToTerminateForLoop(), range: this.peek().range @@ -1365,8 +1382,6 @@ export class Parser { if (!body) { throw this.lastDiagnosticAsError(); } - } else { - endForToken = this.advance(); } // WARNING: BrightScript doesn't delete the loop initial value after a for/to loop! It just @@ -1416,8 +1431,18 @@ export class Parser { this.consumeStatementSeparators(); - let body = this.block(TokenKind.EndFor, TokenKind.Next); - if (!body) { + let body = this.block(TokenKind.EndFor, TokenKind.Next, TokenKind.EndWhile); + let endFor: Token; + if (body && this.checkAny(TokenKind.EndFor, TokenKind.Next)) { + endFor = this.advance(); + } else if (body && this.peek().kind === TokenKind.EndWhile) { + //recover: a stray `end while` is a common mistake when the user means `end for`. + this.diagnostics.push({ + ...DiagnosticMessages.forLoopTerminatedWithEndWhile(), + range: this.peek().range + }); + endFor = this.advance(); + } else { this.diagnostics.push({ ...DiagnosticMessages.expectedEndForOrNextToTerminateForLoop(), range: this.peek().range @@ -1425,8 +1450,6 @@ export class Parser { throw this.lastDiagnosticAsError(); } - let endFor = this.advance(); - return new ForEachStatement( { forEach: forEach, diff --git a/src/parser/tests/controlFlow/For.spec.ts b/src/parser/tests/controlFlow/For.spec.ts index 0130b76d9..066809bdf 100644 --- a/src/parser/tests/controlFlow/For.spec.ts +++ b/src/parser/tests/controlFlow/For.spec.ts @@ -5,6 +5,7 @@ import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; import type { ForStatement } from '../../Statement'; import { LiteralExpression } from '../../Expression'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; describe('parser for loops', () => { it('accepts a \'step\' clause', () => { @@ -165,4 +166,53 @@ describe('parser for loops', () => { Range.create(0, 0, 2, 8) ); }); + + describe('terminator recovery', () => { + it('emits a single targeted diagnostic for `for ... end while`', () => { + const parser = Parser.parse(` + sub a() + for i = 0 to 3 + print i + end while + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(1); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.forLoopTerminatedWithEndWhile().code); + }); + + it('stores the bogus `end while` token on the ForStatement', () => { + const parser = Parser.parse(` + sub a() + for i = 0 to 3 + print i + end while + end sub + `); + const fn = parser.references.functionStatements[0]; + const forStmt = fn.func.body.statements[0] as ForStatement; + expect(forStmt.endForToken.kind).to.equal(TokenKind.EndWhile); + }); + + it('does not regress on the canonical `for ... next` form', () => { + const parser = Parser.parse(` + sub a() + for i = 0 to 3 + print i + next + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(0); + }); + + it('does not regress on the canonical `for ... end for` form', () => { + const parser = Parser.parse(` + sub a() + for i = 0 to 3 + print i + end for + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(0); + }); + }); }); diff --git a/src/parser/tests/controlFlow/ForEach.spec.ts b/src/parser/tests/controlFlow/ForEach.spec.ts index a340992fa..2f3ebdef3 100644 --- a/src/parser/tests/controlFlow/ForEach.spec.ts +++ b/src/parser/tests/controlFlow/ForEach.spec.ts @@ -6,6 +6,7 @@ import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; import { ForEachStatement } from '../../Statement'; import { VariableExpression } from '../../Expression'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; describe('parser foreach loops', () => { it('requires a name and target', () => { @@ -113,4 +114,42 @@ describe('parser foreach loops', () => { Range.create(0, 0, 2, 7) ); }); + + describe('terminator recovery', () => { + it('emits a single targeted diagnostic for `for each ... end while`', () => { + const parser = Parser.parse(` + sub a() + for each x in arr + print x + end while + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(1); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.forLoopTerminatedWithEndWhile().code); + }); + + it('stores the bogus `end while` token on the ForEachStatement', () => { + const parser = Parser.parse(` + sub a() + for each x in arr + print x + end while + end sub + `); + const fn = parser.references.functionStatements[0]; + const forEachStmt = fn.func.body.statements[0] as ForEachStatement; + expect(forEachStmt.tokens.endFor.kind).to.equal(TokenKind.EndWhile); + }); + + it('does not regress on the canonical `for each ... next` form', () => { + const parser = Parser.parse(` + sub a() + for each x in arr + print x + next + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(0); + }); + }); }); diff --git a/src/parser/tests/controlFlow/While.spec.ts b/src/parser/tests/controlFlow/While.spec.ts index c85930281..988304207 100644 --- a/src/parser/tests/controlFlow/While.spec.ts +++ b/src/parser/tests/controlFlow/While.spec.ts @@ -4,6 +4,8 @@ import { Parser } from '../../Parser'; import { TokenKind } from '../../../lexer/TokenKind'; import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; +import type { WhileStatement } from '../../Statement'; describe('parser while statements', () => { @@ -111,4 +113,55 @@ describe('parser while statements', () => { Range.create(0, 0, 2, 9) ); }); + + describe('terminator recovery', () => { + it('emits a single targeted diagnostic for `while ... next`', () => { + const parser = Parser.parse(` + sub a() + while n <= 3 + n = n + 1 + next + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(1); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.whileLoopTerminatedWithNext().code); + }); + + it('stores the bogus `next` token on the WhileStatement so a quick fix can target it', () => { + const parser = Parser.parse(` + sub a() + while n <= 3 + n = n + 1 + next + end sub + `); + const fn = parser.references.functionStatements[0]; + const whileStmt = fn.func.body.statements[0] as WhileStatement; + expect(whileStmt.tokens.endWhile.kind).to.equal(TokenKind.Next); + expect(whileStmt.tokens.endWhile.text).to.equal('next'); + }); + + it('does not flag a valid `for ... next` nested inside a while loop', () => { + const parser = Parser.parse(` + sub a() + while n <= 3 + for i = 0 to 3 + print i + next + n = n + 1 + end while + end sub + `); + expect(parser.diagnostics).to.be.lengthOf(0); + }); + + it('still reports the original diagnostic when the while runs off the end of the file', () => { + const parser = Parser.parse(` + while n <= 3 + n = n + 1 + `); + expect(parser.diagnostics).to.be.lengthOf(1); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.couldNotFindMatchingEndKeyword('while').code); + }); + }); }); From 3dd0e9b9bc168c401fbeacf6ea3b86f373735547 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Tue, 5 May 2026 16:32:07 -0300 Subject: [PATCH 2/2] Combine loop-terminator diagnostics into mismatchedEndingToken Replace whileLoopTerminatedWithNext (1147) and forLoopTerminatedWithEndWhile (1148) with a single mismatchedEndingToken diagnostic that carries { expected, found } on its data payload. Code actions read the structured data to build "Convert '' to ''" fixes, so adding new recovery sites doesn't require new diagnostic codes or new handlers. Also adds isDiagnosticOfType type guard for refining BsDiagnostic to a specific DiagnosticMessageType. --- src/DiagnosticMessages.ts | 38 +++++++--- .../codeActions/CodeActionsProcessor.ts | 76 ++++++------------- src/parser/Parser.ts | 6 +- src/parser/tests/controlFlow/For.spec.ts | 3 +- src/parser/tests/controlFlow/ForEach.spec.ts | 3 +- src/parser/tests/controlFlow/While.spec.ts | 3 +- 6 files changed, 62 insertions(+), 67 deletions(-) diff --git a/src/DiagnosticMessages.ts b/src/DiagnosticMessages.ts index 147aae89d..42d79dc36 100644 --- a/src/DiagnosticMessages.ts +++ b/src/DiagnosticMessages.ts @@ -771,16 +771,23 @@ export let DiagnosticMessages = { code: 1146, severity: DiagnosticSeverity.Error }), - whileLoopTerminatedWithNext: () => ({ - message: `Expected 'end while' but found 'next'`, - code: 1147, - severity: DiagnosticSeverity.Error - }), - forLoopTerminatedWithEndWhile: () => ({ - message: `Expected 'end for' or 'next' but found 'end while'`, - code: 1148, - severity: DiagnosticSeverity.Error - }) + /** + * Emitted when a block recovers from a wrong terminator (e.g. `while ... next` or `for ... end while`). + * `expected` lists the legal terminators in preferred-first order; `found` is the actual text. + * Quick fixes consume the structured `data` to build "Convert '' to ''" actions. + */ + mismatchedEndingToken: (expected: string[] = [], found = '') => { + const expectedList = Array.isArray(expected) ? expected : []; + return { + message: `Expected ${expectedList.map(text => `'${text}'`).join(' or ')} but found '${found}'`, + code: 1147, + data: { + expected: expectedList, + found: found + }, + severity: DiagnosticSeverity.Error + }; + } }; export const DiagnosticCodeMap = {} as Record; @@ -805,3 +812,14 @@ export type DiagnosticMessageType & //include the missing properties from BsDiagnostic Pick; + +/** + * Refines a diagnostic to its concrete `DiagnosticMessageType` shape (including the typed `data` + * payload) when its code matches `DiagnosticCodeMap[key]`. + */ +export function isDiagnosticOfType( + diagnostic: { code?: number | string }, + key: K +): diagnostic is DiagnosticMessageType { + return diagnostic.code === DiagnosticCodeMap[key]; +} diff --git a/src/bscPlugin/codeActions/CodeActionsProcessor.ts b/src/bscPlugin/codeActions/CodeActionsProcessor.ts index b3b116c81..0786f986f 100644 --- a/src/bscPlugin/codeActions/CodeActionsProcessor.ts +++ b/src/bscPlugin/codeActions/CodeActionsProcessor.ts @@ -3,7 +3,7 @@ import { CodeActionKind } from 'vscode-languageserver'; import { codeActionUtil } from '../../CodeActionUtil'; import type { DeleteChange, InsertChange, ReplaceChange } from '../../CodeActionUtil'; import type { DiagnosticMessageType } from '../../DiagnosticMessages'; -import { DiagnosticCodeMap } from '../../DiagnosticMessages'; +import { DiagnosticCodeMap, isDiagnosticOfType } from '../../DiagnosticMessages'; import type { BrsFile } from '../../files/BrsFile'; import type { XmlFile } from '../../files/XmlFile'; import type { BscFile, BsDiagnostic, OnGetCodeActionsEvent } from '../../interfaces'; @@ -53,10 +53,8 @@ export class CodeActionsProcessor { this.suggestMissingOverrideQuickFixes([diagnostic]); } else if (diagnostic.code === DiagnosticCodeMap.cannotUseOverrideKeywordOnConstructorFunction) { this.suggestRemoveOverrideFromConstructorQuickFixes([diagnostic]); - } else if (diagnostic.code === DiagnosticCodeMap.whileLoopTerminatedWithNext) { - this.suggestWhileTerminatorQuickFixes([diagnostic]); - } else if (diagnostic.code === DiagnosticCodeMap.forLoopTerminatedWithEndWhile) { - this.suggestForTerminatorQuickFixes([diagnostic]); + } else if (isDiagnosticOfType(diagnostic, 'mismatchedEndingToken')) { + this.suggestMismatchedEndingTokenQuickFixes([diagnostic]); } } @@ -84,10 +82,8 @@ export class CodeActionsProcessor { this.suggestScriptImportCasingQuickFixes(allInFile as DiagnosticMessageType<'scriptImportCaseMismatch'>[]); } else if (code === DiagnosticCodeMap.missingOverrideKeyword) { this.suggestMissingOverrideQuickFixes(allInFile); - } else if (code === DiagnosticCodeMap.whileLoopTerminatedWithNext) { - this.suggestWhileTerminatorQuickFixes(allInFile); - } else if (code === DiagnosticCodeMap.forLoopTerminatedWithEndWhile) { - this.suggestForTerminatorQuickFixes(allInFile); + } else if (code === DiagnosticCodeMap.mismatchedEndingToken) { + this.suggestMismatchedEndingTokenQuickFixes(allInFile as DiagnosticMessageType<'mismatchedEndingToken'>[]); } } } @@ -775,49 +771,27 @@ export class CodeActionsProcessor { } /** - * Builds a list of replace-changes that replace each diagnostic's range with `newText`. + * Adds one code action per legal terminator. The first entry of `expected` is marked + * `isPreferred`, matching the parser's convention of listing the canonical terminator first. */ - private buildTerminatorReplaceChanges(diagnostics: Diagnostic[], newText: string): ReplaceChange[] { - return diagnostics.map(d => ({ - type: 'replace', - filePath: this.event.file.srcPath, - range: d.range, - newText: newText - })); - } - - /** - * Adds a code action to convert a bogus `next` (closing a `while`) into `end while`. - * `end while` is the only valid terminator for a `while` loop, so this offers a single fix. - */ - private suggestWhileTerminatorQuickFixes(diagnostics: Diagnostic[]) { - this.emitOrFixAll( - `Convert 'next' to 'end while'`, - `Fix all: Convert 'next' to 'end while'`, - this.buildTerminatorReplaceChanges(diagnostics, 'end while'), - diagnostics[0], - true - ); - } - - /** - * Adds two code actions to convert a bogus `end while` (closing a `for`/`for each`) into either - * `end for` (preferred) or `next`. Both are valid terminators for a `for`/`for each` loop. - */ - private suggestForTerminatorQuickFixes(diagnostics: Diagnostic[]) { - this.emitOrFixAll( - `Convert 'end while' to 'end for'`, - `Fix all: Convert 'end while' to 'end for'`, - this.buildTerminatorReplaceChanges(diagnostics, 'end for'), - diagnostics[0], - true - ); - this.emitOrFixAll( - `Convert 'end while' to 'next'`, - `Fix all: Convert 'end while' to 'next'`, - this.buildTerminatorReplaceChanges(diagnostics, 'next'), - diagnostics[0] - ); + private suggestMismatchedEndingTokenQuickFixes(diagnostics: DiagnosticMessageType<'mismatchedEndingToken'>[]) { + const { expected, found } = diagnostics[0].data; + for (let index = 0; index < expected.length; index++) { + const replacement = expected[index]; + const changes = diagnostics.map(diagnostic => ({ + type: 'replace', + filePath: this.event.file.srcPath, + range: diagnostic.range, + newText: replacement + })); + this.emitOrFixAll( + `Convert '${found}' to '${replacement}'`, + `Fix all: Convert '${found}' to '${replacement}'`, + changes, + diagnostics[0], + index === 0 + ); + } } /** diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index f304f5b15..969f34bd0 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -1324,7 +1324,7 @@ export class Parser { //recover: a stray `next` is a common mistake when the user means `end while`. //emit a targeted diagnostic and consume the `next` so the rest of the file parses cleanly. this.diagnostics.push({ - ...DiagnosticMessages.whileLoopTerminatedWithNext(), + ...DiagnosticMessages.mismatchedEndingToken(['end while'], 'next'), range: this.peek().range }); endWhile = this.advance(); @@ -1378,7 +1378,7 @@ export class Parser { } else if (body && this.peek().kind === TokenKind.EndWhile) { //recover: a stray `end while` is a common mistake when the user means `end for`. this.diagnostics.push({ - ...DiagnosticMessages.forLoopTerminatedWithEndWhile(), + ...DiagnosticMessages.mismatchedEndingToken(['end for', 'next'], 'end while'), range: this.peek().range }); endForToken = this.advance(); @@ -1446,7 +1446,7 @@ export class Parser { } else if (body && this.peek().kind === TokenKind.EndWhile) { //recover: a stray `end while` is a common mistake when the user means `end for`. this.diagnostics.push({ - ...DiagnosticMessages.forLoopTerminatedWithEndWhile(), + ...DiagnosticMessages.mismatchedEndingToken(['end for', 'next'], 'end while'), range: this.peek().range }); endFor = this.advance(); diff --git a/src/parser/tests/controlFlow/For.spec.ts b/src/parser/tests/controlFlow/For.spec.ts index 066809bdf..8d6579f77 100644 --- a/src/parser/tests/controlFlow/For.spec.ts +++ b/src/parser/tests/controlFlow/For.spec.ts @@ -177,7 +177,8 @@ describe('parser for loops', () => { end sub `); expect(parser.diagnostics).to.be.lengthOf(1); - expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.forLoopTerminatedWithEndWhile().code); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.mismatchedEndingToken().code); + expect((parser.diagnostics[0] as any).data).to.eql({ expected: ['end for', 'next'], found: 'end while' }); }); it('stores the bogus `end while` token on the ForStatement', () => { diff --git a/src/parser/tests/controlFlow/ForEach.spec.ts b/src/parser/tests/controlFlow/ForEach.spec.ts index 2f3ebdef3..a20f2d012 100644 --- a/src/parser/tests/controlFlow/ForEach.spec.ts +++ b/src/parser/tests/controlFlow/ForEach.spec.ts @@ -125,7 +125,8 @@ describe('parser foreach loops', () => { end sub `); expect(parser.diagnostics).to.be.lengthOf(1); - expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.forLoopTerminatedWithEndWhile().code); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.mismatchedEndingToken().code); + expect((parser.diagnostics[0] as any).data).to.eql({ expected: ['end for', 'next'], found: 'end while' }); }); it('stores the bogus `end while` token on the ForEachStatement', () => { diff --git a/src/parser/tests/controlFlow/While.spec.ts b/src/parser/tests/controlFlow/While.spec.ts index 988304207..ea9966ade 100644 --- a/src/parser/tests/controlFlow/While.spec.ts +++ b/src/parser/tests/controlFlow/While.spec.ts @@ -124,7 +124,8 @@ describe('parser while statements', () => { end sub `); expect(parser.diagnostics).to.be.lengthOf(1); - expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.whileLoopTerminatedWithNext().code); + expect(parser.diagnostics[0].code).to.equal(DiagnosticMessages.mismatchedEndingToken().code); + expect((parser.diagnostics[0] as any).data).to.eql({ expected: ['end while'], found: 'next' }); }); it('stores the bogus `next` token on the WhileStatement so a quick fix can target it', () => {