From fbf7221f2e7a12cb1121df46755e9f87b97b5924 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Sat, 28 Mar 2026 16:42:37 -0300 Subject: [PATCH 01/20] first pass at cdata parsing and passing down the program --- src/Program.ts | 11 ++ src/ProgramBuilder.ts | 5 + src/files/BrsFile.ts | 6 + src/files/XmlFile.spec.ts | 213 ++++++++++++++++++++++++++++++++++++ src/files/XmlFile.ts | 67 +++++++++++- src/parser/SGParser.spec.ts | 6 +- src/parser/SGTypes.ts | 10 ++ 7 files changed, 309 insertions(+), 9 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index 6f1167f67..caeb51e78 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -469,6 +469,17 @@ export class Program { file = xmlFile; + //register synthetic BrsFiles for any inline CDATA script blocks. + //these are treated as first-class files so all plugins (linters, validators, etc.) see them normally. + let cdataScriptIndex = 0; + for (const script of xmlFile.ast.component?.scripts ?? []) { + if (script.cdata) { + const inlinePkgPath = xmlFile.inlineScriptPkgPaths[cdataScriptIndex++]; + const inlineFile = this.setFile(inlinePkgPath, script.cdataText ?? ''); + inlineFile.isSynthetic = true; + } + } + //create a new scope for this xml file let scope = new XmlScope(xmlFile, this); this.addScope(scope); diff --git a/src/ProgramBuilder.ts b/src/ProgramBuilder.ts index 9e13ccf85..887ed546e 100644 --- a/src/ProgramBuilder.ts +++ b/src/ProgramBuilder.ts @@ -250,6 +250,11 @@ export class ProgramBuilder { throw new Error('Internal invariant exception: somehow file watcher ran before `ProgramBuilder.run()`'); } thePath = s`${path.resolve(this.rootDir, thePath)}`; + //ignore file events for synthetic files (e.g. extracted CDATA scripts) — they are managed + //programmatically by XmlFile and don't exist on disk in the source directory + if (this.program.getFile(thePath)?.isSynthetic) { + return; + } if (event === 'add' || event === 'change') { const fileObj = { src: thePath, diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index d8b86cf45..1cfb35b28 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -79,6 +79,12 @@ export class BrsFile { this.srcPath = value; } + /** + * When true, this file was generated from inline CDATA content in an XML file and is not backed by a file on disk. + * The file watcher should ignore change events for paths that match a synthetic file. + */ + public isSynthetic = false; + /** * Will this file result in only comment or whitespace output? If so, it can be excluded from the output if that bsconfig setting is enabled. */ diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index e8726a7e8..5bf2a8f90 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -1324,4 +1324,217 @@ describe('XmlFile', () => { expect(program.getComponent('comp1')!.file.pkgPath).to.equal(comp2.pkgPath); }); }); + + describe('inline CDATA scripts', () => { + + it('registers a synthetic BrsFile for a brightscript CDATA block', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + expect(xmlFile.inlineScriptPkgPaths).to.eql(['components/MyComp.cdata-0.script.brs']); + const syntheticFile = program.getFile('components/MyComp.cdata-0.script.brs'); + expect(syntheticFile).to.exist; + expect(syntheticFile.isSynthetic).to.be.true; + }); + + it('registers a synthetic .bs file when type is text/brighterscript', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + expect(xmlFile.inlineScriptPkgPaths).to.eql(['components/MyComp.cdata-0.script.bs']); + expect(program.getFile('components/MyComp.cdata-0.script.bs')).to.exist; + }); + + it('registers a synthetic .bs file when type is text/bs', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + expect(xmlFile.inlineScriptPkgPaths).to.eql(['components/MyComp.cdata-0.script.bs']); + expect(program.getFile('components/MyComp.cdata-0.script.bs')).to.exist; + }); + + it('does NOT treat text/brightscript as brighterscript', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + expect(xmlFile.inlineScriptPkgPaths[0]).to.equal('components/MyComp.cdata-0.script.brs'); + }); + + it('indexes multiple CDATA blocks independently', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + + `); + expect(xmlFile.inlineScriptPkgPaths).to.eql([ + 'components/MyComp.cdata-0.script.brs', + 'components/MyComp.cdata-1.script.bs' + ]); + expect(program.getFile('components/MyComp.cdata-0.script.brs')).to.exist; + expect(program.getFile('components/MyComp.cdata-1.script.bs')).to.exist; + }); + + it('transpile replaces CDATA blocks with uri script tags', () => { + testTranspile(trim` + + + + + `, trim` + + + + + `, trim` + + + + + `); + expect(program.getFile('components/MyComp.cdata-0.script.brs')).to.exist; + program.removeFile(s`${rootDir}/components/MyComp.xml`); + expect(program.getFile('components/MyComp.cdata-0.script.brs')).to.not.exist; + }); + + it('replaces old synthetic files when the xml file is re-parsed', () => { + program.setFile('components/MyComp.xml', trim` + + + + + `); + const firstFile = program.getFile('components/MyComp.cdata-0.script.brs'); + expect(firstFile).to.exist; + + //re-set with different cdata content + program.setFile('components/MyComp.xml', trim` + + + + + `); + const secondFile = program.getFile('components/MyComp.cdata-0.script.brs'); + expect(secondFile).to.exist; + //a new BrsFile object should have been created + expect(secondFile).to.not.equal(firstFile); + }); + + it('includes the synthetic file in scope dependencies', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + const deps = xmlFile.getOwnDependencies(); + expect(deps.some(d => d.includes('cdata-0.script.brs'))).to.be.true; + }); + + it('validates code inside CDATA blocks', () => { + program.setFile('components/MyComp.xml', trim` + + + + + `); + program.validate(); + //the synthetic file should have been validated — unknown function call should produce a diagnostic + const syntheticFile = program.getFile('components/MyComp.cdata-0.script.brs'); + expect(syntheticFile).to.exist; + const allDiagnostics = program.getDiagnostics(); + //there should be a diagnostic about the unknown function + expect(allDiagnostics.some(d => d.file.pkgPath === syntheticFile.pkgPath)).to.be.true; + }); + + it('sets needsTranspiled on the xml file when CDATA is present', () => { + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + expect(xmlFile.needsTranspiled).to.be.true; + }); + }); }); diff --git a/src/files/XmlFile.ts b/src/files/XmlFile.ts index e3172998a..a6f41f820 100644 --- a/src/files/XmlFile.ts +++ b/src/files/XmlFile.ts @@ -75,6 +75,12 @@ export class XmlFile { */ readonly canBePruned = false; + /** + * XML files are never synthetically generated; this is always false. + * Exists to satisfy the BscFile union type alongside BrsFile.isSynthetic. + */ + readonly isSynthetic = false; + /** * The list of script imports delcared in the XML of this file. * This excludes parent imports and auto codebehind imports @@ -168,6 +174,24 @@ export class XmlFile { //TODO implement the xml CDATA parsing, which would populate this list public functionCalls = [] as FunctionCall[]; + /** + * The pkg paths of synthetic BrsFiles generated from inline CDATA script blocks in this file. + * Populated during `parse()`. Used by Program.setFile() to register the files, by + * attachDependencyGraph() to add them as dependencies, by transpile() to replace CDATA + * with uri script tags, and by dispose() to clean them up. + */ + public inlineScriptPkgPaths: string[] = []; + + /** + * Derive the pkg path for a synthetic file extracted from a CDATA block. + * Format: `components/MyComp.cdata-0.script.brs` (or `.bs` for brighterscript type). + */ + private getInlineScriptPkgPath(cdataIndex: number, scriptType: string | undefined): string { + const isBrighterScript = /brighterscript|text\/bs\b/i.test(scriptType ?? ''); + const ext = isBrighterScript ? '.bs' : '.brs'; + return this.pkgPath.replace(/\.xml$/i, `.cdata-${cdataIndex}.script${ext}`); + } + public functionScopes = [] as FunctionScope[]; /** @@ -209,6 +233,7 @@ export class XmlFile { */ public parse(fileContents: string) { this.fileContents = fileContents; + this.inlineScriptPkgPaths = []; this.parser.parse(this.pkgPath, fileContents); this.diagnostics = this.parser.diagnostics.map(diagnostic => ({ @@ -222,6 +247,16 @@ export class XmlFile { this.needsTranspiled = this.needsTranspiled || this.ast.component?.scripts?.some( script => script.type?.indexOf('brighterscript') > 0 || script.uri?.endsWith('.bs') ); + + //collect inline CDATA scripts and assign them synthetic pkg paths + let cdataIndex = 0; + for (const script of this.ast.component?.scripts ?? []) { + if (script.cdata) { + this.inlineScriptPkgPaths.push(this.getInlineScriptPkgPath(cdataIndex++, script.type)); + //any CDATA script requires the XML to be rewritten (CDATA → uri tag) + this.needsTranspiled = true; + } + } } /** @@ -270,7 +305,8 @@ export class XmlFile { }); let dependencies = [ - ...this.scriptTagImports.map(x => x.pkgPath.toLowerCase()) + ...this.scriptTagImports.map(x => x.pkgPath.toLowerCase()), + ...this.inlineScriptPkgPaths.map(p => p.toLowerCase()) ]; //if autoImportComponentScript is enabled, add the .bs and .brs files with the same name if (this.program.options.autoImportComponentScript) { @@ -434,11 +470,16 @@ export class XmlFile { return map; }, {}); - //if the XML already has this import, skip this one - let alreadyThereScriptImportMap = this.scriptTagImports.reduce((map, fileReference) => { + //if the XML already has this import (either a uri script tag or an inline CDATA script), skip this one + let alreadyThereScriptImportMap = this.scriptTagImports.reduce>((map, fileReference) => { map[fileReference.pkgPath.toLowerCase()] = true; return map; }, {}); + for (const pkgPath of this.inlineScriptPkgPaths) { + alreadyThereScriptImportMap[pkgPath.toLowerCase()] = true; + //also mark the .brs variant so a .bs inline script doesn't get added as an extra import + alreadyThereScriptImportMap[pkgPath.toLowerCase().replace(/\.bs$/, '.brs')] = true; + } let resultMap = {}; let result = [] as string[]; @@ -489,6 +530,20 @@ export class XmlFile { const state = new TranspileState(this.srcPath, this.program.options); const originalScripts = this.ast.component?.scripts ?? []; + + //replace CDATA script blocks with uri-based script tags pointing to the synthetic extracted files + let cdataIndex = 0; + const scriptsWithInlineUris = originalScripts.map(script => { + if (script.cdata) { + const inlinePkgPath = this.inlineScriptPkgPaths[cdataIndex++]; + const uriScript = new SGScript(); + uriScript.type = 'text/brightscript'; + uriScript.uri = util.getRokuPkgPath(inlinePkgPath.replace(/\.bs$/i, '.brs')); + return uriScript; + } + return script; + }); + const extraImportScripts = this.getMissingImportsForTranspile().map(uri => { const script = new SGScript(); script.uri = util.getRokuPkgPath(uri.replace(/\.bs$/, '.brs')); @@ -496,7 +551,7 @@ export class XmlFile { }); const [scriptsHaveChanged, publishableScripts] = this.checkScriptsForPublishableImports([ - ...originalScripts, + ...scriptsWithInlineUris, ...extraImportScripts ]); @@ -537,5 +592,9 @@ export class XmlFile { public dispose() { //unsubscribe from any DependencyGraph subscriptions this.unsubscribeFromDependencyGraph?.(); + //clean up any synthetic BrsFiles that were generated from CDATA blocks in this file + for (const pkgPath of this.inlineScriptPkgPaths) { + this.program.removeFile(pkgPath); + } } } diff --git a/src/parser/SGParser.spec.ts b/src/parser/SGParser.spec.ts index 22f7f451e..f3d2717e5 100644 --- a/src/parser/SGParser.spec.ts +++ b/src/parser/SGParser.spec.ts @@ -63,11 +63,7 @@ describe('SGParser', () => { + + // line 10: + const cdataStartLine = 2; + + let xmlFile: XmlFile; + beforeEach(() => { + xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + program.validate(); + }); + + it('getCompletions returns BrightScript completions inside CDATA', () => { + // position cursor at `name` usage on line 4 col 20 (inside "name") + const completions = program.getCompletions(xmlFile.srcPath, Position.create(4, 20)); + expect(completions.map(c => c.label)).to.include('name'); + }); + + it('getCompletions returns empty outside CDATA', () => { + // position inside the tag (line 1), not in CDATA + const completions = program.getCompletions(xmlFile.srcPath, Position.create(1, 5)); + expect(completions).to.be.empty; + }); + + it('getHover returns hover info for a function inside CDATA', () => { + // hover over `greet` at line 3 col 12 + const hovers = program.getHover(xmlFile.srcPath, Position.create(3, 12)); + expect(hovers).to.have.length.greaterThan(0); + }); + + it('getHover remaps the hover range to XML coordinates', () => { + // hover over `greet` at line 3 col 12 + const hovers = program.getHover(xmlFile.srcPath, Position.create(3, 12)); + for (const hover of hovers) { + if (hover.range) { + // range must be within the CDATA block in XML coordinates, not synthetic line 0 + expect(hover.range.start.line).to.be.at.least(cdataStartLine); + } + } + }); + + it('getDefinition returns a location inside the XML file for a symbol in CDATA', () => { + // go-to-definition on `greet` reference (line 3, col 12 is the definition site itself) + // use getCount call from synthetic line 0 edge case — instead hover greet on its def line + const locations = program.getDefinition(xmlFile.srcPath, Position.create(3, 12)); + expect(locations).to.have.length.greaterThan(0); + for (const loc of locations) { + // all returned locations must use the xml file URI + expect(loc.uri).to.include('MyComp.xml'); + // and be within the CDATA block line range + expect(loc.range.start.line).to.be.at.least(cdataStartLine); + } + }); + + it('getReferences returns locations in XML coordinates', () => { + // find references of the `name` parameter — position inside the word (not at the start) + // line 3: ` sub greet(name as string)` — `name` starts at col 18, use col 20 (inside) + const refs = program.getReferences(xmlFile.srcPath, Position.create(3, 20)); + expect(refs).to.have.length.greaterThan(0); + for (const ref of refs) { + expect(ref.uri).to.include('MyComp.xml'); + expect(ref.range.start.line).to.be.at.least(cdataStartLine); + } + }); + + it('getSemanticTokens returns tokens with XML-file coordinates', () => { + // Use a .bs CDATA block with namespace+class to generate semantic tokens + const bsXmlFile = program.setFile('components/BsComp.xml', trim` + + + + + `); + program.validate(); + const tokens = program.getSemanticTokens(bsXmlFile.srcPath); + expect(tokens).to.exist; + expect(tokens).to.have.length.greaterThan(0); + for (const token of tokens) { + // all tokens must be inside the CDATA block (line 2+ in XML) + expect(token.range.start.line).to.be.at.least(cdataStartLine + 1); + } + }); + + it('getDocumentSymbols returns function symbols with XML-file coordinates', () => { + const symbols = program.getDocumentSymbols(xmlFile.srcPath); + expect(symbols).to.exist; + const names = symbols.map(s => s.name); + expect(names).to.include('greet'); + expect(names).to.include('getCount'); + for (const sym of symbols) { + if (sym.name === 'greet' || sym.name === 'getCount') { + expect(sym.kind).to.equal(SymbolKind.Function); + // symbol range must start inside the CDATA block + expect(sym.range.start.line).to.be.at.least(cdataStartLine + 1); + } + } + }); + + it('getSignatureHelp returns signature for a function call inside CDATA', () => { + // Add a call site so we can test signature help + xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + program.validate(); + // line 6 is ` greet(` — `(` is at col 17, col 18 is inside the arg list + const sigHelp = program.getSignatureHelp(xmlFile.srcPath, Position.create(6, 18)); + expect(sigHelp).to.have.length.greaterThan(0); + expect(sigHelp[0].signature.label).to.include('greet'); + }); + + it('getCodeActions workspace edits reference the XML file URI', () => { + // use a range inside the CDATA block (line 3 = `sub greet(name as string)`) + const actions = program.getCodeActions(xmlFile.srcPath, Range.create(3, 8, 3, 30)); + // if any code action has workspace edits, they must target the xml file + for (const action of actions) { + if (action.edit?.changes) { + for (const uri of Object.keys(action.edit.changes)) { + expect(uri).to.include('MyComp.xml'); + } + } + if (action.edit?.documentChanges) { + for (const change of action.edit.documentChanges) { + if ('textDocument' in change) { + expect(change.textDocument.uri).to.include('MyComp.xml'); + } + } + } + } + }); + + it('uses cdataStartChar correctly for first-line synthetic positions', () => { + // The first line of the synthetic file (line 0) maps to the ('components/Inline.xml', trim` + + + + + `); + program.validate(); + // `sub` and `init` are on the same line as Date: Sat, 28 Mar 2026 18:31:33 -0300 Subject: [PATCH 06/20] test fixes for windows --- src/files/XmlFile.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/files/XmlFile.ts b/src/files/XmlFile.ts index a6f41f820..3e363f7d6 100644 --- a/src/files/XmlFile.ts +++ b/src/files/XmlFile.ts @@ -189,7 +189,7 @@ export class XmlFile { private getInlineScriptPkgPath(cdataIndex: number, scriptType: string | undefined): string { const isBrighterScript = /brighterscript|text\/bs\b/i.test(scriptType ?? ''); const ext = isBrighterScript ? '.bs' : '.brs'; - return this.pkgPath.replace(/\.xml$/i, `.cdata-${cdataIndex}.script${ext}`); + return this.pkgPath.replace(/\\/g, '/').replace(/\.xml$/i, `.cdata-${cdataIndex}.script${ext}`); } public functionScopes = [] as FunctionScope[]; From 6eb27b385933ffcc19d7e23ee016b4e4a2c2d702 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Sat, 28 Mar 2026 18:46:16 -0300 Subject: [PATCH 07/20] testing more fixes for windows --- src/Program.ts | 2 +- src/files/XmlFile.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index 8780003bd..e17b232af 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -708,7 +708,7 @@ export class Program { const inlinePkgPath = xmlFile.inlineScriptPkgPaths[cdataScriptIndex++]; const inlineFile = this.setFile(inlinePkgPath, script.cdataText ?? ''); inlineFile.isSynthetic = true; - this.syntheticFileMeta.set(inlinePkgPath.toLowerCase(), { + this.syntheticFileMeta.set(s`${inlinePkgPath}`.toLowerCase(), { xmlFile: xmlFile, cdataRange: script.cdata.range }); diff --git a/src/files/XmlFile.ts b/src/files/XmlFile.ts index 3e363f7d6..d03bed476 100644 --- a/src/files/XmlFile.ts +++ b/src/files/XmlFile.ts @@ -306,7 +306,7 @@ export class XmlFile { let dependencies = [ ...this.scriptTagImports.map(x => x.pkgPath.toLowerCase()), - ...this.inlineScriptPkgPaths.map(p => p.toLowerCase()) + ...this.inlineScriptPkgPaths.map(p => util.standardizePath(p).toLowerCase()) ]; //if autoImportComponentScript is enabled, add the .bs and .brs files with the same name if (this.program.options.autoImportComponentScript) { @@ -476,9 +476,10 @@ export class XmlFile { return map; }, {}); for (const pkgPath of this.inlineScriptPkgPaths) { - alreadyThereScriptImportMap[pkgPath.toLowerCase()] = true; + const normalizedPkgPath = util.standardizePath(pkgPath).toLowerCase(); + alreadyThereScriptImportMap[normalizedPkgPath] = true; //also mark the .brs variant so a .bs inline script doesn't get added as an extra import - alreadyThereScriptImportMap[pkgPath.toLowerCase().replace(/\.bs$/, '.brs')] = true; + alreadyThereScriptImportMap[normalizedPkgPath.replace(/\.bs$/, '.brs')] = true; } let resultMap = {}; From 5b162310430b2d27b2a3c5ef4ff33199d71383cd Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Mon, 30 Mar 2026 14:24:50 -0300 Subject: [PATCH 08/20] Use offset-padded synthetic files to eliminate CDATA coordinate remapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Synthetic BrsFiles for CDATA blocks are now created with leading newlines and spaces so every position aligns with its parent XML file coordinates. This removes all toSynthetic/fromSynthetic transforms — LSP handlers pass positions through unchanged, and results need only a file URI substitution rather than range remapping. Co-Authored-By: Claude Sonnet 4.6 --- src/Program.ts | 233 +++++++++----------------------------- src/files/XmlFile.spec.ts | 13 +-- 2 files changed, 58 insertions(+), 188 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index e17b232af..b7a412330 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -58,19 +58,15 @@ export interface SignatureInfoObj { } /** - * Coordinate-space remapping context for a single CDATA block. Returned by - * `Program.resolveCdataContext` when a position falls inside an inline script block. - * Provides bound helpers to convert positions between the parent XML file's coordinate - * space and the synthetic BrsFile's coordinate space. + * Context for a single CDATA block. Returned by `Program.resolveCdataContext` when a + * position falls inside an inline script block. Because synthetic BrsFiles are created + * with offset-padded content, their positions are already in parent XML coordinate space + * — no coordinate transformation is required. */ export interface CdataContext { brsFile: BrsFile; xmlFile: XmlFile; cdataRange: Range; - /** Convert a position from parent XML coordinates to synthetic BrsFile coordinates. */ - toSynthetic(pos: Position): Position; - /** Convert a position from synthetic BrsFile coordinates back to parent XML coordinates. */ - fromSynthetic(pos: Position): Position; } export class Program { @@ -336,9 +332,9 @@ export class Program { } /** - * Remaps diagnostics from synthetic CDATA BrsFiles back to their parent XmlFile at the - * correct position. ` { @@ -349,75 +345,30 @@ export class Program { if (!meta) { return diagnostic; } - return { - ...diagnostic, - file: meta.xmlFile, - range: { - start: this.remapPosFromSynthetic(meta.cdataRange, diagnostic.range.start), - end: this.remapPosFromSynthetic(meta.cdataRange, diagnostic.range.end) - } - }; + return { ...diagnostic, file: meta.xmlFile }; }); } /** - * Like `remapSyntheticFileDiagnostics`, but skips remapping for `contextFile` so its - * diagnostics remain in synthetic-file coordinate space. Used during code action events - * so that plugins can match diagnostics by file identity (`x.file === event.file`). + * Like `remapSyntheticFileDiagnostics`, but keeps `contextFile`'s diagnostics pointing + * at the synthetic BrsFile so plugins can match by file identity during code action events. */ private partialRemapSyntheticFileDiagnostics(diagnostics: BsDiagnostic[], contextFile: BrsFile): BsDiagnostic[] { return diagnostics.map(diagnostic => { if (!diagnostic.file?.isSynthetic) { return diagnostic; } - // Keep the context file's diagnostics in synthetic coordinate space if (diagnostic.file === contextFile) { return diagnostic; } - // All other synthetic files remap to XML as usual const meta = this.syntheticFileMeta.get(diagnostic.file.pkgPath.toLowerCase()); if (!meta) { return diagnostic; } - return { - ...diagnostic, - file: meta.xmlFile, - range: { - start: this.remapPosFromSynthetic(meta.cdataRange, diagnostic.range.start), - end: this.remapPosFromSynthetic(meta.cdataRange, diagnostic.range.end) - } - }; + return { ...diagnostic, file: meta.xmlFile }; }); } - /** - * Converts a position in synthetic BrsFile coordinates to its equivalent position in the - * parent XML file. Line 0 of the synthetic file maps to the line containing `` block, or `undefined` if it does not. Use the returned context - * to redirect LSP events to the synthetic BrsFile and remap positions in both directions. + * inside a `` block, or `undefined` if it does not. + * Because synthetic files use offset-padded content, no position transformation is needed — + * pass the original XML-space position directly to the synthetic BrsFile's handlers. */ public resolveCdataContext(xmlFile: XmlFile, position: Position): CdataContext | undefined { const pointRange = util.createRange(position.line, position.character, position.line, position.character); @@ -483,55 +422,21 @@ export class Program { if (!info) { return undefined; } - const cdataRange = info.meta.cdataRange; return { brsFile: info.brsFile, xmlFile: xmlFile, - cdataRange: cdataRange, - toSynthetic: (pos) => this.remapPosToSynthetic(cdataRange, pos), - fromSynthetic: (pos) => this.remapPosFromSynthetic(cdataRange, pos) + cdataRange: info.meta.cdataRange }; } /** - * Remaps any `Location` entries that reference the synthetic BrsFile back to the parent - * XML file with positions converted to XML coordinates. Locations in other files are - * returned unchanged. + * Substitutes the synthetic BrsFile URI with the parent XML file URI in any Location + * entries that reference it. Ranges are already in XML coordinate space. */ private remapLocationsFromSynthetic(locations: Location[], cdataCtx: CdataContext): Location[] { const syntheticUri = URI.file(cdataCtx.brsFile.srcPath).toString(); const xmlUri = URI.file(cdataCtx.xmlFile.srcPath).toString(); - return locations.map(loc => { - if (loc.uri !== syntheticUri) { - return loc; - } - return { - uri: xmlUri, - range: { - start: cdataCtx.fromSynthetic(loc.range.start), - end: cdataCtx.fromSynthetic(loc.range.end) - } - }; - }); - } - - /** - * Recursively remaps `range` and `selectionRange` in a `DocumentSymbol` tree from - * synthetic BrsFile coordinates to parent XML file coordinates. - */ - private remapDocumentSymbolsFromSynthetic(symbols: DocumentSymbol[], cdataCtx: CdataContext): DocumentSymbol[] { - return symbols.map(sym => ({ - ...sym, - range: { - start: cdataCtx.fromSynthetic(sym.range.start), - end: cdataCtx.fromSynthetic(sym.range.end) - }, - selectionRange: { - start: cdataCtx.fromSynthetic(sym.selectionRange.start), - end: cdataCtx.fromSynthetic(sym.selectionRange.end) - }, - children: sym.children ? this.remapDocumentSymbolsFromSynthetic(sym.children, cdataCtx) : undefined - })); + return locations.map(loc => (loc.uri === syntheticUri ? { uri: xmlUri, range: loc.range } : loc)); } public addDiagnostics(diagnostics: BsDiagnostic[]) { @@ -706,11 +611,20 @@ export class Program { for (const script of xmlFile.ast.component?.scripts ?? []) { if (script.cdata) { const inlinePkgPath = xmlFile.inlineScriptPkgPaths[cdataScriptIndex++]; - const inlineFile = this.setFile(inlinePkgPath, script.cdataText ?? ''); + // Pad the content with leading newlines and spaces so that every + // position in the synthetic file naturally aligns with its position in + // the parent XML file. This eliminates all coordinate remapping for LSP + // events — only file URI substitution is needed in results. + const cdataRange = script.cdata.range; + const contentStartChar = cdataRange.start.character + '(inlinePkgPath, paddedContent); inlineFile.isSynthetic = true; this.syntheticFileMeta.set(s`${inlinePkgPath}`.toLowerCase(), { xmlFile: xmlFile, - cdataRange: script.cdata.range + cdataRange: cdataRange }); } } @@ -1211,9 +1125,7 @@ export class Program { return []; } - const cdataCtx = isXmlFile(file) ? this.resolveCdataContext(file, position) : undefined; - const effectiveFile = cdataCtx?.brsFile ?? file; - const effectivePosition = cdataCtx ? cdataCtx.toSynthetic(position) : position; + const effectiveFile = isXmlFile(file) ? (this.resolveCdataContext(file, position)?.brsFile ?? file) : file; //find the scopes for this file let scopes = this.getScopesForFile(effectiveFile); @@ -1225,7 +1137,7 @@ export class Program { program: this, file: effectiveFile, scopes: scopes, - position: effectivePosition, + position: position, completions: [] }; @@ -1233,21 +1145,6 @@ export class Program { this.plugins.emit('provideCompletions', event); this.plugins.emit('afterProvideCompletions', event); - if (cdataCtx) { - for (const completion of event.completions) { - if (completion.textEdit) { - if ('range' in completion.textEdit) { - completion.textEdit = { ...completion.textEdit, range: { start: cdataCtx.fromSynthetic(completion.textEdit.range.start), end: cdataCtx.fromSynthetic(completion.textEdit.range.end) } }; - } else { - completion.textEdit = { ...completion.textEdit, insert: { start: cdataCtx.fromSynthetic(completion.textEdit.insert.start), end: cdataCtx.fromSynthetic(completion.textEdit.insert.end) }, replace: { start: cdataCtx.fromSynthetic(completion.textEdit.replace.start), end: cdataCtx.fromSynthetic(completion.textEdit.replace.end) } }; - } - } - if (completion.additionalTextEdits) { - completion.additionalTextEdits = completion.additionalTextEdits.map(e => ({ ...e, range: { start: cdataCtx.fromSynthetic(e.range.start), end: cdataCtx.fromSynthetic(e.range.end) } })); - } - } - } - return event.completions; } @@ -1277,12 +1174,11 @@ export class Program { const cdataCtx = isXmlFile(file) ? this.resolveCdataContext(file, position) : undefined; const effectiveFile = cdataCtx?.brsFile ?? file; - const effectivePosition = cdataCtx ? cdataCtx.toSynthetic(position) : position; const event: ProvideDefinitionEvent = { program: this, file: effectiveFile, - position: effectivePosition, + position: position, definitions: [] }; @@ -1299,14 +1195,12 @@ export class Program { let file = this.getFile(srcPath); let result: Hover[]; if (file) { - const cdataCtx = isXmlFile(file) ? this.resolveCdataContext(file, position) : undefined; - const effectiveFile = cdataCtx?.brsFile ?? file; - const effectivePosition = cdataCtx ? cdataCtx.toSynthetic(position) : position; + const effectiveFile = isXmlFile(file) ? (this.resolveCdataContext(file, position)?.brsFile ?? file) : file; const event = { program: this, file: effectiveFile, - position: effectivePosition, + position: position, scopes: this.getScopesForFile(effectiveFile), hovers: [] } as ProvideHoverEvent; @@ -1314,9 +1208,7 @@ export class Program { this.plugins.emit('provideHover', event); this.plugins.emit('afterProvideHover', event); - result = cdataCtx - ? event.hovers.map(h => (h.range ? { ...h, range: { start: cdataCtx.fromSynthetic(h.range.start), end: cdataCtx.fromSynthetic(h.range.end) } } : h)) - : event.hovers; + result = event.hovers; } return result ?? []; @@ -1340,7 +1232,8 @@ export class Program { this.plugins.emit('provideDocumentSymbols', event); this.plugins.emit('afterProvideDocumentSymbols', event); - // For XML files, also collect symbols from each inline CDATA block and remap + // For XML files, also collect symbols from each inline CDATA block. + // Ranges are already in XML coordinate space — no remapping needed. if (isXmlFile(file)) { for (const [pkgPath, meta] of this.syntheticFileMeta) { if (meta.xmlFile !== file) { @@ -1350,10 +1243,6 @@ export class Program { if (!brsFile) { continue; } - const cdataCtx = this.resolveCdataContext(file, meta.cdataRange.start); - if (!cdataCtx) { - continue; - } const cdataEvent: ProvideDocumentSymbolsEvent = { program: this, file: brsFile, @@ -1362,7 +1251,7 @@ export class Program { this.plugins.emit('beforeProvideDocumentSymbols', cdataEvent); this.plugins.emit('provideDocumentSymbols', cdataEvent); this.plugins.emit('afterProvideDocumentSymbols', cdataEvent); - event.documentSymbols.push(...this.remapDocumentSymbolsFromSynthetic(cdataEvent.documentSymbols, cdataCtx)); + event.documentSymbols.push(...cdataEvent.documentSymbols); } } @@ -1387,10 +1276,6 @@ export class Program { // so that plugins can match diagnostics by file identity (x.file === event.file) // and derive correct edit positions from diagnostic.data (AST node ranges). const effectiveFile = cdataCtx?.brsFile ?? file; - const effectiveRange = cdataCtx ? { - start: cdataCtx.toSynthetic(range.start), - end: cdataCtx.toSynthetic(range.end) - } : range; this._cdataDiagnosticsContext = cdataCtx?.brsFile; @@ -1400,14 +1285,14 @@ export class Program { //only keep diagnostics related to this file .filter(x => x.file === effectiveFile) //only keep diagnostics that touch this range - .filter(x => util.rangesIntersectOrTouch(x.range, effectiveRange)); + .filter(x => util.rangesIntersectOrTouch(x.range, range)); const scopes = this.getScopesForFile(effectiveFile); this.plugins.emit('onGetCodeActions', { program: this, file: effectiveFile, - range: effectiveRange, + range: range, diagnostics: diagnostics, scopes: scopes, codeActions: codeActions @@ -1416,7 +1301,7 @@ export class Program { this._cdataDiagnosticsContext = undefined; if (cdataCtx) { - this.remapCodeActionChangesToXml(codeActions, cdataCtx, cdataCtx.brsFile, file as XmlFile); + this.remapCodeActionChangesToXml(codeActions, cdataCtx.brsFile, file as XmlFile); } } return codeActions; @@ -1438,7 +1323,8 @@ export class Program { semanticTokens: result }); - // For XML files, also collect semantic tokens from each inline CDATA block and remap + // For XML files, also collect semantic tokens from each inline CDATA block. + // Ranges are already in XML coordinate space — no remapping needed. if (isXmlFile(file)) { for (const [pkgPath, meta] of this.syntheticFileMeta) { if (meta.xmlFile !== file) { @@ -1448,10 +1334,6 @@ export class Program { if (!brsFile) { continue; } - const cdataCtx = this.resolveCdataContext(file, meta.cdataRange.start); - if (!cdataCtx) { - continue; - } const cdataTokens = [] as SemanticToken[]; this.plugins.emit('onGetSemanticTokens', { program: this, @@ -1459,15 +1341,7 @@ export class Program { scopes: this.getScopesForFile(brsFile), semanticTokens: cdataTokens }); - for (const token of cdataTokens) { - result.push({ - ...token, - range: { - start: cdataCtx.fromSynthetic(token.range.start), - end: cdataCtx.fromSynthetic(token.range.end) - } - }); - } + result.push(...cdataTokens); } } @@ -1479,13 +1353,11 @@ export class Program { if (!file) { return []; } - const cdataCtx = isXmlFile(file) ? this.resolveCdataContext(file, position) : undefined; - const effectiveFile = cdataCtx?.brsFile ?? file; - const effectivePosition = cdataCtx ? cdataCtx.toSynthetic(position) : position; + const effectiveFile = isXmlFile(file) ? (this.resolveCdataContext(file, position)?.brsFile ?? file) : file; if (!isBrsFile(effectiveFile)) { return []; } - let callExpressionInfo = new CallExpressionInfo(effectiveFile, effectivePosition); + let callExpressionInfo = new CallExpressionInfo(effectiveFile, position); let signatureHelpUtil = new SignatureHelpUtil(); return signatureHelpUtil.getSignatureHelpItems(callExpressionInfo); } @@ -1499,12 +1371,11 @@ export class Program { const cdataCtx = isXmlFile(file) ? this.resolveCdataContext(file, position) : undefined; const effectiveFile = cdataCtx?.brsFile ?? file; - const effectivePosition = cdataCtx ? cdataCtx.toSynthetic(position) : position; const event: ProvideReferencesEvent = { program: this, file: effectiveFile, - position: effectivePosition, + position: position, references: [] }; diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index 196a8360f..11fef6134 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -1706,10 +1706,10 @@ describe('XmlFile', () => { } }); - it('uses cdataStartChar correctly for first-line synthetic positions', () => { - // The first line of the synthetic file (line 0) maps to the { + // The synthetic BrsFile is padded so its positions match XML coordinates directly. + // When content starts inline with ('components/Inline.xml', trim` @@ -1718,9 +1718,8 @@ describe('XmlFile', () => { `); program.validate(); - // `sub` and `init` are on the same line as Date: Mon, 30 Mar 2026 16:51:42 -0300 Subject: [PATCH 09/20] Transpile CDATA blocks back into XML instead of extracting to separate files - SGScript gains a `transpileSourceNode` property; when set, transpileBody() embeds it inline as `` rather than writing a uri-based tag - Program.transpileSyntheticBrsFileToSourceNode() fires beforeFileTranspile plugin events and returns a SourceNode whose positions reference the parent XML file (offset-padded content ensures no coordinate remapping is needed) - XmlFile.transpile() pre-transpiles any BrighterScript CDATA blocks via the new method and stores the result on the SGScript before the AST transpile pass, then cleans up afterward; synthetic files are skipped in the output - Add tests for transpile output of enums, namespaces, ternary, const, and null coalescing inside CDATA blocks - Add source map tests verifying that generated positions trace back to the correct line/col in the parent XML file Co-Authored-By: Claude Sonnet 4.6 --- src/Program.ts | 45 +++++++ src/files/XmlFile.spec.ts | 258 +++++++++++++++++++++++++++++++++++- src/files/XmlFile.ts | 29 ++-- src/parser/SGParser.spec.ts | 6 +- src/parser/SGTypes.ts | 18 ++- 5 files changed, 339 insertions(+), 17 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index b7a412330..1a45958cb 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -24,7 +24,9 @@ import { isBrsFile, isXmlFile, isXmlScope, isNamespaceStatement } from './astUti import type { FunctionStatement, NamespaceStatement } from './parser/Statement'; import { BscPlugin } from './bscPlugin/BscPlugin'; import { AstEditor } from './astUtils/AstEditor'; +import type { SourceNode } from 'source-map'; import type { SourceMapGenerator } from 'source-map'; +import { BrsTranspileState } from './parser/BrsTranspileState'; import type { Statement } from './parser/AstNode'; import { CallExpressionInfo } from './bscPlugin/CallExpressionInfo'; import { SignatureHelpUtil } from './bscPlugin/SignatureHelpUtil'; @@ -1464,6 +1466,44 @@ export class Program { return Promise.resolve(result); } + /** + * Transpile a synthetic CDATA BrsFile and return a SourceNode suitable for embedding + * back into the parent XML file's transpile output. Plugin beforeFileTranspile events are + * fired so AST edits (e.g. built-in transpile transforms) are applied. afterFileTranspile + * is intentionally skipped — it is designed for standalone file output, not embedded content. + * + * Because synthetic BrsFiles are created with offset-padded content, their token positions + * already align with positions in the parent XML file. Overriding state.srcPath to the XML + * file's path makes the resulting SourceNode reference the XML file directly, producing a + * correct unified source map. + */ + public transpileSyntheticBrsFileToSourceNode(brsFile: BrsFile, xmlSrcPath: string): SourceNode { + const editor = new AstEditor(); + + this.plugins.emit('beforeFileTranspile', { + program: this, + file: brsFile, + outputPath: undefined, + editor: editor + }); + if (editor.hasChanges) { + editor.setProperty(brsFile, 'needsTranspiled', true); + } + + // Override srcPath so every SourceNode references the XML file at the correct position + const state = new BrsTranspileState(brsFile); + state.srcPath = xmlSrcPath; + + const sourceNode = util.sourceNodeFromTranspileResult( + null, null, state.srcPath, brsFile.ast.transpile(state) + ); + + state.editor.undoAll(); + editor.undoAll(); + + return sourceNode; + } + /** * Internal function used to transpile files. * This does not write anything to the file system @@ -1574,6 +1614,11 @@ export class Program { //mark this file as processed so we don't process it more than once processedFiles.add(outputPath?.toLowerCase()); + //synthetic CDATA BrsFiles are embedded back into their parent XML — don't write them as separate output files + if (isBrsFile(file) && file.isSynthetic) { + return; + } + if (!this.options.pruneEmptyCodeFiles || !file.canBePruned) { //skip transpiling typedef files if (isBrsFile(file) && file.isTypedef) { diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index 11fef6134..5633e2b2e 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -1,4 +1,5 @@ import { assert, expect } from '../chai-config.spec'; +import { SourceMapConsumer } from 'source-map'; import * as path from 'path'; import * as sinonImport from 'sinon'; import type { CompletionItem } from 'vscode-languageserver'; @@ -1406,7 +1407,7 @@ describe('XmlFile', () => { expect(program.getFile('components/MyComp.cdata-1.script.bs')).to.exist; }); - it('transpile replaces CDATA blocks with uri script tags', () => { + it('transpile preserves CDATA blocks inline in the xml output', () => { testTranspile(trim` @@ -1418,13 +1419,16 @@ describe('XmlFile', () => { `, trim` - + + + `, trim` + + + + + + `, trim` + + + + + + `, trim` + + + + + + `, trim` + + + + + `, trim` + + + + + + `); + program.options.sourceMap = true; + program.validate(); + const result = xmlFile.transpile(); + expect(result.map).to.exist; + + await SourceMapConsumer.with(result.map.toJSON(), null, (consumer) => { + // raw CDATA lines map at col 0 back to the same line in the xml source + const subPos = consumer.originalPositionFor({ line: 4, column: 0 }); + expect(subPos.source).to.include('MyComp.xml'); + expect(subPos.line).to.equal(4); + expect(subPos.column).to.equal(0); + + const endSubPos = consumer.originalPositionFor({ line: 5, column: 0 }); + expect(endSubPos.source).to.include('MyComp.xml'); + expect(endSubPos.line).to.equal(5); + expect(endSubPos.column).to.equal(0); + }); + }); + + it('source map points back to xml positions for transpiled brighterscript CDATA', async () => { + // Generated output (1-based lines): + // 3: " + + `); + program.options.sourceMap = true; + program.validate(); + const result = xmlFile.transpile(); + expect(result.map).to.exist; + // only the xml file should appear in sources — not the synthetic brs file + expect(result.map.toJSON().sources).to.eql([s`${rootDir}/components/MyComp.xml`]); + + await SourceMapConsumer.with(result.map.toJSON(), null, (consumer) => { + // 'sub' is on gen line 3 right after ' { + // Generated output (1-based lines): + // 5: "sub init()" ← col 0 → source (7, 8) + // 6: " d = \"up\"" ← col 4 → source (8, 12) + // 7: "end sub]]>..." ← col 0 → source (9, 8) + const xmlFile = program.setFile({ src: s`${rootDir}/components/MyComp.xml`, dest: 'components/MyComp.xml' }, trim` + + + + + `); + program.options.sourceMap = true; + program.validate(); + const result = xmlFile.transpile(); + expect(result.map).to.exist; + expect(result.map.toJSON().sources).to.eql([s`${rootDir}/components/MyComp.xml`]); + + await SourceMapConsumer.with(result.map.toJSON(), null, (consumer) => { + // 'sub' on gen line 5 col 0 → source (7, 8) + const subPos = consumer.originalPositionFor({ line: 5, column: 0 }); + expect(subPos.source).to.include('MyComp.xml'); + expect(subPos.line).to.equal(7); + expect(subPos.column).to.equal(8); + + // 'd' on gen line 6 col 4 → source (8, 12) + const dPos = consumer.originalPositionFor({ line: 6, column: 4 }); + expect(dPos.source).to.include('MyComp.xml'); + expect(dPos.line).to.equal(8); + expect(dPos.column).to.equal(12); + }); + }); + it('cleans up synthetic files when the xml file is removed', () => { program.setFile('components/MyComp.xml', trim` diff --git a/src/files/XmlFile.ts b/src/files/XmlFile.ts index d03bed476..018a48026 100644 --- a/src/files/XmlFile.ts +++ b/src/files/XmlFile.ts @@ -6,6 +6,7 @@ import { DiagnosticCodeMap, diagnosticCodes } from '../DiagnosticMessages'; import type { FunctionScope } from '../FunctionScope'; import type { Callable, BsDiagnostic, File, FileReference, FunctionCall, CommentFlag } from '../interfaces'; import type { Program } from '../Program'; +import type { BrsFile } from './BrsFile'; import util from '../util'; import SGParser, { rangeFromTokenValue } from '../parser/SGParser'; import chalk from 'chalk'; @@ -532,18 +533,20 @@ export class XmlFile { const originalScripts = this.ast.component?.scripts ?? []; - //replace CDATA script blocks with uri-based script tags pointing to the synthetic extracted files + //pre-transpile any CDATA scripts that need it, storing the SourceNode on the SGScript + //so that SGScript.transpileBody() can embed it directly into the XML output let cdataIndex = 0; - const scriptsWithInlineUris = originalScripts.map(script => { + let anySyntheticNeedsTranspiled = false; + for (const script of originalScripts) { if (script.cdata) { const inlinePkgPath = this.inlineScriptPkgPaths[cdataIndex++]; - const uriScript = new SGScript(); - uriScript.type = 'text/brightscript'; - uriScript.uri = util.getRokuPkgPath(inlinePkgPath.replace(/\.bs$/i, '.brs')); - return uriScript; + const brsFile = this.program.getFile(inlinePkgPath); + if (brsFile?.needsTranspiled) { + anySyntheticNeedsTranspiled = true; + script.transpileSourceNode = this.program.transpileSyntheticBrsFileToSourceNode(brsFile, state.srcPath); + } } - return script; - }); + } const extraImportScripts = this.getMissingImportsForTranspile().map(uri => { const script = new SGScript(); @@ -552,21 +555,25 @@ export class XmlFile { }); const [scriptsHaveChanged, publishableScripts] = this.checkScriptsForPublishableImports([ - ...scriptsWithInlineUris, + ...originalScripts, ...extraImportScripts ]); let transpileResult: SourceNode | undefined; - if (this.needsTranspiled || extraImportScripts.length > 0 || scriptsHaveChanged) { + if (this.needsTranspiled || anySyntheticNeedsTranspiled || extraImportScripts.length > 0 || scriptsHaveChanged) { //temporarily add the missing imports as script tags this.ast.component.scripts = publishableScripts; - transpileResult = util.sourceNodeFromTranspileResult(null, null, state.srcPath, this.parser.ast.transpile(state)); //restore the original scripts array this.ast.component.scripts = originalScripts; + //clean up transpileSourceNode — don't leave SourceNodes on AST nodes after transpile + for (const script of originalScripts) { + delete script.transpileSourceNode; + } + } else if (this.program.options.sourceMap) { //emit code as-is with a simple map to the original file location transpileResult = util.simpleMap(state.srcPath, this.fileContents); diff --git a/src/parser/SGParser.spec.ts b/src/parser/SGParser.spec.ts index f3d2717e5..22f7f451e 100644 --- a/src/parser/SGParser.spec.ts +++ b/src/parser/SGParser.spec.ts @@ -63,7 +63,11 @@ describe('SGParser', () => { + + `); + program.validate(); + }); + + // Note: afterFileParse cannot be covered here because isSynthetic is set on the + // BrsFile *after* setFile() returns, which is after afterFileParse fires. So the + // emitWithSyntheticFileContext wrapper cannot identify the file as synthetic at + // that point. This is a known limitation. + + it('onFileValidate: _cdataDiagnosticsContext is set to the synthetic BrsFile during the event', () => { + // onFileValidate fires during the validation pass, before scope diagnostics are + // produced. We verify the context flag is set so getDiagnostics() would scope + // correctly if called by a plugin. + let contextFileInsideHandler: BrsFile | undefined; + program.plugins.add({ + name: 'test', + onFileValidate: function(event) { + if (isBrsFile(event.file) && event.file.isSynthetic) { + contextFileInsideHandler = (program as any)._cdataDiagnosticsContext; + } + } + }); + program.setFile('components/MyComp.xml', xmlFile.fileContents); + program.validate(); + expect(contextFileInsideHandler).to.exist; + expect((contextFileInsideHandler as BrsFile).isSynthetic).to.be.true; + }); + + it('provideDocumentSymbols: program.getDiagnostics() inside handler associates diagnostics with the synthetic BrsFile', () => { + let fileIdentityWorked = false; + program.plugins.add({ + name: 'test', + provideDocumentSymbols: function(event) { + if (isBrsFile(event.file) && (event.file as BrsFile).isSynthetic) { + const diags = program.getDiagnostics() + .filter(x => x.file === event.file); + if (diags.length > 0) { + fileIdentityWorked = true; + } + } + } + }); + program.getDocumentSymbols(xmlFile.srcPath); + expect(fileIdentityWorked).to.be.true; + }); + + it('onGetSemanticTokens: program.getDiagnostics() inside handler associates diagnostics with the synthetic BrsFile', () => { + let fileIdentityWorked = false; + program.plugins.add({ + name: 'test', + onGetSemanticTokens: function(event) { + if (isBrsFile(event.file) && (event.file as BrsFile).isSynthetic) { + const diags = program.getDiagnostics() + .filter(x => x.file === event.file); + if (diags.length > 0) { + fileIdentityWorked = true; + } + } + } + }); + program.getSemanticTokens(xmlFile.srcPath); + expect(fileIdentityWorked).to.be.true; + }); + + it('onGetCodeActions: program.getDiagnostics() inside handler supports "fix all" pattern across multiple CDATA diagnostics', () => { + // Two `then` violations in one CDATA block — simulate the bslint "fix all" pattern + const twoThenFile = program.setFile('components/TwoThen.xml', trim` + + + + + `); + program.validate(); + const twoThenSynthetic = program.getFile('components/TwoThen.cdata-0.script.brs'); + + // Simulate plugin calling getDiagnostics() filtered by event.file identity + const fixAllDiagsFoundBySyntheticIdentity: BsDiagnostic[][] = []; + program.plugins.add({ + name: 'test', + onGetCodeActions: function(event) { + if (event.file === twoThenSynthetic) { + // This is exactly the pattern a "fix all" plugin would use + const allInFile = program.getDiagnostics() + .filter(x => x.file === event.file); + fixAllDiagsFoundBySyntheticIdentity.push(allInFile); + } + } + }); + + // trigger inside first function range + program.getCodeActions(twoThenFile.srcPath, Range.create(3, 12, 3, 12)); + + // the plugin must have seen diagnostics associated with the synthetic BrsFile + expect(fixAllDiagsFoundBySyntheticIdentity.length).to.be.greaterThan(0); + // and all returned diagnostics must reference the synthetic file, not the xml file + for (const group of fixAllDiagsFoundBySyntheticIdentity) { + for (const d of group) { + expect(d.file).to.equal(twoThenSynthetic, 'expected diagnostic.file to be the synthetic BrsFile, not the XmlFile'); + } + } + }); + }); }); }); + From a8d8803da5494ff2a795d519f22a38bbca1888f9 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Mon, 30 Mar 2026 19:39:56 -0300 Subject: [PATCH 14/20] Fix CDATA remapping gaps in workspace symbols and afterFileParse timing - WorkspaceSymbolProcessor now skips synthetic BrsFiles and instead collects their symbols via getXmlFileWorkspaceSymbols(), which iterates inlineScriptPkgPaths and remaps each symbol's URI to the parent XML file. Previously, symbols from CDATA blocks appeared under the synthetic pkg path (e.g. *.cdata-0.script.brs) rather than the XML file. - Add _pendingSyntheticSrcPaths so that isSynthetic is set on the BrsFile before afterFileParse fires. Previously emitWithSyntheticFileContext checked isSynthetic after setFile returned, so the _cdataDiagnosticsContext guard was always a no-op during the initial parse. Co-Authored-By: Claude Sonnet 4.6 --- src/Program.ts | 20 +++++++++++++++- .../symbols/WorkspaceSymbolProcessor.ts | 23 +++++++++++++++---- src/bscPlugin/symbols/symbolUtils.ts | 4 ++-- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index 14bb5f494..66cbc5621 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -181,6 +181,14 @@ export class Program { */ private _cdataDiagnosticsContext: BrsFile | undefined; + /** + * Set of absolute srcPaths for BrsFiles that are about to be registered as synthetic (CDATA) + * files. Populated before `setFile` is called so that `isSynthetic` can be pre-applied inside + * `setFile` before `afterFileParse` fires — otherwise `emitWithSyntheticFileContext` would see + * `isSynthetic === false` and skip the diagnostic-context guard. + */ + private _pendingSyntheticSrcPaths = new Set(); + private scopes = {} as Record; protected addScope(scope: Scope) { @@ -563,6 +571,13 @@ export class Program { new BrsFile(srcPath, pkgPath, this) ); + // Pre-mark as synthetic before parsing so that `afterFileParse` fires with + // `isSynthetic === true`, allowing `emitWithSyntheticFileContext` to correctly + // set `_cdataDiagnosticsContext` for any plugins that call `getDiagnostics()`. + if (this._pendingSyntheticSrcPaths.has(srcPath)) { + brsFile.isSynthetic = true; + } + //add file to the `source` dependency list if (brsFile.pkgPath.startsWith(startOfSourcePkgPath)) { this.createSourceScope(); @@ -631,8 +646,11 @@ export class Program { const paddedContent = '\n'.repeat(cdataRange.start.line) + ' '.repeat(contentStartChar) + (script.cdataText ?? ''); + const inlineSrcPath = s`${path.resolve(this.options.rootDir, inlinePkgPath)}`; + this._pendingSyntheticSrcPaths.add(inlineSrcPath); const inlineFile = this.setFile(inlinePkgPath, paddedContent); - inlineFile.isSynthetic = true; + this._pendingSyntheticSrcPaths.delete(inlineSrcPath); + // isSynthetic was pre-applied inside setFile (see _pendingSyntheticSrcPaths) inlineFile.excludeFromOutput = true; inlineFile.parentXmlFile = xmlFile; inlineFile.cdataScript = script; diff --git a/src/bscPlugin/symbols/WorkspaceSymbolProcessor.ts b/src/bscPlugin/symbols/WorkspaceSymbolProcessor.ts index cc5eb4561..23be8423f 100644 --- a/src/bscPlugin/symbols/WorkspaceSymbolProcessor.ts +++ b/src/bscPlugin/symbols/WorkspaceSymbolProcessor.ts @@ -1,7 +1,9 @@ -import { isBrsFile } from '../../astUtils/reflection'; +import { isBrsFile, isXmlFile } from '../../astUtils/reflection'; import type { BrsFile } from '../../files/BrsFile'; +import type { XmlFile } from '../../files/XmlFile'; import type { ProvideWorkspaceSymbolsEvent } from '../../interfaces'; import { getWorkspaceSymbolsFromBrsFile } from './symbolUtils'; +import util from '../../util'; export class WorkspaceSymbolProcessor { public constructor( @@ -12,17 +14,30 @@ export class WorkspaceSymbolProcessor { public process() { const results = Object.values(this.event.program.files).map(file => { - if (isBrsFile(file)) { + if (isBrsFile(file) && !file.isSynthetic) { return this.getBrsFileWorkspaceSymbols(file); + } else if (isXmlFile(file)) { + return this.getXmlFileWorkspaceSymbols(file); } return []; }); return results.flat(); } - private getBrsFileWorkspaceSymbols(file: BrsFile) { - const symbols = getWorkspaceSymbolsFromBrsFile(file); + private getBrsFileWorkspaceSymbols(file: BrsFile, uriOverride?: string) { + const symbols = getWorkspaceSymbolsFromBrsFile(file, uriOverride); this.event.workspaceSymbols.push(...symbols); return this.event.workspaceSymbols; } + + private getXmlFileWorkspaceSymbols(file: XmlFile) { + const xmlUri = util.pathToUri(file.srcPath); + for (const pkgPath of file.inlineScriptPkgPaths) { + const brsFile = this.event.program.getFile(pkgPath); + if (brsFile) { + this.getBrsFileWorkspaceSymbols(brsFile, xmlUri); + } + } + return this.event.workspaceSymbols; + } } diff --git a/src/bscPlugin/symbols/symbolUtils.ts b/src/bscPlugin/symbols/symbolUtils.ts index 416ff6c81..3dcdfae3e 100644 --- a/src/bscPlugin/symbols/symbolUtils.ts +++ b/src/bscPlugin/symbols/symbolUtils.ts @@ -28,9 +28,9 @@ export function getDocumentSymbolsFromBrsFile(file: BrsFile) { } } -export function getWorkspaceSymbolsFromBrsFile(file: BrsFile) { +export function getWorkspaceSymbolsFromBrsFile(file: BrsFile, uriOverride?: string) { const result: WorkspaceSymbol[] = []; - const uri = util.pathToUri(file.srcPath); + const uri = uriOverride ?? util.pathToUri(file.srcPath); let symbolsToProcess = getSymbolsFromAstNode(file.ast); while (symbolsToProcess.length > 0) { //get the symbol From 123813b02534d3408cad8329731e047f646577ef Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Mon, 30 Mar 2026 20:26:02 -0300 Subject: [PATCH 15/20] Enhance CDATA handling by adding parse options for accurate position tracking in lexer --- src/Program.ts | 35 ++++++++++++++++++++++++++++------- src/files/BrsFile.ts | 18 ++++++++++++++---- src/lexer/Lexer.ts | 21 +++++++++++++++++---- 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index 66cbc5621..c581890e0 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -554,6 +554,16 @@ export class Program { */ public setFile(fileEntry: FileObj, fileContents: string): T; public setFile(fileParam: FileObj | string, fileContents: string): T { + return this.setFileInternal(fileParam, fileContents); + } + + /** + * Internal implementation of setFile that accepts optional BrsFile parse options. + * The extra `parseOptions` parameter is intentionally not exposed on the public overloads — it is + * only used when registering synthetic inline CDATA BrsFiles so that the lexer can start its + * position tracking at the correct XML-space offset without needing a side-channel map. + */ + private setFileInternal(fileParam: FileObj | string, fileContents: string, parseOptions?: { startLine?: number; startCharacter?: number; rawSource?: string }): T { //normalize the file paths const { srcPath, pkgPath } = this.getPaths(fileParam, this.options.rootDir); @@ -593,7 +603,7 @@ export class Program { this.plugins.emit('beforeFileParse', sourceObj); this.logger.time(LogLevel.debug, ['parse', chalk.green(srcPath)], () => { - brsFile.parse(sourceObj.source); + brsFile.parse(sourceObj.source, parseOptions); }); //notify plugins that this file has finished parsing @@ -637,18 +647,29 @@ export class Program { for (const script of xmlFile.ast.component?.scripts ?? []) { if (script.cdata) { const inlinePkgPath = xmlFile.inlineScriptPkgPaths[cdataScriptIndex++]; - // Pad the content with leading newlines and spaces so that every - // position in the synthetic file naturally aligns with its position in - // the parent XML file. This eliminates all coordinate remapping for LSP - // events — only file URI substitution is needed in results. + // The synthetic BrsFile needs two forms of the same source: + // + // • fileContents = padded content (newlines + spaces prepended so that + // XML-coordinate line numbers index correctly into the text). Tools + // like SignatureHelpUtil use fileContents for line-based text extraction. + // + // • rawSource = the actual cdataText passed to the lexer together with + // startLine/startCharacter so that every token range is already in the + // parent XML coordinate space — without the spurious Newline tokens that + // the padding newlines would otherwise introduce into the token stream. const cdataRange = script.cdata.range; const contentStartChar = cdataRange.start.character + '(inlinePkgPath, paddedContent); + const inlineFile = this.setFileInternal(inlinePkgPath, paddedContent, { + startLine: cdataRange.start.line, + startCharacter: contentStartChar, + rawSource: rawSource + }); this._pendingSyntheticSrcPaths.delete(inlineSrcPath); // isSynthetic was pre-applied inside setFile (see _pendingSyntheticSrcPaths) inlineFile.excludeFromOutput = true; diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index f850e4e99..f6aa4b7c2 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -333,9 +333,17 @@ export class BrsFile { /** * Calculate the AST for this file - * @param fileContents the raw source code to parse + * @param fileContents the source to store as fileContents (used for line-based text extraction). For + * inline CDATA fragments this should be the offset-padded content so that XML-coordinate line + * numbers index correctly into the text. + * @param options optional scan options forwarded to the lexer + * @param options.startLine the zero-indexed line to start position tracking from + * @param options.startCharacter the zero-indexed character offset on the first line + * @param options.rawSource if provided, the lexer scans this string instead of `fileContents`. + * Use this when `fileContents` is padded for line-index alignment but you want tokens to be + * produced without the padding characters (which would otherwise become spurious Newline tokens). */ - public parse(fileContents: string) { + public parse(fileContents: string, options?: { startLine?: number; startCharacter?: number; rawSource?: string }) { try { this.fileContents = fileContents; this.diagnostics = []; @@ -349,8 +357,10 @@ export class BrsFile { //tokenize the input file let lexer = this.program.logger.time('debug', ['lexer.lex', chalk.green(this.srcPath)], () => { - return Lexer.scan(fileContents, { - includeWhitespace: false + return Lexer.scan(options?.rawSource ?? fileContents, { + includeWhitespace: false, + startLine: options?.startLine, + startCharacter: options?.startCharacter }); }); diff --git a/src/lexer/Lexer.ts b/src/lexer/Lexer.ts index 6f45df185..b6c9b4960 100644 --- a/src/lexer/Lexer.ts +++ b/src/lexer/Lexer.ts @@ -93,10 +93,10 @@ export class Lexer { this.options = this.sanitizeOptions(options); this.start = 0; this.current = 0; - this.lineBegin = 0; - this.lineEnd = 0; - this.columnBegin = 0; - this.columnEnd = 0; + this.lineBegin = this.options.startLine ?? 0; + this.lineEnd = this.options.startLine ?? 0; + this.columnBegin = this.options.startCharacter ?? 0; + this.columnEnd = this.options.startCharacter ?? 0; this.tokens = []; this.diagnostics = []; while (!this.isAtEnd()) { @@ -1102,4 +1102,17 @@ export interface ScanOptions { * @default true */ trackLocations?: boolean; + /** + * The zero-indexed line number to start position tracking from. + * Useful when scanning a fragment of a larger file (e.g. an inline CDATA block) + * so that all token ranges are in the coordinate space of the parent file. + * @default 0 + */ + startLine?: number; + /** + * The zero-indexed character offset to start position tracking from on the first line. + * Only applies to the first line; subsequent lines always start at column 0. + * @default 0 + */ + startCharacter?: number; } From 99f3d8f7aa52a997788d7be463ea486776d1341e Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Mon, 30 Mar 2026 20:34:21 -0300 Subject: [PATCH 16/20] unit tests --- src/files/XmlFile.spec.ts | 41 +++++++++++++++++++++++++++++++++++++++ src/lexer/Lexer.spec.ts | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index 16c0ef25c..ed46fa7c3 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -1407,6 +1407,47 @@ describe('XmlFile', () => { expect(program.getFile('components/MyComp.cdata-1.script.bs')).to.exist; }); + it('synthetic BrsFile token ranges start at the CDATA position in the XML file', () => { + // The ('components/MyComp.xml', trim` + + + + + `); + const brsFile = program.getFile(xmlFile.inlineScriptPkgPaths[0]); + // line 2 is where { + // fileContents must be indexable by XML-space line numbers so that tools like + // SignatureHelpUtil can extract the function signature text correctly. + const xmlFile = program.setFile('components/MyComp.xml', trim` + + + + + `); + const brsFile = program.getFile(xmlFile.inlineScriptPkgPaths[0]); + const lines = brsFile.fileContents.split(/\r?\n/g); + // line 3 (0-indexed) of the XML is " sub greet(name as string)" + expect(lines[3]).to.include('sub greet'); + }); + it('transpile preserves CDATA blocks inline in the xml output', () => { testTranspile(trim` diff --git a/src/lexer/Lexer.spec.ts b/src/lexer/Lexer.spec.ts index 6dfaf2c3f..e4bfdaccf 100644 --- a/src/lexer/Lexer.spec.ts +++ b/src/lexer/Lexer.spec.ts @@ -1225,6 +1225,40 @@ describe('lexer', () => { Range.create(2, 7, 2, 8) // EOF ]); }); + + it('offsets all token ranges by startLine and startCharacter', () => { + // Simulates scanning an inline CDATA fragment that begins on line 5, col 10 of a parent file. + // The first character of the source is a newline (right after ({ kind: t.kind, range: t.range }))).to.eql([ + { kind: TokenKind.Newline, range: Range.create(5, 10, 5, 11) }, + { kind: TokenKind.Sub, range: Range.create(6, 0, 6, 3) }, + { kind: TokenKind.Identifier, range: Range.create(6, 4, 6, 7) }, // foo + { kind: TokenKind.LeftParen, range: Range.create(6, 7, 6, 8) }, + { kind: TokenKind.RightParen, range: Range.create(6, 8, 6, 9) }, + { kind: TokenKind.Newline, range: Range.create(6, 9, 6, 10) }, + { kind: TokenKind.EndSub, range: Range.create(7, 0, 7, 7) }, + { kind: TokenKind.Eof, range: Range.create(7, 7, 7, 8) } + ]); + }); + + it('produces no tokens before startLine when scanning a fragment with a leading newline', () => { + // The leading newline is at the startLine position — no tokens at lines 0..startLine-1. + const { tokens } = Lexer.scan('\nsub foo()\nend sub', { + includeWhitespace: false, + startLine: 5, + startCharacter: 10 + }); + for (const token of tokens) { + expect(token.range.start.line).to.be.at.least(5); + } + }); }); describe('two word keywords', () => { From 67f55dc25e1065284a881e8401686f116bfafea0 Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Mon, 30 Mar 2026 20:57:11 -0300 Subject: [PATCH 17/20] Code review improvements: docs, cdataText regex, and setFileInternal refactor - Add JSDoc to all plugin events that may receive a synthetic BrsFile (afterFileParse, validate hooks, all LSP events) explaining the new CDATA behavior so plugin authors know event.file may be isSynthetic - Replace fragile two-pass cdataText regex with a single capture-group match (/^/) that handles >, trailing whitespace, and empty blocks correctly; add five SGParser.spec tests - Remove _pendingSyntheticSrcPaths side-channel in favor of a configure callback param on setFileInternal so isSynthetic is set cleanly before afterFileParse fires without program-level mutable state - Restore blank lines between before/on/after emit calls in getCompletions Co-Authored-By: Claude Sonnet 4.6 --- src/Program.ts | 29 ++++++---------- src/interfaces.ts | 46 ++++++++++++++++++++++---- src/parser/SGParser.spec.ts | 66 +++++++++++++++++++++++++++++++++++++ src/parser/SGTypes.ts | 8 +++-- 4 files changed, 121 insertions(+), 28 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index c581890e0..5380609f0 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -181,14 +181,6 @@ export class Program { */ private _cdataDiagnosticsContext: BrsFile | undefined; - /** - * Set of absolute srcPaths for BrsFiles that are about to be registered as synthetic (CDATA) - * files. Populated before `setFile` is called so that `isSynthetic` can be pre-applied inside - * `setFile` before `afterFileParse` fires — otherwise `emitWithSyntheticFileContext` would see - * `isSynthetic === false` and skip the diagnostic-context guard. - */ - private _pendingSyntheticSrcPaths = new Set(); - private scopes = {} as Record; protected addScope(scope: Scope) { @@ -563,7 +555,7 @@ export class Program { * only used when registering synthetic inline CDATA BrsFiles so that the lexer can start its * position tracking at the correct XML-space offset without needing a side-channel map. */ - private setFileInternal(fileParam: FileObj | string, fileContents: string, parseOptions?: { startLine?: number; startCharacter?: number; rawSource?: string }): T { + private setFileInternal(fileParam: FileObj | string, fileContents: string, parseOptions?: { startLine?: number; startCharacter?: number; rawSource?: string }, configure?: (file: BrsFile) => void): T { //normalize the file paths const { srcPath, pkgPath } = this.getPaths(fileParam, this.options.rootDir); @@ -581,12 +573,11 @@ export class Program { new BrsFile(srcPath, pkgPath, this) ); - // Pre-mark as synthetic before parsing so that `afterFileParse` fires with - // `isSynthetic === true`, allowing `emitWithSyntheticFileContext` to correctly - // set `_cdataDiagnosticsContext` for any plugins that call `getDiagnostics()`. - if (this._pendingSyntheticSrcPaths.has(srcPath)) { - brsFile.isSynthetic = true; - } + // Apply any caller-provided configuration (e.g. marking synthetic files) before + // parsing so that `afterFileParse` fires with the correct state — allowing + // `emitWithSyntheticFileContext` to set `_cdataDiagnosticsContext` for plugins + // that call `getDiagnostics()` from inside the handler. + configure?.(brsFile); //add file to the `source` dependency list if (brsFile.pkgPath.startsWith(startOfSourcePkgPath)) { @@ -663,15 +654,13 @@ export class Program { const paddedContent = '\n'.repeat(cdataRange.start.line) + ' '.repeat(contentStartChar) + rawSource; - const inlineSrcPath = s`${path.resolve(this.options.rootDir, inlinePkgPath)}`; - this._pendingSyntheticSrcPaths.add(inlineSrcPath); const inlineFile = this.setFileInternal(inlinePkgPath, paddedContent, { startLine: cdataRange.start.line, startCharacter: contentStartChar, rawSource: rawSource + }, (file) => { + file.isSynthetic = true; }); - this._pendingSyntheticSrcPaths.delete(inlineSrcPath); - // isSynthetic was pre-applied inside setFile (see _pendingSyntheticSrcPaths) inlineFile.excludeFromOutput = true; inlineFile.parentXmlFile = xmlFile; inlineFile.cdataScript = script; @@ -1193,7 +1182,9 @@ export class Program { }; this.plugins.emit('beforeProvideCompletions', event); + this.plugins.emit('provideCompletions', event); + this.plugins.emit('afterProvideCompletions', event); return event.completions; diff --git a/src/interfaces.ts b/src/interfaces.ts index 79137bb1d..758266c79 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -319,17 +319,27 @@ export interface Plugin { afterScopeValidate?: ValidateHandler; //file events beforeFileParse?: (source: SourceObj) => void; + /** + * Called after each file has been parsed. When a SceneGraph XML file contains inline + * `` script blocks, this event is also fired once per CDATA block with a + * synthetic `BrsFile` as the argument (`file.isSynthetic === true`). The synthetic file's + * positions are already in the parent XML file's coordinate space. + */ afterFileParse?: (file: BscFile) => void; /** - * Called before each file is validated + * Called before each file is validated. For SceneGraph XML files with inline CDATA script + * blocks, this is also called for each synthetic `BrsFile` (`file.isSynthetic === true`). */ beforeFileValidate?: PluginHandler; /** * Called during the file validation process. If your plugin contributes file validations, this is a good place to contribute them. + * For SceneGraph XML files with inline CDATA script blocks, this is also called for each + * synthetic `BrsFile` (`event.file.isSynthetic === true`). */ onFileValidate?: PluginHandler; /** - * Called after each file is validated + * Called after each file is validated. For SceneGraph XML files with inline CDATA script + * blocks, this is also called for each synthetic `BrsFile` (`file.isSynthetic === true`). */ afterFileValidate?: (file: BscFile) => void; beforeFileTranspile?: PluginHandler; @@ -341,6 +351,11 @@ export type PluginHandler = (event: T) => R; export interface OnGetCodeActionsEvent { program: Program; + /** + * The file the code action request was invoked in. When the range falls inside a + * `` block of an XML file, this will be the synthetic `BrsFile` for that + * block (`file.isSynthetic === true`) rather than the `XmlFile` itself. + */ file: BscFile; range: Range; scopes: Scope[]; @@ -350,6 +365,11 @@ export interface OnGetCodeActionsEvent { export interface ProvideCompletionsEvent { program: Program; + /** + * The file the completion request was invoked in. When the cursor position falls inside a + * `` block of an XML file, this will be the synthetic `BrsFile` for that + * block (`file.isSynthetic === true`) rather than the `XmlFile` itself. + */ file: TFile; scopes: Scope[]; position: Position; @@ -360,6 +380,11 @@ export type AfterProvideCompletionsEvent = Prov export interface ProvideHoverEvent { program: Program; + /** + * The file the hover request was invoked in. When the cursor position falls inside a + * `` block of an XML file, this will be the synthetic `BrsFile` for that + * block (`file.isSynthetic === true`) rather than the `XmlFile` itself. + */ file: BscFile; position: Position; scopes: Scope[]; @@ -386,7 +411,9 @@ export type AfterProvideHoverEvent = ProvideHoverEvent; export interface ProvideDefinitionEvent { program: Program; /** - * The file that the getDefinition request was invoked in + * The file that the getDefinition request was invoked in. When the cursor position falls inside + * a `` block of an XML file, this will be the synthetic `BrsFile` for that + * block (`file.isSynthetic === true`) rather than the `XmlFile` itself. */ file: TFile; /** @@ -404,7 +431,9 @@ export type AfterProvideDefinitionEvent = ProvideDefinitionEven export interface ProvideReferencesEvent { program: Program; /** - * The file that the getDefinition request was invoked in + * The file that the getReferences request was invoked in. When the cursor position falls inside + * a `` block of an XML file, this will be the synthetic `BrsFile` for that + * block (`file.isSynthetic === true`) rather than the `XmlFile` itself. */ file: TFile; /** @@ -423,7 +452,10 @@ export type AfterProvideReferencesEvent = ProvideReferencesEven export interface ProvideDocumentSymbolsEvent { program: Program; /** - * The file that the `documentSymbol` request was invoked in + * The file that the `documentSymbol` request was invoked in. For SceneGraph XML files with + * inline CDATA script blocks, this event is also emitted once per block with the synthetic + * `BrsFile` (`file.isSynthetic === true`). Symbol ranges are already in the parent XML + * coordinate space. */ file: TFile; /** @@ -452,7 +484,9 @@ export interface OnGetSemanticTokensEvent { */ program: Program; /** - * The file to get semantic tokens for + * The file to get semantic tokens for. For SceneGraph XML files with inline CDATA script + * blocks, this event is also emitted once per block with the synthetic `BrsFile` + * (`file.isSynthetic === true`). Token ranges are already in the parent XML coordinate space. */ file: T; /** diff --git a/src/parser/SGParser.spec.ts b/src/parser/SGParser.spec.ts index 22f7f451e..743db90a9 100644 --- a/src/parser/SGParser.spec.ts +++ b/src/parser/SGParser.spec.ts @@ -158,4 +158,70 @@ describe('SGParser', () => { range: Range.create(0, 0, 1, 12) }); }); + + describe('SGScript.cdataText', () => { + function parseScript(xml: string) { + const parser = new SGParser(); + parser.parse('pkg:/components/Comp.xml', xml); + return parser.ast.component?.scripts?.[0]; + } + + it('strips from multiline content', () => { + const script = parseScript(trim` + + + + + `); + // cdataText should not contain the delimiters; whitespace inside is preserved as-is + expect(script?.cdataText).to.not.include(''); + expect(script?.cdataText?.trim()).to.equal('function init()\n end function'); + }); + + it('strips from inline (single-line) content', () => { + const script = parseScript(trim` + + + + + `); + expect(script?.cdataText).to.equal('function init() : end function'); + }); + + it('returns empty string for empty CDATA block', () => { + const script = parseScript(trim` + + + + + `); + expect(script?.cdataText).to.equal(''); + }); + + it('preserves > characters inside CDATA content', () => { + const script = parseScript(trim` + + + + + `); + expect(script?.cdataText).to.equal('if x > 5 then : end if'); + }); + + it('returns undefined when there is no CDATA block', () => { + const script = parseScript(trim` + + + + + `); + program.validate(); + expectZeroDiagnostics(program); + // line 1 of MyComp.brs is " greet("world")" — col 10 is inside the arg list + const help = program.getSignatureHelp('components/MyComp.brs', util.createPosition(1, 10)); + expect(help).to.have.length.greaterThan(0); + expect(help[0].signature.label).to.equal('sub greet(name as string)'); }); it('transpile preserves CDATA blocks inline in the xml output', () => { From 5cba030aae3d9beaa5bc4c72a090b26359bfcb1c Mon Sep 17 00:00:00 2001 From: Christopher Dwyer-Perkins Date: Mon, 30 Mar 2026 21:17:56 -0300 Subject: [PATCH 19/20] lintting --- src/files/XmlFile.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index 5d32745e9..de3b91396 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -1445,7 +1445,7 @@ describe('XmlFile', () => { const brsFile = program.getFile(xmlFile.inlineScriptPkgPaths[0]); // fileContents is the raw CDATA source, not padded with leading newlines expect(brsFile.fileContents).to.include('sub greet'); - expect(brsFile.fileContents.split(/\r?\n/g)[0]).to.equal(''); // first "line" is the \n right after Date: Mon, 30 Mar 2026 21:21:51 -0300 Subject: [PATCH 20/20] Fix lint: use RegExp#exec() instead of String#match() Co-Authored-By: Claude Sonnet 4.6 --- src/parser/SGTypes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/SGTypes.ts b/src/parser/SGTypes.ts index dad6036ed..ea6a430d9 100644 --- a/src/parser/SGTypes.ts +++ b/src/parser/SGTypes.ts @@ -172,7 +172,7 @@ export class SGScript extends SGTag { // Use a capture group with [\s\S]*? so multiline content and edge cases // (e.g. trailing whitespace after ]]>, or > characters inside the content) are handled // correctly without relying on ^ / $ anchors that can misbehave at string boundaries. - const match = this.cdata?.text.match(/^/); + const match = /^/.exec(this.cdata?.text); return match?.[1]; }