Skip to content

Commit fe6e705

Browse files
beccamcbeccamc
committed
pr comments
1 parent dde28b6 commit fe6e705

File tree

4 files changed

+73
-59
lines changed

4 files changed

+73
-59
lines changed

‎src/lsptoolshost/buildDiagnosticsService.ts‎

Lines changed: 54 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import * as vscode from 'vscode';
6+
import { commonOptions } from '../shared/options';
67

78
export enum AnalysisSetting {
89
FullSolution = 'fullSolution',
@@ -45,7 +46,7 @@ export class BuildDiagnosticsService {
4546
// Show the build-only diagnostics
4647
displayedBuildDiagnostics.push([
4748
uri,
48-
BuildDiagnosticsService.filerDiagnosticsFromBuild(diagnosticList, buildOnlyIds, isDocumentOpen),
49+
BuildDiagnosticsService.filterDiagnosticsFromBuild(diagnosticList, buildOnlyIds, isDocumentOpen),
4950
]);
5051
});
5152

@@ -63,7 +64,7 @@ export class BuildDiagnosticsService {
6364
// The document is now open in the editor and live diagnostics are being shown. Filter diagnostics
6465
// reported by the build to show build-only problems.
6566
if (currentFileBuildDiagnostics) {
66-
const buildDiagnostics = BuildDiagnosticsService.filerDiagnosticsFromBuild(
67+
const buildDiagnostics = BuildDiagnosticsService.filterDiagnosticsFromBuild(
6768
currentFileBuildDiagnostics[1],
6869
buildOnlyIds,
6970
true
@@ -72,64 +73,69 @@ export class BuildDiagnosticsService {
7273
}
7374
}
7475

75-
public static filerDiagnosticsFromBuild(
76+
public static filterDiagnosticsFromBuild(
7677
diagnosticList: vscode.Diagnostic[],
7778
buildOnlyIds: string[],
7879
isDocumentOpen: boolean
7980
): vscode.Diagnostic[] {
80-
const configuration = vscode.workspace.getConfiguration();
81-
const analyzerDiagnosticScope = configuration.get(
82-
'dotnet.backgroundAnalysis.analyzerDiagnosticsScope'
83-
) as AnalysisSetting;
84-
const compilerDiagnosticScope = configuration.get(
85-
'dotnet.backgroundAnalysis.compilerDiagnosticsScope'
86-
) as AnalysisSetting;
81+
const analyzerDiagnosticScope = commonOptions.analyzerDiagnosticScope as AnalysisSetting;
82+
const compilerDiagnosticScope = commonOptions.compilerDiagnosticScope as AnalysisSetting;
8783

8884
// If compiler and analyzer diagnostics are set to "none", show everything reported by the build
8985
if (analyzerDiagnosticScope === AnalysisSetting.None && compilerDiagnosticScope === AnalysisSetting.None) {
9086
return diagnosticList;
9187
}
9288

9389
// Filter the diagnostics reported by the build. Some may already be shown by live diagnostics.
94-
const buildOnlyDiagnostics: vscode.Diagnostic[] = [];
95-
diagnosticList.forEach((d) => {
96-
if (d.code) {
97-
// If it is a project system diagnostic (e.g. "Target framework out of support")
98-
// then always show it. It cannot be reported by live.
99-
if (this.isProjectSystemDiagnostic(d)) {
100-
buildOnlyDiagnostics.push(d);
101-
}
102-
// If it is a "build-only"diagnostics (i.e. it can only be found by building)
103-
// then always show it. It cannot be reported by live.
104-
else if (buildOnlyIds.find((b_id) => b_id === d.code)) {
105-
buildOnlyDiagnostics.push(d);
106-
} else {
107-
const isAnalyzerDiagnostic = BuildDiagnosticsService.isAnalyzerDiagnostic(d);
108-
const isCompilerDiagnostic = BuildDiagnosticsService.isCompilerDiagnostic(d);
109-
110-
if (
111-
(isAnalyzerDiagnostic && analyzerDiagnosticScope === AnalysisSetting.None) ||
112-
(isCompilerDiagnostic && compilerDiagnosticScope === AnalysisSetting.None)
113-
) {
114-
// If live diagnostics are completely turned off for this type, then show the build diagnostic
115-
buildOnlyDiagnostics.push(d);
116-
} else if (isDocumentOpen) {
117-
// no-op. The document is open and live diagnostis are on. This diagnostic is already being shown.
118-
} else if (
119-
(isAnalyzerDiagnostic && analyzerDiagnosticScope === AnalysisSetting.FullSolution) ||
120-
(isCompilerDiagnostic && compilerDiagnosticScope === AnalysisSetting.FullSolution)
121-
) {
122-
// no-op. Full solution analysis is on for this diagnostic type. All diagnostics are already being shown.
123-
} else {
124-
// The document is closed, and the analysis setting is to only show for open files.
125-
// Show the diagnostic reported by build.
126-
buildOnlyDiagnostics.push(d);
127-
}
128-
}
129-
}
130-
});
90+
const buildDiagnosticsToDisplay: vscode.Diagnostic[] = [];
91+
92+
// If it is a project system diagnostic (e.g. "Target framework out of support")
93+
// then always show it. It cannot be reported by live.
94+
const projectSystemDiagnostics = diagnosticList.filter((d) =>
95+
BuildDiagnosticsService.isProjectSystemDiagnostic(d)
96+
);
97+
buildDiagnosticsToDisplay.push(...projectSystemDiagnostics);
98+
99+
// If it is a "build-only"diagnostics (i.e. it can only be found by building)
100+
// then always show it. It cannot be reported by live.
101+
const buildOnlyDiagnostics = diagnosticList.filter((d) =>
102+
BuildDiagnosticsService.isBuildOnlyDiagnostic(buildOnlyIds, d)
103+
);
104+
buildDiagnosticsToDisplay.push(...buildOnlyDiagnostics);
105+
106+
// Check the analyzer diagnostic setting. If the setting is "none" or if the file is closed,
107+
// then no live analyzers are being shown and bulid analyzers should be added.
108+
// If FSA is on, then this is a no-op as FSA will report all analyzer diagnostics
109+
if (
110+
analyzerDiagnosticScope === AnalysisSetting.None ||
111+
(analyzerDiagnosticScope === AnalysisSetting.OpenFiles && !isDocumentOpen)
112+
) {
113+
const analyzerDiagnostics = diagnosticList.filter(
114+
// Needs to be analyzer diagnostics and not already reported as "build only"
115+
(d) => BuildDiagnosticsService.isAnalyzerDiagnostic(d) && !this.isBuildOnlyDiagnostic(buildOnlyIds, d)
116+
);
117+
buildDiagnosticsToDisplay.push(...analyzerDiagnostics);
118+
}
119+
120+
// Check the compiler diagnostic setting. If the setting is "none" or if the file is closed,
121+
// then no live compiler diagnostics are being shown and bulid compiler diagnostics should be added.
122+
// If FSA is on, then this is a no-op as FSA will report all compiler diagnostics
123+
if (
124+
compilerDiagnosticScope === AnalysisSetting.None ||
125+
(compilerDiagnosticScope === AnalysisSetting.OpenFiles && !isDocumentOpen)
126+
) {
127+
const compilerDiagnostics = diagnosticList.filter(
128+
// Needs to be analyzer diagnostics and not already reported as "build only"
129+
(d) => BuildDiagnosticsService.isCompilerDiagnostic(d) && !this.isBuildOnlyDiagnostic(buildOnlyIds, d)
130+
);
131+
buildDiagnosticsToDisplay.push(...compilerDiagnostics);
132+
}
133+
134+
return buildDiagnosticsToDisplay;
135+
}
131136

132-
return buildOnlyDiagnostics;
137+
private static isBuildOnlyDiagnostic(buildOnlyIds: string[], d: vscode.Diagnostic): boolean {
138+
return buildOnlyIds.find((b_id) => b_id === d.code) !== undefined;
133139
}
134140

135141
private static isCompilerDiagnostic(d: vscode.Diagnostic): boolean {

‎src/lsptoolshost/services/buildResultReporterService.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ export class BuildResultDiagnostics implements IBuildResultDiagnostics {
1818
constructor(private _languageServerPromise: Promise<RoslynLanguageServer>) {}
1919

2020
public async buildStarted(): Promise<void> {
21-
const langServer = await this._languageServerPromise;
21+
const langServer: RoslynLanguageServer = await this._languageServerPromise;
2222
langServer._buildDiagnosticService.clearDiagnostics();
2323
}
2424

2525
public async reportBuildResult(buildDiagnostics: Array<[Uri, Diagnostic[]]>): Promise<void> {
26-
const langServer = await this._languageServerPromise;
27-
const buildOnlyIds = await langServer.getBuildOnlyDiagnosticIds(CancellationToken.None);
26+
const langServer: RoslynLanguageServer = await this._languageServerPromise;
27+
const buildOnlyIds: string[] = await langServer.getBuildOnlyDiagnosticIds(CancellationToken.None);
2828
langServer._buildDiagnosticService.setBuildDiagnostics(buildDiagnostics, buildOnlyIds);
2929
}
3030
}

‎src/shared/options.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export interface CommonOptions {
1818
readonly defaultSolution: string;
1919
readonly unitTestDebuggingOptions: object;
2020
readonly runSettingsPath: string;
21+
readonly analyzerDiagnosticScope: string;
22+
readonly compilerDiagnosticScope: string;
2123
}
2224

2325
export interface OmnisharpServerOptions {
@@ -153,6 +155,12 @@ class CommonOptionsImpl implements CommonOptions {
153155
public get runSettingsPath() {
154156
return readOption<string>('dotnet.unitTests.runSettingsPath', '', 'omnisharp.testRunSettings');
155157
}
158+
public get analyzerDiagnosticScope() {
159+
return readOption<string>('dotnet.backgroundAnalysis.analyzerDiagnosticsScope', 'openFiles');
160+
}
161+
public get compilerDiagnosticScope() {
162+
return readOption<string>('dotnet.backgroundAnalysis.compilerDiagnosticsScope', 'openFiles');
163+
}
156164
}
157165

158166
class OmnisharpOptionsImpl implements OmnisharpServerOptions {

‎test/integrationTests/buildDiagnostics.integration.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ describe(`Build and live diagnostics dedupe ${testAssetWorkspace.description}`,
2828
const buildOnlyIds = ['CS1001'];
2929
const diagnostics = [getTestDiagnostic('CS1001'), getTestDiagnostic('CS1002')];
3030

31-
const displayedBuildResultsClosedFile = BuildDiagnosticsService.filerDiagnosticsFromBuild(
31+
const displayedBuildResultsClosedFile = BuildDiagnosticsService.filterDiagnosticsFromBuild(
3232
diagnostics,
3333
buildOnlyIds,
3434
false
3535
);
3636

37-
const displayedBuildResultsOpenFile = BuildDiagnosticsService.filerDiagnosticsFromBuild(
37+
const displayedBuildResultsOpenFile = BuildDiagnosticsService.filterDiagnosticsFromBuild(
3838
diagnostics,
3939
buildOnlyIds,
4040
true
@@ -50,13 +50,13 @@ describe(`Build and live diagnostics dedupe ${testAssetWorkspace.description}`,
5050
const buildOnlyIds = ['CS1001'];
5151
const diagnostics = [getTestDiagnostic('CS1001'), getTestDiagnostic('CS2001'), getTestDiagnostic('SA3001')];
5252

53-
const displayedBuildResultsOpenFile = BuildDiagnosticsService.filerDiagnosticsFromBuild(
53+
const displayedBuildResultsOpenFile = BuildDiagnosticsService.filterDiagnosticsFromBuild(
5454
diagnostics,
5555
buildOnlyIds,
5656
true
5757
);
5858

59-
const displayedBuildResultsClosedFile = BuildDiagnosticsService.filerDiagnosticsFromBuild(
59+
const displayedBuildResultsClosedFile = BuildDiagnosticsService.filterDiagnosticsFromBuild(
6060
diagnostics,
6161
buildOnlyIds,
6262
false
@@ -76,7 +76,7 @@ describe(`Build and live diagnostics dedupe ${testAssetWorkspace.description}`,
7676
const buildOnlyIds = ['CS1001'];
7777
const diagnostics = [getTestDiagnostic('CS1001'), getTestDiagnostic('CS2001'), getTestDiagnostic('SA3001')];
7878

79-
const displayedBuildResults = BuildDiagnosticsService.filerDiagnosticsFromBuild(
79+
const displayedBuildResults = BuildDiagnosticsService.filterDiagnosticsFromBuild(
8080
diagnostics,
8181
buildOnlyIds,
8282
false
@@ -93,7 +93,7 @@ describe(`Build and live diagnostics dedupe ${testAssetWorkspace.description}`,
9393
const buildOnlyIds = ['CS1001'];
9494
const diagnostics = [getTestDiagnostic('CS1001'), getTestDiagnostic('CS2001'), getTestDiagnostic('SA3001')];
9595

96-
const displayedBuildResults = BuildDiagnosticsService.filerDiagnosticsFromBuild(
96+
const displayedBuildResults = BuildDiagnosticsService.filterDiagnosticsFromBuild(
9797
diagnostics,
9898
buildOnlyIds,
9999
false
@@ -110,7 +110,7 @@ describe(`Build and live diagnostics dedupe ${testAssetWorkspace.description}`,
110110
const buildOnlyIds = ['CS1001'];
111111
const diagnostics = [getTestDiagnostic('CS1001'), getTestDiagnostic('CS2001'), getTestDiagnostic('SA3001')];
112112

113-
const displayedBuildResults = BuildDiagnosticsService.filerDiagnosticsFromBuild(
113+
const displayedBuildResults = BuildDiagnosticsService.filterDiagnosticsFromBuild(
114114
diagnostics,
115115
buildOnlyIds,
116116
false
@@ -129,7 +129,7 @@ describe(`Build and live diagnostics dedupe ${testAssetWorkspace.description}`,
129129
const buildOnlyIds = ['CS1001'];
130130
const diagnostics = [getTestDiagnostic('NETSDK3001')];
131131

132-
const displayedBuildResults = BuildDiagnosticsService.filerDiagnosticsFromBuild(
132+
const displayedBuildResults = BuildDiagnosticsService.filterDiagnosticsFromBuild(
133133
diagnostics,
134134
buildOnlyIds,
135135
true

0 commit comments

Comments
 (0)