Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -354,6 +354,11 @@ export let DiagnosticMessages = {
code: 1065,
severity: DiagnosticSeverity.Error
}),
xmlTagCaseMismatch: (tagName: string, expectedTagName: string) => ({
message: `Tag '${tagName}' must be all lower case. Use '${expectedTagName}' instead.`,
code: 1143,
severity: DiagnosticSeverity.Error
}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to the bottom of the list so its code is in the correct order

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved xmlTagCaseMismatch diagnostic to the bottom of the list so its code (1143) is in the correct numerical order after code 1142.

expectedStatementOrFunctionCallButReceivedExpression: () => ({
message: `Expected statement or function call but instead found expression`,
code: 1066,
Expand Down
52 changes: 52 additions & 0 deletions src/parser/SGParser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,58 @@ describe('SGParser', () => {
});
});

it('Adds error when incorrect casing is used for children tag', () => {
const parser = new SGParser();
parser.parse(
'pkg:/components/ParentScene.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="ChildScene" extends="ParentScene">
<Children>
<Label id="test" />
</Children>
</component>
`);
expect(parser.diagnostics).to.be.lengthOf(1);
expect(parser.diagnostics[0]).to.deep.include({
...DiagnosticMessages.xmlTagCaseMismatch('Children', 'children'),
range: Range.create(2, 5, 2, 13)
});
});

it('Adds error when incorrect casing is used for interface tag', () => {
const parser = new SGParser();
parser.parse(
'pkg:/components/ParentScene.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="ChildScene" extends="ParentScene">
<Interface>
<field id="test" type="string" />
</Interface>
</component>
`);
expect(parser.diagnostics).to.be.lengthOf(1);
expect(parser.diagnostics[0]).to.deep.include({
...DiagnosticMessages.xmlTagCaseMismatch('Interface', 'interface'),
range: Range.create(2, 5, 2, 14)
});
});

it('Adds error when incorrect casing is used for script tag', () => {
const parser = new SGParser();
parser.parse(
'pkg:/components/ParentScene.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="ChildScene" extends="ParentScene">
<Script type="text/brightscript" uri="./test.brs" />
</component>
`);
expect(parser.diagnostics).to.be.lengthOf(1);
expect(parser.diagnostics[0]).to.deep.include({
...DiagnosticMessages.xmlTagCaseMismatch('Script', 'script'),
range: Range.create(2, 5, 2, 11)
});
});

it('Adds error when a leaf tag is found to have children', () => {
const parser = new SGParser();
parser.parse(
Expand Down
22 changes: 17 additions & 5 deletions src/parser/SGParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,23 @@ function mapElements(content: ContentCstNode, allow: string[], diagnostics: Diag
if (allow.includes(name.image)) {
tags.push(mapElement(entry, diagnostics));
} else {
//unexpected tag
diagnostics.push({
...DiagnosticMessages.xmlUnexpectedTag(name.image),
range: rangeFromTokens(name)
});
// Check if this is a case mismatch for a known tag
const lowerCaseTag = name.image.toLowerCase();
const matchingAllowedTag = allow.find(allowedTag => allowedTag.toLowerCase() === lowerCaseTag);

if (matchingAllowedTag && matchingAllowedTag !== name.image) {
// Case mismatch for a known tag
diagnostics.push({
...DiagnosticMessages.xmlTagCaseMismatch(name.image, matchingAllowedTag),
range: rangeFromTokens(name)
});
} else {
// Truly unexpected tag
diagnostics.push({
...DiagnosticMessages.xmlUnexpectedTag(name.image),
range: rangeFromTokens(name)
});
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this into XmlFileValidator so that way it catches these casing issues even after parsing, (like when plugins change or inject AST).

Add a unit test where a plugin changes the casing of one of these elements and then it gets caught during validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the XML tag casing validation to XmlFileValidator as requested. The validation now catches casing issues even when plugins modify the AST after parsing, and includes the requested unit test demonstrating this behavior (commit 016d2db).

} else {
//bad xml syntax...
Expand Down