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
+    }),
+    xmlInvalidFieldValue: (fieldName: string, expectedType: string) => ({
+        message: `Value for field '${fieldName}' is not a valid '${expectedType}'`,
+        code: 1157,
+        severity: DiagnosticSeverity.Error
+    }),
+    xmlStartEndTagMismatch: (openName: string, closeName: string) => ({
+        message: `Opening tag '${openName}' does not match closing tag '${closeName}'`,
+        code: 1158,
+        severity: DiagnosticSeverity.Error
     })
 };
 

src/bscPlugin/validation/XmlFieldTypeValidator.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,142 @@
+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('does not flag a case-differing field name (no longer a diagnostic)', () => {
+        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)).to.be.empty;
+    });
+
+    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(1157);
+        expect(scenegraphDiagnostics(goodFile)).to.be.empty;
+    });
+
+    it('does not flag component-library components', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <MyLib:SomeView />
+                </children>
+            </component>
+        `);
+        program.validate();
+        //complib-scoped names (containing ':') can't be resolved yet, so they should not be flagged
+        expect(scenegraphDiagnostics(file)).to.be.empty;
+    });
+
+    it('lets a plugin claim an attribute to opt out of field validation', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <Label data-custom="x" />
+                </children>
+            </component>
+        `);
+        program.plugins.add({
+            name: 'claims custom attributes',
+            onValidateXmlAttribute: (event) => {
+                if (event.attribute.key.text.startsWith('data-')) {
+                    event.handled = true;
+                }
+            }
+        });
+        program.validate();
+        //the plugin claimed `data-custom`, so brighterscript should not flag it as an unknown field
+        expect(scenegraphDiagnostics(file)).to.be.empty;
+    });
+
+    it('flags mismatched opening and closing tag names', () => {
+        const file = program.setFile<XmlFile>('components/main.xml', trim`
+            <component name="Main" extends="Group">
+                <children>
+                    <Rectangle></rectangle>
+                </children>
+            </component>
+        `);
+        program.validate();
+        expect(scenegraphDiagnostics(file).map(x => x.code)).to.include(1158);
+    });
+});

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, SGAttribute, SGNode } from '../../parser/SGTypes';
 import util from '../../util';
+import { isValidSceneGraphFieldValue } from './XmlFieldTypeValidator';
 
 export class XmlFileValidator {
     constructor(
@@ -60,6 +61,81 @@ 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 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);
+        //skip component-library components (e.g. `ComplibName:SomeView`); we can't resolve those yet
+        } else if (!nodeName.includes(':')) {
+            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 ?? []) {
+            //let plugins claim an attribute (e.g. custom/transformed attributes) before we validate it
+            if (this.isAttributeHandledByPlugin(node, attribute)) {
+                continue;
+            }
+            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.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
+                });
+            }
+        }
+    }
+
+    /**
+     * Emit the `onValidateXmlAttribute` event so plugins can claim an attribute (returning `handled: true`)
+     * to opt it out of brighterscript's built-in field validation.
+     */
+    private isAttributeHandledByPlugin(node: SGNode, attribute: SGAttribute): boolean {
+        const event = {
+            program: this.event.program,
+            file: this.event.file,
+            node: node,
+            attribute: attribute,
+            handled: false
+        };
+        this.event.program.plugins.emit('onValidateXmlAttribute', event);
+        return event.handled;
     }
 
 }

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');

src/interfaces.ts

@@ -8,6 +8,7 @@ import type { ParseMode } from './parser/Parser';
 import type { Program, SourceObj, TranspileObj } from './Program';
 import type { ProgramBuilder } from './ProgramBuilder';
 import type { FunctionStatement } from './parser/Statement';
+import type { SGAttribute, SGNode } from './parser/SGTypes';
 import type { Expression } from './parser/AstNode';
 import type { TranspileState } from './parser/TranspileState';
 import type { SourceMapGenerator, SourceNode } from 'source-map';
@@ -390,6 +391,12 @@ export interface Plugin {
      * Called during the file validation process. If your plugin contributes file validations, this is a good place to contribute them.
      */
     onFileValidate?: PluginHandler<OnFileValidateEvent>;
+    /**
+     * Called while validating each attribute on a SceneGraph node in an xml component. Set `handled: true`
+     * to claim the attribute so brighterscript skips its own field validation of it (useful for plugins
+     * that inject or transform custom attributes).
+     */
+    onValidateXmlAttribute?: PluginHandler<OnValidateXmlAttributeEvent>;
     /**
      * Called after each file is validated
      */
@@ -593,6 +600,23 @@ export interface OnFileValidateEvent<T extends BscFile = BscFile> {
     file: T;
 }
 
+export interface OnValidateXmlAttributeEvent {
+    program: Program;
+    file: XmlFile;
+    /**
+     * The SceneGraph node the attribute is declared on
+     */
+    node: SGNode;
+    /**
+     * The attribute being validated
+     */
+    attribute: SGAttribute;
+    /**
+     * Set to `true` from a plugin to claim this attribute, so brighterscript skips its own validation of it
+     */
+    handled: boolean;
+}
+
 export interface OnScopeValidateEvent {
     program: Program;
     scope: Scope;