diff --git a/src/DiagnosticMessages.ts b/src/DiagnosticMessages.ts index cd81e60bc..86a619121 100644 --- a/src/DiagnosticMessages.ts +++ b/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 }) }; diff --git a/src/bscPlugin/validation/XmlFieldTypeValidator.ts b/src/bscPlugin/validation/XmlFieldTypeValidator.ts new file mode 100644 index 000000000..7bc2688d8 --- /dev/null +++ b/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 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) +}; diff --git a/src/bscPlugin/validation/XmlFileValidator.spec.ts b/src/bscPlugin/validation/XmlFileValidator.spec.ts new file mode 100644 index 000000000..73765c135 --- /dev/null +++ b/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 ', () => { + const file = program.setFile('components/main.xml', trim` + + + + + + `); + 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` + + + `); + const file = program.setFile('components/main.xml', trim` + + + + + `); + program.validate(); + expect(scenegraphDiagnostics(file)).to.be.empty; + }); + + it('flags an unknown field on a known node', () => { + const file = program.setFile('components/main.xml', trim` + + + + + `); + 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('components/main.xml', trim` + + + + + `); + 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('components/bad.xml', trim` + + + + + `); + const goodFile = program.setFile('components/good.xml', trim` + + + + + `); + 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('components/main.xml', trim` + + + + + + `); + 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('components/main.xml', trim` + + + + + `); + 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('components/main.xml', trim` + + + + + + `); + program.validate(); + expect(scenegraphDiagnostics(file).map(x => x.code)).to.include(1158); + }); +}); diff --git a/src/bscPlugin/validation/XmlFileValidator.ts b/src/bscPlugin/validation/XmlFileValidator.ts index 44912dced..979e639db 100644 --- a/src/bscPlugin/validation/XmlFileValidator.ts +++ b/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 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; } } diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index 373d87633..27ccc2b81 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -679,7 +679,7 @@ describe('XmlFile', () => { - + `, trim` @@ -687,7 +687,7 @@ describe('XmlFile', () => {