Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,11 @@ export let DiagnosticMessages = {
message: `Mismatched closing tag: expected '</${openingTag}>' but found '</${closingTag}>'`,
code: 1156,
severity: DiagnosticSeverity.Error
}),
xmlTagWrongCase: (actualTag: string, expectedTag: string) => ({
message: `Tag '${actualTag}' must be all lower case. Use '${expectedTag}' instead`,
code: 1157,
severity: DiagnosticSeverity.Error
})
};

Expand Down
37 changes: 37 additions & 0 deletions src/bscPlugin/validation/XmlFileValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { XmlFile } from '../../files/XmlFile';
import type { OnFileValidateEvent } from '../../interfaces';
import type { SGAst, SGTag } from '../../parser/SGTypes';
import { isSGInterface } from '../../astUtils/xml';
import util from '../../util';

export class XmlFileValidator {
Expand All @@ -15,6 +16,7 @@ export class XmlFileValidator {
if (this.event.file.parser.ast.root) {
this.validateComponent(this.event.file.parser.ast);
this.validateTagClosings(this.event.file.parser.ast.root);
this.validateTagCasing(this.event.file.parser.ast.root);
} else {
//skip empty XML
}
Expand Down Expand Up @@ -42,6 +44,41 @@ export class XmlFileValidator {
}
}

/**
* Report any structural tag that isn't all lower case (e.g. `<Children>` instead of
* `<children>`). Roku requires these tags to be lower case, but the parser matches them
* case-insensitively so we can emit this specific diagnostic instead of a generic
* "unexpected tag" error.
*
* Only the structural spine is walked (the component tag, its direct children, and the
* members of `<interface>`). Tags inside `<children>` are node/component names (like
* `<Label>`) whose casing is author-defined, so they're intentionally not validated.
*/
private validateTagCasing(root: SGTag) {
const validate = (tag: SGTag) => {
const tagText = tag.tag.text;
if (tagText !== tagText.toLowerCase()) {
this.event.file.diagnostics.push({
...DiagnosticMessages.xmlTagWrongCase(tagText, tagText.toLowerCase()),
range: tag.tag.range,
file: this.event.file
});
}
};

validate(root);
//`<children>` holds author-cased node names, so validate the tag itself but not its contents
for (const child of root.getChildren()) {
validate(child);
if (isSGInterface(child)) {
//`<field>` and `<function>` members are structural too
for (const member of child.getChildren()) {
validate(member);
}
}
}
}

private validateComponent(ast: SGAst) {
const { root, component } = ast;
if (!component) {
Expand Down
140 changes: 140 additions & 0 deletions src/files/XmlFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,145 @@ describe('XmlFile', () => {
]);
});

describe('xml tag casing', () => {
it('emits a casing diagnostic for <Children>', () => {
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<Children>
<Label id="myLabel" />
</Children>
</component>
`);
program.validate();
expectDiagnostics(file, [
DiagnosticMessages.xmlTagWrongCase('Children', 'children')
]);
});

it('emits a casing diagnostic for <Interface>', () => {
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<Interface>
<field id="foo" type="string" />
</Interface>
</component>
`);
program.validate();
expectDiagnostics(file, [
DiagnosticMessages.xmlTagWrongCase('Interface', 'interface')
]);
});

it('emits a casing diagnostic for <Script>', () => {
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<Script type="text/brightscript" uri="pkg:/source/main.brs" />
</component>
`);
program.setFile('source/main.brs', '');
program.validate();
expectDiagnostics(file, [
DiagnosticMessages.xmlTagWrongCase('Script', 'script')
]);
});

it('emits a casing diagnostic for <Component>', () => {
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<Component name="Comp" extends="Group">
</Component>
`);
program.validate();
expectDiagnostics(file, [
DiagnosticMessages.xmlTagWrongCase('Component', 'component')
]);
});

it('emits casing diagnostics for <Field> and <Function>', () => {
program.setFile('source/main.brs', `sub doThing()
end sub`);
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/main.brs" />
<interface>
<Field id="foo" type="string" />
<Function name="doThing" />
</interface>
</component>
`);
program.validate();
expectDiagnostics(file, [
DiagnosticMessages.xmlTagWrongCase('Field', 'field'),
DiagnosticMessages.xmlTagWrongCase('Function', 'function')
]);
});

it('does not emit a casing diagnostic for node tags inside <children>', () => {
//node names inside <children> are author-defined component names, so their
//casing must be left alone
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<children>
<Group id="outer">
<Label id="inner" text="hello" />
</Group>
</children>
</component>
`);
program.validate();
expectZeroDiagnostics(file);
});

it('points the diagnostic range at the opening tag name', () => {
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<Children>
</Children>
</component>
`);
program.validate();
expect(file.diagnostics).to.have.lengthOf(1);
//the squiggle lands on `Children` (line 2)
expect(file.diagnostics[0].range).to.eql(
Range.create(2, 5, 2, 13)
);
});

it('preserves the original casing when transpiling', () => {
//we report the problem but must not silently rewrite the author's markup
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<Children>
<Label id="myLabel" />
</Children>
</component>
`);
program.validate();
expect(file.transpile().code).to.include('<Children>');
expect(file.transpile().code).to.include('</Children>');
});

it('still emits the generic unexpected-tag diagnostic for unknown tags', () => {
const file = program.setFile<XmlFile>('components/Comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<bogus />
</component>
`);
program.validate();
expectDiagnostics(file, [
DiagnosticMessages.xmlUnexpectedTag('bogus')
]);
});
});

describe('transpile', () => {
it('handles single quotes properly', () => {
testTranspile(trim`
Expand Down Expand Up @@ -1038,6 +1177,7 @@ describe('XmlFile', () => {
expectZeroDiagnostics(file);
});


it('does not include additional bslib script if already there ', () => {
testTranspile(trim`
<?xml version="1.0" encoding="utf-8" ?>
Expand Down
8 changes: 6 additions & 2 deletions src/parser/SGParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ function mapElement({ children }: ElementCstNode, diagnostics: Diagnostic[]): SG

const attributes = mapAttributes(children.attribute);
const content = children.content?.[0];
switch (name.text) {
//match structural tags case-insensitively so wrong-cased tags (like `<Children>`) still
//produce a proper AST node. `XmlFileValidator` reports the casing problem during validation.
switch (name.text.toLowerCase()) {
case 'component':
const componentContent = mapElements(content, ['interface', 'script', 'children', 'customization'], diagnostics);
return new SGComponent(name, attributes, componentContent, range, closingName);
Expand Down Expand Up @@ -269,7 +271,9 @@ function mapElements(content: ContentCstNode, allow: string[], diagnostics: Diag
for (const entry of element) {
const name = entry.children.Name?.[0];
if (name?.image) {
if (allow.includes(name.image)) {
//compare case-insensitively so wrong-cased tags (like `<Children>`) are still
//mapped into the AST. `XmlFileValidator` reports the casing problem separately.
if (allow.some(x => x === name.image.toLowerCase())) {
tags.push(mapElement(entry, diagnostics));
} else {
//unexpected tag
Expand Down