Repository: rokucommunity/brighterscript
Pull request #660 (2022-08-04 15:58:35)

Fix scope-specific diagnostic grouping issue
Fix issue when building multi-scope diagnostics where bsc wasn't taking the actual file path into consideration. So it was incorrectly grouping all diagnostics together by code+message+range without considering file path. 

Also makes the expectDiagnostics function more flexible so it can check for file paths and relatedInformation.


Patch:
diff --git a/src/Scope.spec.ts b/src/Scope.spec.ts
index f7274663..e35524f6 100644
--- a/src/Scope.spec.ts
+++ b/src/Scope.spec.ts
@@ -198,6 +198,54 @@ describe('Scope', () => {
     });
 
     describe('validate', () => {
+        it('diagnostics are assigned to correct child scope', () => {
+            program.options.autoImportComponentScript = true;
+            program.setFile('components/constants.bs', `
+                namespace constants.alpha.beta
+                    const charlie = "charlie"
+                end namespace
+            `);
+
+            program.setFile('components/ButtonBase.xml', `<component name="ButtonBase" extends="Scene" />`);
+
+            const buttonPrimary = program.setFile('components/ButtonPrimary.bs', `
+                import "constants.bs"
+                sub init()
+                    print constants.alpha.delta.charlie
+                end sub
+            `);
+            program.setFile('components/ButtonPrimary.xml', `<component name="ButtonPrimary" extends="ButtonBase" />`);
+
+            const buttonSecondary = program.setFile('components/ButtonSecondary.bs', `
+                import "constants.bs"
+                sub init()
+                    print constants.alpha.delta.charlie
+                end sub
+            `);
+            program.setFile('components/ButtonSecondary.xml', `<component name="ButtonSecondary" extends="ButtonBase" />`);
+
+            program.validate();
+            expectDiagnostics(program, [
+                {
+                    message: DiagnosticMessages.cannotFindName('delta').message,
+                    file: {
+                        srcPath: buttonPrimary.srcPath
+                    },
+                    relatedInformation: [{
+                        message: `Not defined in scope '${s('components/ButtonPrimary.xml')}'`
+                    }]
+                }, {
+                    message: DiagnosticMessages.cannotFindName('delta').message,
+                    file: {
+                        srcPath: buttonSecondary.srcPath
+                    },
+                    relatedInformation: [{
+                        message: `Not defined in scope '${s('components/ButtonSecondary.xml')}'`
+                    }]
+                }
+            ]);
+        });
+
         it('detects unknown namespace names', () => {
             program.setFile('source/main.bs', `
                 sub main()
diff --git a/src/bscPlugin/validation/ScopeValidator.ts b/src/bscPlugin/validation/ScopeValidator.ts
index edd9842a..07c4e153 100644
--- a/src/bscPlugin/validation/ScopeValidator.ts
+++ b/src/bscPlugin/validation/ScopeValidator.ts
@@ -64,7 +64,7 @@ export class ScopeValidator {
      * Add a diagnostic (to the first scope) that will have `relatedInformation` for each affected scope
      */
     private addMultiScopeDiagnostic(event: OnScopeValidateEvent, diagnostic: BsDiagnostic, message = 'Not defined in scope') {
-        diagnostic = this.multiScopeCache.getOrAdd(`${diagnostic.code}-${diagnostic.message}-${util.rangeToString(diagnostic.range)}`, () => {
+        diagnostic = this.multiScopeCache.getOrAdd(`${diagnostic.file?.srcPath}-${diagnostic.code}-${diagnostic.message}-${util.rangeToString(diagnostic.range)}`, () => {
             if (!diagnostic.relatedInformation) {
                 diagnostic.relatedInformation = [];
             }


Test patch:
diff --git a/src/testHelpers.spec.ts b/src/testHelpers.spec.ts
index 5d6523b3..e342de73 100644
--- a/src/testHelpers.spec.ts
+++ b/src/testHelpers.spec.ts
@@ -64,6 +64,7 @@ interface PartialDiagnostic {
     tags?: Partial<DiagnosticTag>[];
     relatedInformation?: Partial<DiagnosticRelatedInformation>[];
     data?: unknown;
+    file?: Partial<BscFile>;
 }
 
 /**
@@ -72,7 +73,7 @@ interface PartialDiagnostic {
  * @param expected an array of expected diagnostics. if it's a string, assume that's a diagnostic error message
  */
 export function expectDiagnostics(arg: DiagnosticCollection, expected: Array<PartialDiagnostic | string | number>) {
-    const diagnostics = sortDiagnostics(
+    const actualDiagnostics = sortDiagnostics(
         getDiagnostics(arg)
     );
     const expectedDiagnostics = sortDiagnostics(
@@ -88,24 +89,32 @@ export function expectDiagnostics(arg: DiagnosticCollection, expected: Array<Par
     );
 
     const actual = [] as BsDiagnostic[];
-    for (let i = 0; i < diagnostics.length; i++) {
+    for (let i = 0; i < actualDiagnostics.length; i++) {
         const expectedDiagnostic = expectedDiagnostics[i];
-        const clone = cloneObject(
-            diagnostics[i],
+        const actualDiagnostic = cloneObject(
+            actualDiagnostics[i],
             expectedDiagnostic,
             ['message', 'code', 'range', 'severity', 'relatedInformation']
         );
         //deep clone relatedInformation if available
-        if (clone.relatedInformation) {
-            for (let j = 0; j < clone.relatedInformation.length; j++) {
-                clone.relatedInformation[j] = cloneObject(
-                    clone.relatedInformation[j],
-                    expectedDiagnostic.relatedInformation[j],
+        if (actualDiagnostic.relatedInformation) {
+            for (let j = 0; j < actualDiagnostic.relatedInformation.length; j++) {
+                actualDiagnostic.relatedInformation[j] = cloneObject(
+                    actualDiagnostic.relatedInformation[j],
+                    expectedDiagnostic?.relatedInformation[j],
                     ['location', 'message']
                 ) as any;
             }
         }
-        actual.push(clone as any);
+        //deep clone file info if available
+        if (actualDiagnostic.file) {
+            actualDiagnostic.file = cloneObject(
+                actualDiagnostic.file,
+                expectedDiagnostic?.file,
+                ['srcPath', 'pkgPath']
+            ) as any;
+        }
+        actual.push(actualDiagnostic as any);
     }
 
     expect(actual).to.eql(expectedDiagnostics);