Merge pull request #2 from rokucommunity/Return-proper-range-in-XML-w…

…arning Improve range information for the relatedInformation so we take them to the field with that ID.
rokucommunity · Aug 19, 2024 · cbe67db · cbe67db
2 parents c74522b + d4d1dc2
commit cbe67db
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 40 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -13,7 +13,8 @@ module.exports = {
     plugins: [
         '@typescript-eslint',
         'no-only-tests',
-        'jsdoc'
+        'jsdoc',
+        'import'
     ],
     extends: [
         'eslint:all',
@@ -133,6 +134,8 @@ module.exports = {
         'max-lines-per-function': 'off',
         'max-params': 'off',
         'max-statements': 'off',
+        'import/no-duplicates': ['error'],
+        'no-duplicate-imports': 'off',
         'no-only-tests/no-only-tests': 'error',
         'multiline-comment-style': 'off',
         'multiline-ternary': 'off',

diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -2,6 +2,7 @@
     "editor.tabSize": 4,
     "editor.insertSpaces": true,
     "editor.detectIndentation": false,
+    "editor.formatOnSave": true,
     "cSpell.words": [
         "brighterscript",
         "findnode",

diff --git a/package-lock.json b/package-lock.json
diff --git a/src/Plugin.ts b/src/Plugin.ts
@@ -20,4 +20,4 @@ export class Plugin implements CompilerPlugin {
             program.removeFile(file.pkgPath);
         }
     }
-}
+}
diff --git a/src/findNodes.spec.ts b/src/findNodes.spec.ts
@@ -6,9 +6,10 @@ import undent from 'undent';
 
 describe('findnode', () => {
     let program: Program;
+    const rootDir = path.join(__dirname, '../.tmp');
 
     beforeEach(() => {
-        program = new Program({});
+        program = new Program({ rootDir: rootDir });
         program.plugins.add(new Plugin());
     });
 
@@ -167,13 +168,27 @@ describe('findnode', () => {
 
         program.validate();
         expect(
-            program.getDiagnostics().map(x => ({ message: x.message, range: x.range }))
+            program.getDiagnostics().map(x => ({ message: x.message, range: x.range, relatedInformation: x.relatedInformation }))
         ).to.eql([{
             message: `Unnecessary call to 'm.top.findNode("helloZombieText")'`,
-            range: util.createRange(2, 36, 2, 69)
+            range: util.createRange(2, 36, 2, 69),
+            relatedInformation: [{
+                message: `In scope 'components${path.sep}ZombieKeyboard.xml'`,
+                location: util.createLocation(
+                    util.pathToUri(`${rootDir}/components/ZombieKeyboard.xml`),
+                    util.createRange(4, 31, 4, 46)
+                )
+            }]
         }, {
             message: `Unnecessary call to 'm.top.findNode("helloZombieText")'`,
-            range: util.createRange(3, 37, 3, 70)
+            range: util.createRange(3, 37, 3, 70),
+            relatedInformation: [{
+                message: `In scope 'components${path.sep}ZombieKeyboard.xml'`,
+                location: util.createLocation(
+                    util.pathToUri(`${rootDir}/components/ZombieKeyboard.xml`),
+                    util.createRange(4, 31, 4, 46)
+                )
+            }]
         }]);
     });
 

diff --git a/src/findNodes.ts b/src/findNodes.ts
@@ -1,17 +1,16 @@
-import type { AstEditor, FunctionStatement, BrsFile, Program, TranspileObj, BscFile, Statement } from 'brighterscript';
-// eslint-disable-next-line no-duplicate-imports
+import type { AstEditor, FunctionStatement, BrsFile, Program, TranspileObj, BscFile, Statement, Range } from 'brighterscript';
 import { isBrsFile, Parser, isXmlScope, DiagnosticSeverity, createVisitor, WalkMode, isDottedGetExpression, isVariableExpression, isLiteralString, util } from 'brighterscript';
 import type { SGNode } from 'brighterscript/dist/parser/SGTypes';
 
-function findChildrenWithIDs(children: Array<SGNode>): string[] {
-    let foundIDs: string[] = [];
+function findChildrenWithIDs(children: Array<SGNode>): Map<string, Range> {
+    let foundIDs = new Map<string, Range>();
     if (children) {
         children.forEach(child => {
             if (child.id) {
-                foundIDs.push(child.id);
+                foundIDs.set(child.id, child.attributes.find(x => x.key.text === 'id')?.value?.range ?? util.createRange(0, 0, 0, 100));
             }
             const subChildren = findChildrenWithIDs(child.children);
-            foundIDs = foundIDs.concat(subChildren);
+            foundIDs = new Map([...foundIDs, ...subChildren]);
         });
     }
     return foundIDs;
@@ -22,7 +21,7 @@ export function findNodeWithIDInjection(program: Program, entries: TranspileObj[
         if (isXmlScope(scope)) {
             const xmlFile = scope.xmlFile;
             const ids = findChildrenWithIDs(xmlFile.parser.ast.component?.children?.children ?? []);
-            if (ids.length > 0) {
+            if (ids.size > 0) {
                 const scopeFiles: BscFile[] = scope.getOwnFiles();
 
                 //find an init function from all the scope's files
@@ -55,12 +54,12 @@ export function findNodeWithIDInjection(program: Program, entries: TranspileObj[
                 if (initFunction) {
                     //add m variables for every xml component that has an id
                     // eslint-disable-next-line max-statements-per-line, @typescript-eslint/brace-style
-                    const assignments = ids.map(id => { return `m.${id} = m.top.findNode("${id}")`; }).join('\n');
+                    const assignments = Array.from(ids).map(([id, range]) => { return `m.${id} = m.top.findNode("${id}")`; }).join('\n');
                     const statements = (Parser.parse(`
-					sub temp()
-						${assignments}
-					end sub
-				`).statements[0] as FunctionStatement).func.body.statements;
+                        sub temp()
+                            ${assignments}
+                        end sub
+                    `).statements[0] as FunctionStatement).func.body.statements;
                     //add the assignments to the top of the init function
                     editor.arrayUnshift(initFunction.func.body.statements, ...statements);
                 }
@@ -74,11 +73,11 @@ export function validateNodeWithIDInjection(program: Program) {
         if (isXmlScope(scope)) {
             const xmlFile = scope.xmlFile;
             const ids = findChildrenWithIDs(xmlFile.parser.ast.component?.children?.children ?? []);
-            if (ids.length > 0) {
+            if (ids.size > 0) {
                 const scopeFiles: BscFile[] = scope.getOwnFiles();
 
                 let initFunction: FunctionStatement | undefined;
-                let initFunctionFile: BrsFile | undefined;
+                let initFunctionFile: BscFile | undefined;
 
                 for (const file of scopeFiles) {
                     if (isBrsFile(file)) {
@@ -103,7 +102,8 @@ export function validateNodeWithIDInjection(program: Program) {
                                 isLiteralString(expression.args[0])
                             ) {
                                 let id = expression.args[0].token.text.replace(/^"/, '').replace(/"$/, '');
-                                if (id && ids.includes(id)) {
+                                let warningRange = ids.get(id);
+                                if (warningRange !== undefined) {
                                     initFunctionFile!.diagnostics.push({
                                         file: initFunctionFile!,
                                         range: expression.range,
@@ -113,7 +113,7 @@ export function validateNodeWithIDInjection(program: Program) {
                                             message: `In scope '${scope.name}'`,
                                             location: util.createLocation(
                                                 util.pathToUri(xmlFile.srcPath),
-                                                util.createRange(0, 0, 0, 100)
+                                                warningRange
                                             )
                                         }]
                                     });
-Original file line number
+Diff line change
@@ Expand Up / @@ -20,4 +20,4 @@ export class Plugin implements CompilerPlugin { @@
                 program.removeFile(file.pkgPath);
             }
         }
-    }
+    }