Fix GitHub 1401, diagnostics relative to workspace folder (#3780)

* Fix GitHub 1401, diagnostics relative to workspace folder - relative diagnostics files are searched in build_dir and src_dir, first existing file is returned - defaults to old behavior : `path.resolve(build_dir, file)` - `resolveDiagnostics` has been made async to not slow down IDE when testing files * update ChangeLog for #1401 * fix relative file resolution diagnostics test - diagnostics paths always use POSIX separators (see `normalizePath` in [src/util.ts](src/util.ts)) --------- Co-authored-by: Sylvain Fargier <sylvain.fargier@cern.ch> Co-authored-by: Garrett Campbell <86264750+gcampbell-msft@users.noreply.github.com>
microsoft · Jul 29, 2024 · 97b36c9 · 97b36c9
1 parent 998ebbd
commit 97b36c9
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ Improvements:
 
 - Add `Unspecified` option for selecting a kit variant to allow CMake itself to select the build type. [#3821](https://github.com/microsoft/vscode-cmake-tools/issues/3821)
 - Skip loading variants when using CMakePresets. [#3300](https://github.com/microsoft/vscode-cmake-tools/issues/3300)
+- Resolve diagnostics files relative to workspace and build directory, fixes [#1401](https://github.com/microsoft/vscode-cmake-tools/issues/1401) [@fargies](https://github.com/fargies)
 - Add setting to only show the cmake log on target failure. [#3785](https://github.com/microsoft/vscode-cmake-tools/pull/3785) [@stepeos](https://github.com/stepeos)
 - Preset expansion occurs on `CMakePresets.json` or `CMakeUserPresets.json` save, and if there are no errors the expanded presets are cached. The VS Developer Environment will only be applied to a preset if it is selected. Expansion errors will show in the problems panel and preset files with errors will be invalid, and any presets they contain cannot be used. [#3905](https://github.com/microsoft/vscode-cmake-tools/pull/3905)
 

diff --git a/src/cmakeProject.ts b/src/cmakeProject.ts
@@ -2041,7 +2041,7 @@ export class CMakeProject {
                         } else {
                             buildLogger.info(localize('build.finished.with.code', 'Build finished with exit code {0}', rc));
                         }
-                        const fileDiags: FileDiagnostic[] | undefined = drv!.config.parseBuildDiagnostics ? consumer!.compileConsumer.resolveDiagnostics(drv!.binaryDir) : [];
+                        const fileDiags: FileDiagnostic[] | undefined = drv!.config.parseBuildDiagnostics ? await consumer!.compileConsumer.resolveDiagnostics(drv!.binaryDir, drv!.sourceDir) : [];
                         if (fileDiags) {
                             populateCollection(collections.build, fileDiags);
                         }

diff --git a/src/diagnostics/build.ts b/src/diagnostics/build.ts
@@ -46,7 +46,17 @@ export class CompileOutputConsumer implements OutputConsumer {
         }
     }
 
-    resolveDiagnostics(basePath: string): FileDiagnostic[] {
+    async resolvePath(file: string, basePaths: string[]): Promise<string> {
+        for (const basePath of basePaths) {
+            const resolved = util.resolvePath(file, basePath);
+            if (await util.checkFileExists(resolved)) {
+                return resolved;
+            }
+        }
+        return util.resolvePath(file, basePaths[0] ?? '');
+    }
+
+    async resolveDiagnostics(...basePaths: string[]): Promise<FileDiagnostic[]> {
         const diags_by_file = new Map<string, vscode.Diagnostic[]>();
 
         const severity_of = (p: string) => {
@@ -75,13 +85,15 @@ export class CompileOutputConsumer implements OutputConsumer {
             link: this.compilers.gnuLD.diagnostics,
             IAR: this.compilers.iar.diagnostics
         };
-        const arrs = util.objectPairs(by_source)
-            .filter(([source, _]) => this.config.enableOutputParsers?.includes(source.toLowerCase()) ?? false)
-            .map(([source, diags]) => diags.map(raw_diag => {
-                const filepath = util.resolvePath(raw_diag.file, basePath);
+        const parsers = util.objectPairs(by_source)
+            .filter(([source, _]) => this.config.enableOutputParsers?.includes(source.toLowerCase()) ?? false);
+        const arrs: FileDiagnostic[] = [];
+        for (const [ source, diags ] of parsers) {
+            for (const raw_diag of diags) {
+                const filepath = await this.resolvePath(raw_diag.file, basePaths);
                 const severity = severity_of(raw_diag.severity);
                 if (severity === undefined) {
-                    return undefined;
+                    continue;
                 }
                 const diag = new vscode.Diagnostic(raw_diag.location, raw_diag.message, severity);
                 diag.source = source;
@@ -93,17 +105,18 @@ export class CompileOutputConsumer implements OutputConsumer {
                 }
                 diag.relatedInformation = [];
                 for (const rel of raw_diag.related) {
-                    const relFilePath = vscode.Uri.file(util.resolvePath(rel.file, basePath));
+                    const relFilePath = vscode.Uri.file(await this.resolvePath(rel.file, basePaths));
                     const related = new vscode.DiagnosticRelatedInformation(new vscode.Location(relFilePath, rel.location), rel.message);
                     diag.relatedInformation.push(related);
                 }
                 diags_by_file.get(filepath)!.push(diag);
-                return {
+                arrs.push({
                     filepath,
                     diag
-                };
-            }).filter(e => e !== undefined) as FileDiagnostic[]);
-        return ([] as FileDiagnostic[]).concat(...arrs);
+                });
+            }
+        }
+        return arrs;
     }
 }
 

diff --git a/test/unit-tests/diagnostics.test.ts b/test/unit-tests/diagnostics.test.ts
@@ -10,9 +10,10 @@ import { expect } from 'chai';
 import * as diags from '@cmt/diagnostics/build';
 import { OutputConsumer } from '../../src/proc';
 import { ExtensionConfigurationSettings, ConfigurationReader } from '../../src/config';
-import { platformPathEquivalent } from '@cmt/util';
+import { platformPathEquivalent, resolvePath } from '@cmt/util';
 import { CMakeOutputConsumer } from '@cmt/diagnostics/cmake';
 import { populateCollection } from '@cmt/diagnostics/util';
+import { getTestResourceFilePath } from '@test/util';
 
 function feedLines(consumer: OutputConsumer, output: string[], error: string[]) {
     for (const line of output) {
@@ -259,11 +260,11 @@ suite('Diagnostics', () => {
         expect(path.posix.normalize(diag.file)).to.eq(diag.file);
         expect(path.posix.isAbsolute(diag.file)).to.be.true;
     });
-    test('Parsing non-diagnostic', () => {
+    test('Parsing non-diagnostic', async () => {
         const lines = ['/usr/include/c++/10/bits/stl_vector.h:98:47: optimized: basic block part vectorized using 32 byte vectors'];
         feedLines(build_consumer, [], lines);
         expect(build_consumer.compilers.gcc.diagnostics).to.have.length(1);
-        const resolved = build_consumer.resolveDiagnostics('dummyPath');
+        const resolved = await build_consumer.resolveDiagnostics('dummyPath');
         expect(resolved.length).to.eq(0);
     });
     test('Parsing linker error', () => {
@@ -480,4 +481,25 @@ suite('Diagnostics', () => {
         expect(diagnostic.message).to.eq('cannot open source file "kjlkjl"\nsearched: "C:\\Program Files (x86)\\IAR Systems\\Embedded Workbench\n8.0\\arm\\inc\\"\ncurrent directory: "C:\\Users\\user\\Documents"');
         expect(diagnostic.severity).to.eq('error');
     });
+
+    test('Relative file resolution', async () => {
+        const project_dir = getTestResourceFilePath('driver/workspace/test_project');
+        build_consumer.config.updatePartial({ enabledOutputParsers: [ 'gcc' ] });
+
+        const lines = ['main.cpp:42:42: error: test warning'];
+        feedLines(build_consumer, [], lines);
+        expect(build_consumer.compilers.gcc.diagnostics).to.have.length(1);
+
+        /* default behavior resolve in build-dir */
+        let resolved = await build_consumer.resolveDiagnostics('dummyPath');
+        expect(resolved.length).to.eq(1);
+        let diagnostic = resolved[0];
+        expect(diagnostic.filepath).to.eq('dummyPath/main.cpp');
+
+        /* resolve first path where file exists (fallback on first argument) */
+        resolved = await build_consumer.resolveDiagnostics('dummyPath', path.resolve(project_dir, 'build'), project_dir, 'dummyPath2');
+        expect(resolved.length).to.eq(1);
+        diagnostic = resolved[0];
+        expect(diagnostic.filepath).to.eq(resolvePath('main.cpp', project_dir));
+    });
 });