Validate SceneGraph node types and fields in xml components#1742
Validate SceneGraph node types and fields in xml components#1742chrisdp wants to merge 6 commits into
Conversation
Provide IntelliSense in .xml component files, driven by the built-in roku-types node metadata and project components. - Program: getSceneGraphNodeNames/getSceneGraphNodeFields (resolves the extends chain across built-in nodes and project components), hasSceneGraphNode, getSceneGraphNode - XmlFile.getCompletions: element (tag name) completions after `<` and in element content, and field (attribute) completions inside a start tag; adds getTokenAt for token-at-position lookups - LanguageServer: add `<` as a completion trigger character - SGParser: skip malformed child elements with no tag name instead of throwing (a lone `<` while typing) - roku-types: add `extends` to SGNodeData
Walk the <children> node tree of each component and report: - unknown component types (error) - unknown field/attribute names on a known node (error) - field-name casing that differs from the declared field (warning) - clearly-invalid scalar field values (warning) Field/value checks are intentionally conservative: only int/float/bool/color values are validated (everything else passes), and field-name checks are skipped when no fields can be resolved for a known node, to avoid false positives. New diagnostics use codes 1155-1158.
Node and component element completions were appearing everywhere in element content (the component root, inside <interface>, etc.). Gate them on the enclosing element so they only appear inside a <children> block or nested inside another node; other locations now yield no node completions.
TwitchBronBron
added
the
create-package
|
Hey there! I just built a new temporary npm package based on 2e72bc9. You can download it here or install it by running the following command: npm install https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.73.0-feature-xml-validation.20260708122551.tgz |
| this.validateNodeAttributes(node, nodeName); | ||
| } else { | ||
| this.event.file.diagnostics.push({ | ||
| ...DiagnosticMessages.xmlUnknownComponentType(nodeName), |
There was a problem hiding this comment.
This works great for known xml files. However, for devs that use component libraries, all of their components are going to show up as errors. I think for now, we should avoid adding this diagnostic for ComplibPrefix:Whatever pattern components, until we have a way of declaring these in type definitions.
| } | ||
| } | ||
|
|
||
| private validateNodeAttributes(node: SGNode, nodeName: string) { |
There was a problem hiding this comment.
This works great for out-of-the-box attributes. But many dev teams have custom plugins that do things like handlebars replacements. Those would all be marked as warnings (or errors) with no way to granularly say "brighterscript can validate these, I will validate those others". maybe a plugin hook for validateSGAttribute or something that lets plugins tap in and claim an attribute before falling back to brighterscript?
| 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 |
| xmlFieldNameCaseMismatch: (actualName: string, expectedName: string) => ({ | ||
| message: `Field '${actualName}' should be '${expectedName}' to match the declared casing`, | ||
| code: 1157, | ||
| severity: DiagnosticSeverity.Warning | ||
| }), |
There was a problem hiding this comment.
This diagnostic belongs in eslint. it's a preference thing, not something that would cause runtime errors.
| code: 1155, | ||
| severity: DiagnosticSeverity.Error | ||
| }), | ||
| xmlUnknownField: (fieldName: string, componentName: string) => ({ |
There was a problem hiding this comment.
We need additional diagonstics for mismatched starting and ending xml attribute names
There was a problem hiding this comment.
Rename this to include xml in the file name. Maybe XmlFieldTypeValidator or something? We have lots of other validation files floating around in this folder.
- Skip the unknown-component diagnostic for component-library components (tag names containing ':'), which can't be resolved yet - Add an onValidateXmlAttribute plugin event so plugins can claim an attribute (handled: true) to opt it out of field validation - Promote xmlInvalidFieldValue from warning to error - Remove the field-name case-mismatch diagnostic (belongs in eslint) - Flag mismatched opening/closing tag names in SGParser (xml-tools matches them case-insensitively, but Roku does not) - Rename FieldTypeValidator -> XmlFieldTypeValidator
|
Hey there! I just built a new temporary npm package based on 7abfbb7. You can download it here or install it by running the following command: npm install https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.73.0-feature-xml-validation.20260709230845.tgz |
2 participants
Validate SceneGraph node usage in
.xmlcomponent files by walking each component's<children>node tree.roSGNodevalidation).Value and field checks are conservative: only int/float/bool/color values are validated (everything else passes), and field-name checks are skipped when no fields resolve for a known node, to avoid false positives. New diagnostics use codes 1155-1158.
Stacked on #1741.