Fix mismatched XML element pairs on transpile

src/DiagnosticMessages.ts

@@ -833,6 +833,11 @@ export let DiagnosticMessages = {
         message: `rsg_version=${rsgVersion} was removed in firmware ${removedAt}; use rsg_version=${replacement}`,
         code: 1154,
         severity: DiagnosticSeverity.Error
+    }),
+    xmlTagMismatch: (openingTag: string, closingTag: string) => ({
+        message: `Mismatched closing tag: expected '</${openingTag}>' but found '</${closingTag}>'`,
+        code: 1155,
+        severity: DiagnosticSeverity.Error
     })
 };
 

src/bscPlugin/validation/XmlFileValidator.ts

@@ -1,7 +1,7 @@
 import { DiagnosticMessages } from '../../DiagnosticMessages';
 import type { XmlFile } from '../../files/XmlFile';
 import type { OnFileValidateEvent } from '../../interfaces';
-import type { SGAst } from '../../parser/SGTypes';
+import type { SGAst, SGTag } from '../../parser/SGTypes';
 import util from '../../util';
 
 export class XmlFileValidator {
@@ -14,11 +14,34 @@ export class XmlFileValidator {
         util.validateTooDeepFile(this.event.file);
         if (this.event.file.parser.ast.root) {
             this.validateComponent(this.event.file.parser.ast);
+            this.validateTagClosings(this.event.file.parser.ast.root);
         } else {
             //skip empty XML
         }
     }
 
+    /**
+     * Walk the SG tag tree and report any tag whose closing tag name does not
+     * match its opening tag name (e.g. `<Group></LayoutGroup>`). This runs at
+     * validation time (rather than parse time) so it also catches mismatches in
+     * AST injected by plugins, not just AST produced by the parser.
+     */
+    private validateTagClosings(tag: SGTag) {
+        const closingTagText = tag.closingTag?.text;
+        //only validate when a closing tag was actually present (self-closing and
+        //programmatically-built tags omit it, and must remain valid)
+        if (closingTagText !== undefined && closingTagText !== tag.tag.text) {
+            this.event.file.diagnostics.push({
+                ...DiagnosticMessages.xmlTagMismatch(tag.tag.text, closingTagText),
+                range: tag.closingTag.range,
+                file: this.event.file
+            });
+        }
+        for (const child of tag.getChildren()) {
+            this.validateTagClosings(child);
+        }
+    }
+
     private validateComponent(ast: SGAst) {
         const { root, component } = ast;
         if (!component) {

src/files/XmlFile.spec.ts

@@ -669,6 +669,130 @@ describe('XmlFile', () => {
             `, 'none', 'components/Comp.xml');
         });
 
+        it('transpiles mismatched tags correctly using opening tag', () => {
+            const file = program.setFile('components/Comp.xml', trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <children>
+                        <Group id="myGroup">
+                        </LayoutGroup>
+                    </children>
+                </component>
+            `);
+            file.needsTranspiled = true;
+            program.validate();
+
+            // Should have a diagnostic for the mismatch
+            expect(file.diagnostics).to.have.lengthOf(1);
+            expect(file.diagnostics[0]).to.deep.include({
+                ...DiagnosticMessages.xmlTagMismatch('Group', 'LayoutGroup')
+            });
+
+            // But transpile should still work correctly (self-closing since no children)
+            const transpiled = file.transpile();
+            expect(trimMap(transpiled.code)).to.equal(trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <script type="text/brightscript" uri="pkg:/source/bslib.brs" />
+                    <children>
+                        <Group id="myGroup" />
+                    </children>
+                </component>
+            `);
+        });
+
+        it('transpiles mismatched tags with children correctly using opening tag', () => {
+            const file = program.setFile('components/Comp.xml', trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <children>
+                        <Group id="myGroup">
+                            <Label text="hello" />
+                        </LayoutGroup>
+                    </children>
+                </component>
+            `);
+            file.needsTranspiled = true;
+            program.validate();
+
+            // Should have a diagnostic for the mismatch
+            expect(file.diagnostics).to.have.lengthOf(1);
+            expect(file.diagnostics[0]).to.deep.include({
+                ...DiagnosticMessages.xmlTagMismatch('Group', 'LayoutGroup')
+            });
+
+            // Transpile should use the opening tag for closing, not the mismatched closing tag
+            const transpiled = file.transpile();
+            expect(trimMap(transpiled.code)).to.equal(trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <script type="text/brightscript" uri="pkg:/source/bslib.brs" />
+                    <children>
+                        <Group id="myGroup">
+                            <Label text="hello" />
+                        </Group>
+                    </children>
+                </component>
+            `);
+        });
+
+        it('does not emit a mismatch diagnostic for well-formed matching tags', () => {
+            const file = program.setFile<XmlFile>('components/Comp.xml', trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <children>
+                        <Group id="myGroup">
+                            <Label text="hello" />
+                        </Group>
+                    </children>
+                </component>
+            `);
+            program.validate();
+            expectZeroDiagnostics(file);
+        });
+
+        it('does not emit a mismatch diagnostic for self-closing tags', () => {
+            const file = program.setFile<XmlFile>('components/Comp.xml', trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <children>
+                        <Group id="myGroup" />
+                    </children>
+                </component>
+            `);
+            program.validate();
+            expectZeroDiagnostics(file);
+        });
+
+        it('catches mismatched tags injected via a plugin (no closingTag on the parser AST)', () => {
+            //this proves the check runs at validation time against the stored closingTag,
+            //so it catches AST mutated/injected by plugins, not just parser output
+            program.plugins.add({
+                name: 'inject-mismatched-closing-tag',
+                afterFileParse: (file) => {
+                    if (isXmlFile(file)) {
+                        const group = file.parser.ast.component?.children?.children?.[0];
+                        if (group) {
+                            //plugin sets a mismatched closing tag programmatically
+                            group.closingTag = { text: 'LayoutGroup' };
+                        }
+                    }
+                }
+            });
+            const file = program.setFile<XmlFile>('components/Comp.xml', trim`
+                <?xml version="1.0" encoding="utf-8" ?>
+                <component name="Comp" extends="Group">
+                    <children>
+                        <Group id="myGroup" />
+                    </children>
+                </component>
+            `);
+            program.validate();
+            expectDiagnostics(file, [
+                DiagnosticMessages.xmlTagMismatch('Group', 'LayoutGroup')
+            ]);
+        });
+
         it('does not include additional bslib script if already there ', () => {
             testTranspile(trim`
                 <?xml version="1.0" encoding="utf-8" ?>

src/parser/SGParser.spec.ts

@@ -158,4 +158,42 @@ describe('SGParser', () => {
             range: Range.create(0, 0, 1, 12)
         });
     });
+
+    it('captures the closing tag name on the AST when it mismatches', () => {
+        const parser = new SGParser();
+        parser.parse(
+            'pkg:/components/ParentScene.xml', trim`
+            <?xml version="1.0" encoding="utf-8" ?>
+            <component name="ChildScene" extends="ParentScene">
+                <children>
+                    <Group id="myGroup">
+                    </LayoutGroup>
+                </children>
+            </component>
+        `);
+        //parsing does not emit the mismatch diagnostic; that happens at validation time
+        expect(parser.diagnostics).to.be.lengthOf(0);
+        //but the parser should capture the (mismatched) closing tag so validation can inspect it
+        const group = parser.ast.component.children.children[0];
+        expect(group.tag.text).to.equal('Group');
+        expect(group.closingTag?.text).to.equal('LayoutGroup');
+        expect(group.closingTag?.range).to.eql(Range.create(4, 10, 4, 21));
+    });
+
+    it('leaves closingTag undefined for self-closing tags', () => {
+        const parser = new SGParser();
+        parser.parse(
+            'pkg:/components/ParentScene.xml', trim`
+            <?xml version="1.0" encoding="utf-8" ?>
+            <component name="ChildScene" extends="ParentScene">
+                <children>
+                    <Group id="myGroup" />
+                </children>
+            </component>
+        `);
+        expect(parser.diagnostics).to.be.lengthOf(0);
+        const group = parser.ast.component.children.children[0];
+        expect(group.tag.text).to.equal('Group');
+        expect(group.closingTag).to.be.undefined;
+    });
 });

src/parser/SGParser.ts

@@ -175,6 +175,7 @@ interface IToken {
 
 function mapElement({ children }: ElementCstNode, diagnostics: Diagnostic[]): SGTag {
     const nameToken = children.Name[0];
+    const closingNameToken = children.END_NAME?.[0];
     let range: Range;
     const selfClosing = !!children.SLASH_CLOSE;
     if (selfClosing) {
@@ -185,37 +186,39 @@ function mapElement({ children }: ElementCstNode, diagnostics: Diagnostic[]): SG
         range = rangeFromTokens(nameToken, endToken);
     }
     const name = mapToken(nameToken);
+    const closingName = closingNameToken ? mapToken(closingNameToken) : undefined;
+
     const attributes = mapAttributes(children.attribute);
     const content = children.content?.[0];
     switch (name.text) {
         case 'component':
             const componentContent = mapElements(content, ['interface', 'script', 'children', 'customization'], diagnostics);
-            return new SGComponent(name, attributes, componentContent, range);
+            return new SGComponent(name, attributes, componentContent, range, closingName);
         case 'interface':
             const interfaceContent = mapElements(content, ['field', 'function'], diagnostics);
-            return new SGInterface(name, interfaceContent, range);
+            return new SGInterface(name, interfaceContent, range, closingName);
         case 'field':
             if (hasElements(content)) {
                 reportUnexpectedChildren(name, diagnostics);
             }
-            return new SGField(name, attributes, range);
+            return new SGField(name, attributes, range, closingName);
         case 'function':
             if (hasElements(content)) {
                 reportUnexpectedChildren(name, diagnostics);
             }
-            return new SGFunction(name, attributes, range);
+            return new SGFunction(name, attributes, range, closingName);
         case 'script':
             if (hasElements(content)) {
                 reportUnexpectedChildren(name, diagnostics);
             }
             const cdata = getCdata(content);
-            return new SGScript(name, attributes, cdata, range);
+            return new SGScript(name, attributes, cdata, range, closingName);
         case 'children':
             const childrenContent = mapNodes(content);
-            return new SGChildren(name, childrenContent, range);
+            return new SGChildren(name, childrenContent, range, closingName);
         default:
             const nodeContent = mapNodes(content);
-            return new SGNode(name, attributes, nodeContent, range);
+            return new SGNode(name, attributes, nodeContent, range, closingName);
     }
 }
 
@@ -228,6 +231,7 @@ function reportUnexpectedChildren(name: SGToken, diagnostics: Diagnostic[]) {
 
 function mapNode({ children }: ElementCstNode): SGNode {
     const nameToken = children.Name[0];
+    const closingNameToken = children.END_NAME?.[0];
     let range: Range;
     const selfClosing = !!children.SLASH_CLOSE;
     if (selfClosing) {
@@ -238,10 +242,12 @@ function mapNode({ children }: ElementCstNode): SGNode {
         range = rangeFromTokens(nameToken, endToken);
     }
     const name = mapToken(nameToken);
+    const closingName = closingNameToken ? mapToken(closingNameToken) : undefined;
+
     const attributes = mapAttributes(children.attribute);
     const content = children.content?.[0];
     const nodeContent = mapNodes(content);
-    return new SGNode(name, attributes, nodeContent, range);
+    return new SGNode(name, attributes, nodeContent, range, closingName);
 }
 
 function mapElements(content: ContentCstNode, allow: string[], diagnostics: Diagnostic[]): SGTag[] {