Validate SceneGraph node types and fields in xml components

src/DiagnosticMessages.ts

@@ -833,6 +833,26 @@ export let DiagnosticMessages = {
         message: `rsg_version=${rsgVersion} was removed in firmware ${removedAt}; use rsg_version=${replacement}`,
         code: 1154,
         severity: DiagnosticSeverity.Error
+    }),
+    xmlUnknownComponentType: (nodeName: string) => ({
+        message: `Unknown component type '${nodeName}'`,
+        code: 1155,
+        severity: DiagnosticSeverity.Error
+    }),
+    xmlUnknownField: (fieldName: string, componentName: string) => ({
+        message: `Field '${fieldName}' does not exist on component '${componentName}'`,
+        code: 1156,
+        severity: DiagnosticSeverity.Error
+    }),
+    xmlFieldNameCaseMismatch: (actualName: string, expectedName: string) => ({
+        message: `Field '${actualName}' should be '${expectedName}' to match the declared casing`,
+        code: 1157,
+        severity: DiagnosticSeverity.Warning
+    }),
+    xmlInvalidFieldValue: (fieldName: string, expectedType: string) => ({
+        message: `Value for field '${fieldName}' is not a valid '${expectedType}'`,
+        code: 1158,
+        severity: DiagnosticSeverity.Warning
     })
 };
 

src/bscPlugin/validation/FieldTypeValidator.ts

@@ -0,0 +1,42 @@
+/**
+ * Best-effort validation of a SceneGraph field value written as an xml attribute string.
+ *
+ * Intentionally conservative: only unambiguous scalar types are checked. Anything ambiguous (strings,
+ * nodes, uris, arrays, vectors, enums/option-strings, unknown types, etc.) passes, so we never emit a
+ * false positive for the many field types whose valid literal form is hard to pin down from xml alone.
+ *
+ * @param value the (unquoted) attribute value
+ * @param type the field's declared type (e.g. `integer`, `float`, `boolean`, `color`, `option string`)
+ * @returns true when the value is valid for the type, or when the type isn't one we confidently validate
+ */
+export function isValidSceneGraphFieldValue(value: string, type: string): boolean {
+    const validator = scalarValidators[normalizeFieldType(type)];
+    //no validator for this type -> assume valid
+    return validator ? validator(value ?? '') : true;
+}
+
+function normalizeFieldType(type: string): string {
+    let normalized = (type ?? '').trim().toLowerCase();
+    //"option string", "value string", etc. are all just strings
+    if (normalized.endsWith(' string')) {
+        normalized = 'string';
+    }
+    return normalized;
+}
+
+const integerPattern = /^[+-]?\d+$/;
+const floatPattern = /^[+-]?(\d+\.?\d*|\.\d+)$/;
+const booleanPattern = /^(true|false)$/i;
+const colorPattern = /^(#[0-9a-f]{3,4}|#[0-9a-f]{6}|#[0-9a-f]{8}|0x[0-9a-f]{6}|0x[0-9a-f]{8})$/i;
+
+const scalarValidators: Record<string, (value: string) => boolean> = {
+    integer: value => integerPattern.test(value),
+    int: value => integerPattern.test(value),
+    longinteger: value => integerPattern.test(value),
+    float: value => floatPattern.test(value),
+    double: value => floatPattern.test(value),
+    time: value => floatPattern.test(value),
+    boolean: value => booleanPattern.test(value),
+    bool: value => booleanPattern.test(value),
+    color: value => colorPattern.test(value)
+};

src/bscPlugin/validation/XmlFileValidator.spec.ts

@@ -0,0 +1,112 @@
+import { expect } from '../../chai-config.spec';
+import { Program } from '../../Program';
+import type { XmlFile } from '../../files/XmlFile';
+import type { BsDiagnostic } from '../../interfaces';
+import { trim, rootDir } from '../../testHelpers.spec';
+
+describe('XmlFileValidator', () => {
+    let program: Program;
+    beforeEach(() => {
+        program = new Program({ rootDir: rootDir });
+    });
+    afterEach(() => {
+        program.dispose();
+    });
+
+    //codes for the scenegraph node/field diagnostics added by this validator
+    const scenegraphCodes = [1155, 1156, 1157, 1158];
+    function scenegraphDiagnostics(file: XmlFile): BsDiagnostic[] {
+        return file.diagnostics.filter(diagnostic => scenegraphCodes.includes(diagnostic.code as number));
+    }
+
+    it('flags an unknown component type in <children>', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <Lable />
+                </children>
+            </component>
+        `);
+        program.validate();
+        const diagnostics = scenegraphDiagnostics(file);
+        expect(diagnostics.map(x => x.code)).to.include(1155);
+        expect(diagnostics[0].message).to.include('Lable');
+    });
+
+    it('does not flag known built-in nodes or project components', () => {
+        program.setFile('components/widget.xml', trim`
+            <component name="Widget" extends="Group">
+            </component>
+        `);
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <Label text="hi" />
+                    <Widget />
+                </children>
+            </component>
+        `);
+        program.validate();
+        expect(scenegraphDiagnostics(file)).to.be.empty;
+    });
+
+    it('flags an unknown field on a known node', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <Label bogusField="x" />
+                </children>
+            </component>
+        `);
+        program.validate();
+        expect(scenegraphDiagnostics(file).map(x => x.code)).to.include(1156);
+    });
+
+    it('flags a field name case mismatch', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <Label Text="hi" />
+                </children>
+            </component>
+        `);
+        program.validate();
+        expect(scenegraphDiagnostics(file).map(x => x.code)).to.include(1157);
+    });
+
+    it('flags a clearly-invalid scalar field value but allows valid and ambiguous ones', () => {
+        const badFile = program.setFile<XmlFile>('components/bad.xml', trim`
+            <component name="Bad" extends="Group">
+                <children>
+                    <Label opacity="halfway" />
+                </children>
+            </component>
+        `);
+        const goodFile = program.setFile<XmlFile>('components/good.xml', trim`
+            <component name="Good" extends="Group">
+                <children>
+                    <Label opacity="0.5" text="anything goes" />
+                </children>
+            </component>
+        `);
+        program.validate();
+        expect(scenegraphDiagnostics(badFile).map(x => x.code)).to.include(1158);
+        expect(scenegraphDiagnostics(goodFile)).to.be.empty;
+    });
+
+    it('validates fields on ContentNode against its known schema', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <ContentNode Title="Home" bogusField="x" />
+                </children>
+            </component>
+        `);
+        program.validate();
+        const codes = scenegraphDiagnostics(file).map(x => x.code);
+        //`Title` is a real ContentNode field, correctly cased -> no diagnostic
+        expect(codes).not.to.include(1157);
+        //`bogusField` is not a ContentNode field -> unknown field
+        expect(codes).to.include(1156);
+    });
+});

src/bscPlugin/validation/XmlFileValidator.ts

@@ -1,8 +1,9 @@
 import { DiagnosticMessages } from '../../DiagnosticMessages';
 import type { XmlFile } from '../../files/XmlFile';
 import type { OnFileValidateEvent } from '../../interfaces';
-import type { SGAst } from '../../parser/SGTypes';
+import type { SGAst, SGNode } from '../../parser/SGTypes';
 import util from '../../util';
+import { isValidSceneGraphFieldValue } from './FieldTypeValidator';
 
 export class XmlFileValidator {
     constructor(
@@ -60,6 +61,67 @@ export class XmlFileValidator {
                 range: explicitCodebehindScriptTag.filePathRange
             });
         }
+
+        //validate the node instances declared in the <children> block
+        for (const node of component.children?.children ?? []) {
+            this.validateNode(node);
+        }
+    }
+
+    /**
+     * Validate a single node instance and recurse into its children. Flags unknown node types, and (for
+     * known types) unknown/mis-cased field names and clearly-invalid field values.
+     */
+    private validateNode(node: SGNode) {
+        const nodeName = node.tag.text;
+        if (this.event.program.hasSceneGraphNode(nodeName)) {
+            this.validateNodeAttributes(node, nodeName);
+        } else {
+            this.event.file.diagnostics.push({
+                ...DiagnosticMessages.xmlUnknownComponentType(nodeName),
+                range: node.tag.range,
+                file: this.event.file
+            });
+        }
+        //recurse regardless, so nested typos are still reported
+        for (const child of node.children ?? []) {
+            this.validateNode(child);
+        }
+    }
+
+    private validateNodeAttributes(node: SGNode, nodeName: string) {
+        const fields = this.event.program.getSceneGraphNodeFields(nodeName);
+        //if we can't resolve any fields for a known node, don't guess (avoids mass false positives)
+        if (fields.length === 0) {
+            return;
+        }
+        const fieldsByLowerName = new Map(fields.map(field => [field.name.toLowerCase(), field]));
+        for (const attribute of node.attributes ?? []) {
+            const attributeName = attribute.key.text;
+            const field = fieldsByLowerName.get(attributeName.toLowerCase());
+            if (!field) {
+                this.event.file.diagnostics.push({
+                    ...DiagnosticMessages.xmlUnknownField(attributeName, nodeName),
+                    range: attribute.key.range,
+                    file: this.event.file
+                });
+                continue;
+            }
+            if (field.name !== attributeName) {
+                this.event.file.diagnostics.push({
+                    ...DiagnosticMessages.xmlFieldNameCaseMismatch(attributeName, field.name),
+                    range: attribute.key.range,
+                    file: this.event.file
+                });
+            }
+            if (field.type && !isValidSceneGraphFieldValue(attribute.value?.text, field.type)) {
+                this.event.file.diagnostics.push({
+                    ...DiagnosticMessages.xmlInvalidFieldValue(field.name, field.type),
+                    range: attribute.value?.range ?? attribute.key.range,
+                    file: this.event.file
+                });
+            }
+        }
     }
 
 }

src/files/XmlFile.spec.ts

@@ -679,15 +679,15 @@ describe('XmlFile', () => {
                 <?xml version="1.0" encoding="utf-8" ?>
                 <component name="AnimationExample" extends="Scene">
                     <children>
-                        <Animated frames='["pkg:/images/animation-1.png"]' />
+                        <Poster uri='["pkg:/images/animation-1.png"]' />
                     </children>
                 </component>
             `, trim`
                 <?xml version="1.0" encoding="utf-8" ?>
                 <component name="AnimationExample" extends="Scene">
                     <script type="text/brightscript" uri="pkg:/source/bslib.brs" />
                     <children>
-                        <Animated frames='["pkg:/images/animation-1.png"]' />
+                        <Poster uri='["pkg:/images/animation-1.png"]' />
                     </children>
                 </component>
             `, 'none', 'components/Comp.xml');
@@ -862,9 +862,9 @@ describe('XmlFile', () => {
                     </interface>
                     <script type="text/brightscript" uri="SimpleScene.bs"/>
                     <children>
-                        <aa id="aa">
-                            <bb id="bb" />
-                        </aa>
+                        <Group id="aa">
+                            <Rectangle id="bb" />
+                        </Group>
                     </children>
                 </component>
             `, trim`
@@ -877,9 +877,9 @@ describe('XmlFile', () => {
                     <script type="text/brightscript" uri="SimpleScene.brs" />
                     <script type="text/brightscript" uri="pkg:/source/bslib.brs" />
                     <children>
-                        <aa id="aa">
-                            <bb id="bb" />
-                        </aa>
+                        <Group id="aa">
+                            <Rectangle id="bb" />
+                        </Group>
                     </children>
                 </component>
             `, 'none', 'components/SimpleScene.xml');