From a43ea18b907d1df9a6b964abe40cf0eb6e107303 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:39:22 -0700 Subject: [PATCH 01/19] Satisfy TSLint --- src/features/diagnosticsProvider.ts | 2 +- src/features/fileOpenCloseProvider.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/features/diagnosticsProvider.ts b/src/features/diagnosticsProvider.ts index 4a903ef39..c45b6c4ad 100644 --- a/src/features/diagnosticsProvider.ts +++ b/src/features/diagnosticsProvider.ts @@ -137,7 +137,7 @@ class DiagnosticsProvider extends AbstractSupport { this._subscriptions.push(this._validateCurrentDocumentPipe .pipe(debounceTime(750)) - .subscribe(x => this._validateDocument(x))); + .subscribe(async document => await this._validateDocument(document))); this._subscriptions.push(this._validateAllPipe .pipe(debounceTime(3000)) diff --git a/src/features/fileOpenCloseProvider.ts b/src/features/fileOpenCloseProvider.ts index e1e8e7787..fe7cad785 100644 --- a/src/features/fileOpenCloseProvider.ts +++ b/src/features/fileOpenCloseProvider.ts @@ -1,3 +1,8 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + import { IDisposable } from "../Disposable"; import { OmniSharpServer } from "../omnisharp/server"; import * as vscode from 'vscode'; From 80406031bc01ed300f514c677ba976040be57403 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:41:41 -0700 Subject: [PATCH 02/19] Mark isNetCoreProject as synchronous Needed for a telemetry summation in extension.ts to function properly. --- src/omnisharp/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/omnisharp/utils.ts b/src/omnisharp/utils.ts index cac8b3dc8..bc3e4b1be 100644 --- a/src/omnisharp/utils.ts +++ b/src/omnisharp/utils.ts @@ -205,7 +205,7 @@ export async function fileClose(server: OmniSharpServer, request: protocol.Reque return server.makeRequest(protocol.Requests.FileClose, request); } -export async function isNetCoreProject(project: protocol.MSBuildProject) { +export function isNetCoreProject(project: protocol.MSBuildProject) { return project.TargetFrameworks.find(tf => tf.ShortName.startsWith('netcoreapp') || tf.ShortName.startsWith('netstandard')) !== undefined; } From eb8e2a8fbf9cf9d534bb16f037564a4d7fe3e6b3 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:42:21 -0700 Subject: [PATCH 03/19] Remove Range support from createRequest No consumer uses Range. --- src/omnisharp/typeConversion.ts | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/omnisharp/typeConversion.ts b/src/omnisharp/typeConversion.ts index f6b183c94..d299f5caf 100644 --- a/src/omnisharp/typeConversion.ts +++ b/src/omnisharp/typeConversion.ts @@ -62,29 +62,18 @@ export function toVSCodePosition(point: protocol.V2.Point): vscode.Position { return new vscode.Position(point.Line, point.Column); } -export function createRequest(document: vscode.TextDocument, where: vscode.Position | vscode.Range, includeBuffer: boolean = false): T { - - let Line: number, Column: number; - - if (where instanceof vscode.Position) { - Line = where.line; - Column = where.character; - } else if (where instanceof vscode.Range) { - Line = where.start.line; - Column = where.start.character; - } - +export function createRequest(document: vscode.TextDocument, where: vscode.Position, includeBuffer: boolean = false): T { // for metadata sources, we need to remove the [metadata] from the filename, and prepend the $metadata$ authority // this is expected by the Omnisharp server to support metadata-to-metadata navigation - let fileName = document.uri.scheme === "omnisharp-metadata" ? + const fileName = document.uri.scheme === "omnisharp-metadata" ? `${document.uri.authority}${document.fileName.replace("[metadata] ", "")}` : document.fileName; - let request: protocol.Request = { + const request: protocol.Request = { FileName: fileName, Buffer: includeBuffer ? document.getText() : undefined, - Line, - Column + Line: where.line, + Column: where.character, }; return request; From b4b003385a1886d88d90e4c1e6863fa1224d084b Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:42:55 -0700 Subject: [PATCH 04/19] Nullability fixes for requestQueue --- src/omnisharp/requestQueue.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/omnisharp/requestQueue.ts b/src/omnisharp/requestQueue.ts index 486be14f7..7a9632959 100644 --- a/src/omnisharp/requestQueue.ts +++ b/src/omnisharp/requestQueue.ts @@ -92,7 +92,7 @@ class RequestQueue { this.eventStream.post(new OmnisharpServerProcessRequestStart(this._name, availableRequestSlots)); for (let i = 0; i < availableRequestSlots && this._pending.length > 0; i++) { - const item = this._pending.shift(); + const item = this._pending.shift()!; // We know from this._pending.length that we can still shift item.startTime = Date.now(); const id = this._makeRequest(item); @@ -117,6 +117,7 @@ export class RequestQueueCollection { concurrency: number, makeRequest: (request: Request) => number ) { + this._isProcessing = false; this._priorityQueue = new RequestQueue('Priority', 1, eventStream, makeRequest); this._normalQueue = new RequestQueue('Normal', concurrency, eventStream, makeRequest); this._deferredQueue = new RequestQueue('Deferred', Math.max(Math.floor(concurrency / 4), 2), eventStream, makeRequest); @@ -188,4 +189,4 @@ export class RequestQueueCollection { this._isProcessing = false; } -} \ No newline at end of file +} From ff51b3b443c533aee2e44c08f1e34260fda00c80 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:43:34 -0700 Subject: [PATCH 05/19] Mark all findNet* functions as nullable --- src/omnisharp/protocol.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index 977a8991c..e50693925 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -957,36 +957,36 @@ export namespace V2 { } } -export function findNetFrameworkTargetFramework(project: MSBuildProject): TargetFramework { - let regexp = new RegExp('^net[1-4]'); +export function findNetFrameworkTargetFramework(project: MSBuildProject): TargetFramework | undefined { + const regexp = new RegExp('^net[1-4]'); return project.TargetFrameworks.find(tf => regexp.test(tf.ShortName)); } -export function findNetCoreTargetFramework(project: MSBuildProject): TargetFramework { +export function findNetCoreTargetFramework(project: MSBuildProject): TargetFramework | undefined { return findNetCoreAppTargetFramework(project) ?? findModernNetFrameworkTargetFramework(project); } -export function findNetCoreAppTargetFramework(project: MSBuildProject): TargetFramework { +export function findNetCoreAppTargetFramework(project: MSBuildProject): TargetFramework | undefined { return project.TargetFrameworks.find(tf => tf.ShortName.startsWith('netcoreapp')); } -export function findModernNetFrameworkTargetFramework(project: MSBuildProject): TargetFramework { - let regexp = new RegExp('^net[5-9]'); +export function findModernNetFrameworkTargetFramework(project: MSBuildProject): TargetFramework | undefined { + const regexp = new RegExp('^net[5-9]'); const targetFramework = project.TargetFrameworks.find(tf => regexp.test(tf.ShortName)); // Shortname is being reported as net50 instead of net5.0 if (targetFramework !== undefined && targetFramework.ShortName.charAt(4) !== ".") { - targetFramework.ShortName = targetFramework.ShortName.substr(0, 4) + "." + targetFramework.ShortName.substr(4); + targetFramework.ShortName = `${targetFramework.ShortName.substring(0, 4)}.${targetFramework.ShortName.substring(4)}`; } return targetFramework; } -export function findNetStandardTargetFramework(project: MSBuildProject): TargetFramework { +export function findNetStandardTargetFramework(project: MSBuildProject): TargetFramework | undefined { return project.TargetFrameworks.find(tf => tf.ShortName.startsWith('netstandard')); } -export function isDotNetCoreProject(project: MSBuildProject): Boolean { +export function isDotNetCoreProject(project: MSBuildProject): boolean { return findNetCoreTargetFramework(project) !== undefined || findNetStandardTargetFramework(project) !== undefined || findNetFrameworkTargetFramework(project) !== undefined; From 6b6a2e4640ffafdc337f39ec3ebbfdc6d17415ad Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:47:37 -0700 Subject: [PATCH 06/19] Nullability fixes for options * Change optional properties to nullable to better reflect their status * Simplify nullable config retrieval * Simplify readUseGlobalMonoOptions --- src/omnisharp/options.ts | 76 ++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/src/omnisharp/options.ts b/src/omnisharp/options.ts index 1025f5b0a..bd7e63ff4 100644 --- a/src/omnisharp/options.ts +++ b/src/omnisharp/options.ts @@ -7,7 +7,7 @@ import { vscode, WorkspaceConfiguration } from '../vscodeAdapter'; export class Options { constructor( - public path: string, + public path: string | undefined, public useModernNet: boolean, public useGlobalMono: string, public waitForDebugger: boolean, @@ -48,13 +48,13 @@ export class Options { public inlayHintsForImplicitVariableTypes: boolean, public inlayHintsForLambdaParameterTypes: boolean, public inlayHintsForImplicitObjectCreation: boolean, - public razorPluginPath?: string, - public defaultLaunchSolution?: string, - public monoPath?: string, - public dotnetPath?: string, - public excludePaths?: string[], - public maxProjectFileCountForDiagnosticAnalysis?: number | null, - public testRunSettings?: string) { + public razorPluginPath: string | undefined, + public defaultLaunchSolution: string | undefined, + public monoPath: string | undefined, + public dotnetPath: string | undefined, + public excludePaths: string[], + public maxProjectFileCountForDiagnosticAnalysis: number, + public testRunSettings: string | undefined) { } public static Read(vscode: vscode): Options { @@ -72,8 +72,8 @@ export class Options { const path = Options.readPathOption(csharpConfig, omnisharpConfig); const useModernNet = omnisharpConfig.get("useModernNet", false); const useGlobalMono = Options.readUseGlobalMonoOption(omnisharpConfig, csharpConfig); - const monoPath = omnisharpConfig.get('monoPath', undefined) || undefined; - const dotnetPath = omnisharpConfig.get('dotnetPath', undefined) || undefined; + const monoPath = omnisharpConfig.get('monoPath'); + const dotnetPath = omnisharpConfig.get('dotnetPath'); const waitForDebugger = omnisharpConfig.get('waitForDebugger', false); @@ -87,7 +87,7 @@ export class Options { const projectLoadTimeout = omnisharpConfig.get('projectLoadTimeout', 60); const maxProjectResults = omnisharpConfig.get('maxProjectResults', 250); - const defaultLaunchSolution = omnisharpConfig.get('defaultLaunchSolution', undefined); + const defaultLaunchSolution = omnisharpConfig.get('defaultLaunchSolution'); const useEditorFormattingSettings = omnisharpConfig.get('useEditorFormattingSettings', true); const enableRoslynAnalyzers = omnisharpConfig.get('enableRoslynAnalyzers', false); @@ -130,13 +130,13 @@ export class Options { const enableMsBuildLoadProjectsOnDemand = omnisharpConfig.get('enableMsBuildLoadProjectsOnDemand', false); - const razorDisabled = !!razorConfig && razorConfig.get('disabled', false); - const razorDevMode = !!razorConfig && razorConfig.get('devmode', false); - const razorPluginPath = razorConfig ? razorConfig.get('plugin.path', undefined) : undefined; + const razorDisabled = razorConfig?.get('disabled', false) ?? false; + const razorDevMode = razorConfig?.get('devmode', false) ?? false; + const razorPluginPath = razorConfig?.get('plugin.path') ?? undefined; - const maxProjectFileCountForDiagnosticAnalysis = csharpConfig.get('maxProjectFileCountForDiagnosticAnalysis', 1000); + const maxProjectFileCountForDiagnosticAnalysis = csharpConfig.get('maxProjectFileCountForDiagnosticAnalysis', 1000); - const testRunSettings = omnisharpConfig.get('testRunSettings', undefined); + const testRunSettings = omnisharpConfig.get('testRunSettings'); const excludePaths = this.getExcludedPaths(vscode); @@ -193,10 +193,7 @@ export class Options { } public static getExcludedPaths(vscode: vscode, includeSearchExcludes: boolean = false): string[] { - let workspaceConfig = vscode.workspace.getConfiguration(undefined, null); - if (!workspaceConfig) { - return []; - } + const workspaceConfig = vscode.workspace.getConfiguration(); let excludePaths = getExcludes(workspaceConfig, 'files.exclude'); @@ -207,8 +204,8 @@ export class Options { return excludePaths; function getExcludes(config: WorkspaceConfiguration, option: string): string[] { - let optionValue = config.get<{ [i: string]: boolean }>(option); - if (!optionValue) { + const optionValue = config.get<{ [i: string]: boolean }>(option); + if (optionValue === undefined) { return []; } @@ -218,7 +215,7 @@ export class Options { } } - private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string | null { + private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string | undefined { if (omnisharpConfig.has('path')) { // If 'omnisharp.path' setting was found, use it. return omnisharpConfig.get('path'); @@ -228,32 +225,27 @@ export class Options { return csharpConfig.get('omnisharp'); } else { - // Otherwise, null. - return null; + // Otherwise, undefined. + return undefined; } } private static readUseGlobalMonoOption(omnisharpConfig: WorkspaceConfiguration, csharpConfig: WorkspaceConfiguration): string { - function toUseGlobalMonoValue(value: boolean): string { + function toUseGlobalMonoValue(value?: boolean): string | undefined { + if (value === undefined) { + return undefined; + } + // True means 'always' and false means 'auto'. return value ? "always" : "auto"; } - if (omnisharpConfig.has('useGlobalMono')) { - // If 'omnisharp.useGlobalMono' setting was found, just use it. - return omnisharpConfig.get('useGlobalMono', "auto"); - } - else if (omnisharpConfig.has('useMono')) { - // BACKCOMPAT: If 'omnisharp.useMono' setting was found, true maps to "always" and false maps to "auto" - return toUseGlobalMonoValue(omnisharpConfig.get('useMono')); - } - else if (csharpConfig.has('omnisharpUsesMono')) { - // BACKCOMPAT: If 'csharp.omnisharpUsesMono' setting was found, true maps to "always" and false maps to "auto" - return toUseGlobalMonoValue(csharpConfig.get('omnisharpUsesMono')); - } - else { - // Otherwise, the default value is "auto". - return "auto"; - } + // If 'omnisharp.useGlobalMono' is set, use that. + // Otherwise, for backcompat, look for 'omnisharp.useMono' and 'csharp.omnisharpUsesMono'. + // If both of those aren't found, return 'auto' as the default value. + return omnisharpConfig.get('useGlobalMono') ?? + toUseGlobalMonoValue(omnisharpConfig.get('useMono')) ?? + toUseGlobalMonoValue(csharpConfig.get('omnisharpUsesMono')) ?? + "auto"; } } From 9b216d207c14c62434e15429aacde4d2e81a0284 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:48:24 -0700 Subject: [PATCH 07/19] Explicitly check omnisharpPath for undefinedness --- src/omnisharp/OmnisharpManager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/omnisharp/OmnisharpManager.ts b/src/omnisharp/OmnisharpManager.ts index 30b7d223e..952bfbca8 100644 --- a/src/omnisharp/OmnisharpManager.ts +++ b/src/omnisharp/OmnisharpManager.ts @@ -22,10 +22,10 @@ export class OmnisharpManager { private platformInfo: PlatformInformation) { } - public async GetOmniSharpLaunchInfo(defaultOmnisharpVersion: string, omnisharpPath: string, useFramework: boolean, serverUrl: string, latestVersionFileServerPath: string, installPath: string, extensionPath: string): Promise { - if (!omnisharpPath) { + public async GetOmniSharpLaunchInfo(defaultOmnisharpVersion: string, omnisharpPath: string | undefined, useFramework: boolean, serverUrl: string, latestVersionFileServerPath: string, installPath: string, extensionPath: string): Promise { + if (omnisharpPath === undefined) { // If omnisharpPath was not specified, return the default path. - let basePath = path.resolve(extensionPath, '.omnisharp', defaultOmnisharpVersion + (useFramework ? '' : `-net${modernNetVersion}`)); + const basePath = path.resolve(extensionPath, '.omnisharp', defaultOmnisharpVersion + (useFramework ? '' : `-net${modernNetVersion}`)); return this.GetLaunchInfo(this.platformInfo, useFramework, basePath); } From e6a3014784a519e2609720f2f713d369ed75739d Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:49:02 -0700 Subject: [PATCH 08/19] Always return bool in DownloadAndInstallOmnisharp --- src/omnisharp/OmnisharpDownloader.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/omnisharp/OmnisharpDownloader.ts b/src/omnisharp/OmnisharpDownloader.ts index f580dc88f..1918dfc78 100644 --- a/src/omnisharp/OmnisharpDownloader.ts +++ b/src/omnisharp/OmnisharpDownloader.ts @@ -35,9 +35,8 @@ export class OmnisharpDownloader { this.eventStream.post(new InstallationSuccess()); return true; } - - return false; } + return false; } public async GetLatestVersion(serverUrl: string, latestVersionFileServerPath: string): Promise { From 7a747617e9007ce7fd23e1db1eb8b18076aee36d Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:49:23 -0700 Subject: [PATCH 09/19] Assert non-null in launcher.ts --- src/omnisharp/launcher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/omnisharp/launcher.ts b/src/omnisharp/launcher.ts index 51cc82219..a2ff4b0b0 100644 --- a/src/omnisharp/launcher.ts +++ b/src/omnisharp/launcher.ts @@ -107,7 +107,7 @@ export function resourcesToLaunchTargets(resources: vscode.Uri[]): LaunchTarget[ let buckets: vscode.Uri[]; if (workspaceFolderToUriMap.has(folder.index)) { - buckets = workspaceFolderToUriMap.get(folder.index); + buckets = workspaceFolderToUriMap.get(folder.index)!; // Ensured valid via has. } else { buckets = []; workspaceFolderToUriMap.set(folder.index, buckets); From 6eea46b15dc616d18d609482def4c4dae5080b11 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:49:50 -0700 Subject: [PATCH 10/19] Guard against possible undefined in LanguageMiddlewareFeature --- src/omnisharp/LanguageMiddlewareFeature.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/omnisharp/LanguageMiddlewareFeature.ts b/src/omnisharp/LanguageMiddlewareFeature.ts index a7e4b77ba..fe174708e 100644 --- a/src/omnisharp/LanguageMiddlewareFeature.ts +++ b/src/omnisharp/LanguageMiddlewareFeature.ts @@ -22,14 +22,15 @@ type RemapParameterType = GetRemapType Date: Sun, 24 Apr 2022 10:50:06 -0700 Subject: [PATCH 11/19] Change fileToOpen to a nullable type --- src/omnisharp/fileOperationsResponseEditBuilder.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/omnisharp/fileOperationsResponseEditBuilder.ts b/src/omnisharp/fileOperationsResponseEditBuilder.ts index 8c4bd6c66..bf2ec84c0 100644 --- a/src/omnisharp/fileOperationsResponseEditBuilder.ts +++ b/src/omnisharp/fileOperationsResponseEditBuilder.ts @@ -12,7 +12,7 @@ import { toRange2 } from './typeConversion'; export async function buildEditForResponse(changes: FileOperationResponse[], languageMiddlewareFeature: LanguageMiddlewareFeature, token: vscode.CancellationToken): Promise { let edit = new vscode.WorkspaceEdit(); - let fileToOpen: Uri = null; + let fileToOpen: Uri | undefined; if (!changes || !Array.isArray(changes) || !changes.length) { return true; @@ -55,7 +55,7 @@ export async function buildEditForResponse(changes: FileOperationResponse[], lan // and replaced with a command that can only close the active editor. // If files were renamed that weren't the active editor, their tabs will // be left open and marked as "deleted" by VS Code - return fileToOpen != null + return fileToOpen !== undefined ? applyEditPromise.then(_ => { return vscode.commands.executeCommand("vscode.open", fileToOpen); }) From 592cd72271ee2d3cc8fe2eb5d1f96af6c8994a53 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:50:29 -0700 Subject: [PATCH 12/19] Remove unused buildPromiseChain --- src/common.ts | 8 +------- test/unitTests/common.test.ts | 18 +----------------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/common.ts b/src/common.ts index b3fd90247..13c196dc5 100644 --- a/src/common.ts +++ b/src/common.ts @@ -61,12 +61,6 @@ export function safeLength(arr: T[] | undefined) { return arr ? arr.length : 0; } -export async function buildPromiseChain(array: T[], builder: (item: T) => Promise): Promise { - return array.reduce( - async (promise, n) => promise.then(async () => builder(n)), - Promise.resolve(null)); -} - export async function execChildProcess(command: string, workingDirectory: string = getExtensionPath()): Promise { return new Promise((resolve, reject) => { cp.exec(command, { cwd: workingDirectory, maxBuffer: 500 * 1024 }, (error, stdout, stderr) => { @@ -229,4 +223,4 @@ export function isSubfolderOf(subfolder: string, folder: string): boolean { } } } -} \ No newline at end of file +} diff --git a/test/unitTests/common.test.ts b/test/unitTests/common.test.ts index a9a083291..344c1a6e1 100644 --- a/test/unitTests/common.test.ts +++ b/test/unitTests/common.test.ts @@ -5,29 +5,13 @@ import * as path from 'path'; -import { buildPromiseChain, isSubfolderOf, safeLength, sum } from '../../src/common'; +import { isSubfolderOf, safeLength, sum } from '../../src/common'; import { should, expect } from 'chai'; suite("Common", () => { suiteSetup(() => should()); - suite("buildPromiseChain", () => { - test("produce a sequence of promises", async () => { - let array: number[] = []; - let items = [1, 2, 3, 4, 5]; - - let promise = buildPromiseChain(items, async n => new Promise((resolve, reject) => { - array.push(n); - resolve(); - })); - - return promise.then(() => { - array.should.deep.equal([1, 2, 3, 4, 5]); - }); - }); - }); - suite("safeLength", () => { test("return 0 for empty array", () => { let array: any[] = []; From 6ff620f9206f831a34d8a886020aa06e3be732da Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:53:07 -0700 Subject: [PATCH 13/19] Some low-hanging nullability fixes in server --- src/omnisharp/server.ts | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index b8927b497..29fed75c5 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -86,12 +86,12 @@ export class OmniSharpServer { private _readLine: ReadLine; private _disposables: CompositeDisposable; - private _delayTrackers: { [requestName: string]: DelayTracker }; - private _telemetryIntervalId: NodeJS.Timer = undefined; + private _delayTrackers!: { [requestName: string]: DelayTracker }; // Initialized via _start + private _telemetryIntervalId: NodeJS.Timer | undefined; private _eventBus = new EventEmitter(); private _state: ServerState = ServerState.Stopped; - private _launchTarget: LaunchTarget; + private _launchTarget: LaunchTarget | undefined; private _requestQueue: RequestQueueCollection; private _serverProcess: ChildProcess; private _sessionProperties: { [key: string]: any } = {}; @@ -165,10 +165,8 @@ export class OmniSharpServer { } } - public getSolutionPathOrFolder(): string { - return this._launchTarget - ? this._launchTarget.target - : undefined; + public getSolutionPathOrFolder(): string | undefined { + return this._launchTarget?.target; } // --- eventing @@ -342,10 +340,9 @@ export class OmniSharpServer { '--loglevel', options.loggingLevel ]; - let razorPluginPath: string; if (!options.razorDisabled) { // Razor support only exists for certain platforms, so only load the plugin if present - razorPluginPath = options.razorPluginPath || path.join( + const razorPluginPath = options.razorPluginPath ?? path.join( this.extensionPath, '.razor', 'OmniSharpPlugin', @@ -417,7 +414,8 @@ export class OmniSharpServer { try { launchInfo = await this._omnisharpManager.GetOmniSharpLaunchInfo(this.packageJSON.defaults.omniSharp, options.path, /* useFramework */ !options.useModernNet, serverUrl, latestVersionFileServerPath, installPath, this.extensionPath); } - catch (error) { + catch (e) { + const error = e as Error; // Unsafe TypeScript hack to recognize the catch type as Error. this.eventStream.post(new ObservableEvents.OmnisharpFailure(`Error occurred in loading omnisharp from omnisharp.path\nCould not start the server due to ${error.toString()}`, error)); return; } @@ -429,11 +427,11 @@ export class OmniSharpServer { const launchResult = await launchOmniSharp(cwd, args, launchInfo, this.platformInfo, options, this.monoResolver, this.dotnetResolver); this.eventStream.post(new ObservableEvents.OmnisharpLaunch(launchResult.hostVersion, launchResult.hostPath, launchResult.hostIsMono, launchResult.command, launchResult.process.pid)); - if (razorPluginPath && options.razorPluginPath) { - if (fs.existsSync(razorPluginPath)) { - this.eventStream.post(new ObservableEvents.RazorPluginPathSpecified(razorPluginPath)); + if (!options.razorDisabled && options.razorPluginPath !== undefined) { + if (fs.existsSync(options.razorPluginPath)) { + this.eventStream.post(new ObservableEvents.RazorPluginPathSpecified(options.razorPluginPath)); } else { - this.eventStream.post(new ObservableEvents.RazorPluginPathDoesNotExist(razorPluginPath)); + this.eventStream.post(new ObservableEvents.RazorPluginPathDoesNotExist(options.razorPluginPath)); } } @@ -534,13 +532,13 @@ export class OmniSharpServer { }); } - public async restart(launchTarget: LaunchTarget = this._launchTarget): Promise { + public async restart(launchTarget: LaunchTarget | undefined = this._launchTarget): Promise { if (this._state == ServerState.Starting) { this.eventStream.post(new ObservableEvents.OmnisharpServerOnServerError("Attempt to restart OmniSharp server failed because another server instance is starting.")); return; } - if (launchTarget) { + if (launchTarget !== undefined) { await this.stop(); this.eventStream.post(new ObservableEvents.OmnisharpRestart()); const options = this.optionProvider.GetLatestOptions(); @@ -548,7 +546,7 @@ export class OmniSharpServer { } } - public autoStart(preferredPath: string): Thenable { + public autoStart(preferredPath: string | undefined): Thenable { const options = this.optionProvider.GetLatestOptions(); return findLaunchTargets(options).then(async launchTargets => { // If there aren't any potential launch targets, we create file watcher and try to @@ -584,7 +582,7 @@ export class OmniSharpServer { // If there's more than one launch target, we start the server if one of the targets // matches the preferred path. - if (preferredPath) { + if (preferredPath !== undefined) { const preferredLaunchTarget = launchTargets.find((a) => a.target === preferredPath); if (preferredLaunchTarget) { return this.restart(preferredLaunchTarget); From 36ddb31d423f23c7577fadfaf70b2421e621d41c Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 10:53:26 -0700 Subject: [PATCH 14/19] Make localDisposables a nullable type in extension --- src/omnisharp/extension.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/omnisharp/extension.ts b/src/omnisharp/extension.ts index 01171fa3c..4cce16249 100644 --- a/src/omnisharp/extension.ts +++ b/src/omnisharp/extension.ts @@ -68,7 +68,7 @@ export async function activate(context: vscode.ExtensionContext, packageJSON: an const languageMiddlewareFeature = new LanguageMiddlewareFeature(); languageMiddlewareFeature.register(); disposables.add(languageMiddlewareFeature); - let localDisposables: CompositeDisposable; + let localDisposables: CompositeDisposable | undefined; const testManager = new TestManager(optionProvider, server, eventStream, languageMiddlewareFeature); const completionProvider = new CompletionProvider(server, languageMiddlewareFeature); @@ -130,7 +130,7 @@ export async function activate(context: vscode.ExtensionContext, packageJSON: an if (localDisposables) { localDisposables.dispose(); } - localDisposables = null; + localDisposables = undefined; })); disposables.add(registerCommands(context, server, platformInfo, eventStream, optionProvider, omnisharpMonoResolver, packageJSON, extensionPath)); From ca1e483c44c7b1b44b8a78877408d2ba6fdbba35 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 11:17:30 -0700 Subject: [PATCH 15/19] Mark properties in OmnisharpLaunch as nullable --- src/omnisharp/loggingEvents.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/omnisharp/loggingEvents.ts b/src/omnisharp/loggingEvents.ts index b4ff179ea..dc463ec63 100644 --- a/src/omnisharp/loggingEvents.ts +++ b/src/omnisharp/loggingEvents.ts @@ -46,7 +46,7 @@ export class OmnisharpInitialisation implements BaseEvent { export class OmnisharpLaunch implements BaseEvent { type = EventType.OmnisharpLaunch; - constructor(public hostVersion: string, public hostPath: string, public hostIsMono: boolean, public command: string, public pid: number) { } + constructor(public hostVersion: string | undefined, public hostPath: string | undefined, public hostIsMono: boolean, public command: string, public pid: number) { } } export class PackageInstallStart implements BaseEvent { From 3f4edeb73b3734b6df3aa631ad98a1ba7d605eae Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 12:14:47 -0700 Subject: [PATCH 16/19] Update options.path-related unit tests --- test/unitTests/OmnisharpManager.test.ts | 2 +- test/unitTests/optionStream.test.ts | 4 ++-- test/unitTests/options.test.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unitTests/OmnisharpManager.test.ts b/test/unitTests/OmnisharpManager.test.ts index e32b6aed7..e13a8568b 100644 --- a/test/unitTests/OmnisharpManager.test.ts +++ b/test/unitTests/OmnisharpManager.test.ts @@ -136,7 +136,7 @@ suite(OmnisharpManager.name, () => { }); test('Returns the default path if the omnisharp path is not set', async () => { - let launchInfo = await manager.GetOmniSharpLaunchInfo(defaultVersion, "", useFramework, server.baseUrl, latestfilePath, installPath, extensionPath); + const launchInfo = await manager.GetOmniSharpLaunchInfo(defaultVersion, undefined, useFramework, server.baseUrl, latestfilePath, installPath, extensionPath); if (useFramework) { expect(launchInfo.LaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion + suffix, elem.executable)); if (elem.platformInfo.isWindows()) { diff --git a/test/unitTests/optionStream.test.ts b/test/unitTests/optionStream.test.ts index df53949c5..4af2fd414 100644 --- a/test/unitTests/optionStream.test.ts +++ b/test/unitTests/optionStream.test.ts @@ -36,7 +36,7 @@ suite('OptionStream', () => { }); test('Returns the default options if there is no change', () => { - expect(options.path).to.be.null; + expect(options.path).to.be.undefined; options.useGlobalMono.should.equal("auto"); options.waitForDebugger.should.equal(false); options.loggingLevel.should.equal("information"); @@ -60,7 +60,7 @@ suite('OptionStream', () => { }); test('Gives the changed option when the omnisharp config changes', () => { - expect(options.path).to.be.null; + expect(options.path).to.be.undefined; let changingConfig = "omnisharp"; updateConfig(vscode, changingConfig, 'path', "somePath"); listenerFunction.forEach(listener => listener(GetConfigChangeEvent(changingConfig))); diff --git a/test/unitTests/options.test.ts b/test/unitTests/options.test.ts index 357a27211..809b308fb 100644 --- a/test/unitTests/options.test.ts +++ b/test/unitTests/options.test.ts @@ -13,7 +13,7 @@ suite("Options tests", () => { test('Verify defaults', () => { const vscode = getVSCodeWithConfig(); const options = Options.Read(vscode); - expect(options.path).to.be.null; + expect(options.path).to.be.undefined; options.useGlobalMono.should.equal("auto"); expect(options.monoPath).to.be.undefined; options.waitForDebugger.should.equal(false); From f6655825bef32fdb1c8c04d393d128df91747912 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 20:26:52 -0700 Subject: [PATCH 17/19] Test for empty omnisharp.path --- src/omnisharp/OmnisharpManager.ts | 2 +- test/unitTests/OmnisharpManager.test.ts | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/omnisharp/OmnisharpManager.ts b/src/omnisharp/OmnisharpManager.ts index 952bfbca8..1253283d4 100644 --- a/src/omnisharp/OmnisharpManager.ts +++ b/src/omnisharp/OmnisharpManager.ts @@ -23,7 +23,7 @@ export class OmnisharpManager { } public async GetOmniSharpLaunchInfo(defaultOmnisharpVersion: string, omnisharpPath: string | undefined, useFramework: boolean, serverUrl: string, latestVersionFileServerPath: string, installPath: string, extensionPath: string): Promise { - if (omnisharpPath === undefined) { + if (omnisharpPath === undefined || omnisharpPath.length === 0) { // If omnisharpPath was not specified, return the default path. const basePath = path.resolve(extensionPath, '.omnisharp', defaultOmnisharpVersion + (useFramework ? '' : `-net${modernNetVersion}`)); return this.GetLaunchInfo(this.platformInfo, useFramework, basePath); diff --git a/test/unitTests/OmnisharpManager.test.ts b/test/unitTests/OmnisharpManager.test.ts index e13a8568b..9ae0bafb7 100644 --- a/test/unitTests/OmnisharpManager.test.ts +++ b/test/unitTests/OmnisharpManager.test.ts @@ -153,6 +153,24 @@ suite(OmnisharpManager.name, () => { } }); + test('Returns the default path if the omnisharp path is empty', async () => { + const launchInfo = await manager.GetOmniSharpLaunchInfo(defaultVersion, "", useFramework, server.baseUrl, latestfilePath, installPath, extensionPath); + if (useFramework) { + expect(launchInfo.LaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion + suffix, elem.executable)); + if (elem.platformInfo.isWindows()) { + expect(launchInfo.MonoLaunchPath).to.be.undefined; + } + else { + expect(launchInfo.MonoLaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion, "omnisharp", "OmniSharp.exe")); + } + } + else { + expect(launchInfo.LaunchPath).to.be.undefined; + expect(launchInfo.MonoLaunchPath).to.be.undefined; + expect(launchInfo.DotnetLaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion + suffix, elem.executable)); + } + }); + test('Installs the latest version and returns the launch path ', async () => { let launchInfo = await manager.GetOmniSharpLaunchInfo(defaultVersion, "latest", useFramework, server.baseUrl, latestfilePath, installPath, extensionPath); if (useFramework) { From 43bacd34f58c2561529e5899542dc343d3e8af41 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 24 Apr 2022 20:29:11 -0700 Subject: [PATCH 18/19] Try not setting defaults for string options --- package.json | 37 ++++++------------------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/package.json b/package.json index fabdfee27..559169f6f 100644 --- a/package.json +++ b/package.json @@ -915,11 +915,7 @@ "description": "Show hints for implicit object creation" }, "omnisharp.path": { - "type": [ - "string", - "null" - ], - "default": null, + "type": "string", "scope": "machine", "description": "Specifies the path to OmniSharp. When left empty the OmniSharp version pinned to the C# Extension is used. This can be the absolute path to an OmniSharp executable, a specific version number, or \"latest\". If a version number or \"latest\" is specified, the appropriate version of OmniSharp will be downloaded on your behalf. Setting \"latest\" is an opt-in into latest beta releases of OmniSharp." }, @@ -946,20 +942,12 @@ "description": "Launch OmniSharp with the globally-installed Mono. If set to \"always\", \"mono\" version 6.4.0 or greater must be available on the PATH. If set to \"auto\", OmniSharp will be launched with \"mono\" if version 6.4.0 or greater is available on the PATH." }, "omnisharp.monoPath": { - "type": [ - "string", - "null" - ], - "default": null, + "type": "string", "scope": "machine", "description": "Specifies the path to a mono installation to use when \"useGlobalMono\" is set to \"always\", instead of the default system one. Example: \"/Library/Frameworks/Mono.framework/Versions/Current\"" }, "omnisharp.dotnetPath": { - "type": [ - "string", - "null" - ], - "default": null, + "type": "string", "scope": "window", "description": "Specified the path to a dotnet installation to use when \"useModernNet\" is set to true, instead of the default system one. Example: \"/home/username/mycustomdotnetdirectory\"." }, @@ -998,7 +986,6 @@ }, "omnisharp.defaultLaunchSolution": { "type": "string", - "default": null, "description": "The name of the default solution used at start up if the repo has multiple solutions. e.g.'MyAwesomeSolution.sln'. Default value is `null` which will cause the first in alphabetical order to be chosen." }, "omnisharp.useEditorFormattingSettings": { @@ -1063,19 +1050,11 @@ "description": "Only run analyzers against open files when 'enableRoslynAnalyzers' is true" }, "omnisharp.testRunSettings": { - "type": [ - "string", - "null" - ], - "default": null, + "type": "string", "description": "Path to the .runsettings file which should be used when running unit tests." }, "razor.plugin.path": { - "type": [ - "string", - "null" - ], - "default": null, + "type": "string", "scope": "machine", "description": "Overrides the path to the Razor plugin dll." }, @@ -1090,11 +1069,7 @@ "description": "Specifies whether to disable Razor language features." }, "razor.languageServer.directory": { - "type": [ - "string", - "null" - ], - "default": null, + "type": "string", "scope": "machine", "description": "Overrides the path to the Razor Language Server directory." }, From 6576c9584e285cd95fe045137501f39f62040107 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 25 Apr 2022 04:31:27 +0000 Subject: [PATCH 19/19] Default string settings to the empty string --- src/features/dotnetTest.ts | 2 +- src/omnisharp/OmniSharpDotnetResolver.ts | 2 +- src/omnisharp/OmniSharpMonoResolver.ts | 5 ++- src/omnisharp/OmnisharpManager.ts | 4 +-- src/omnisharp/options.ts | 43 ++++++++++++------------ src/omnisharp/server.ts | 4 +-- test/unitTests/OmnisharpManager.test.ts | 18 ---------- test/unitTests/optionStream.test.ts | 8 ++--- test/unitTests/options.test.ts | 8 ++--- 9 files changed, 38 insertions(+), 56 deletions(-) diff --git a/src/features/dotnetTest.ts b/src/features/dotnetTest.ts index 405676cc5..c7b8a8283 100644 --- a/src/features/dotnetTest.ts +++ b/src/features/dotnetTest.ts @@ -194,7 +194,7 @@ export default class TestManager extends AbstractProvider { private _getRunSettings(filename: string): string | undefined { const testSettingsPath = this.optionProvider.GetLatestOptions().testRunSettings; - if (!testSettingsPath) { + if (testSettingsPath.length === 0) { return undefined; } diff --git a/src/omnisharp/OmniSharpDotnetResolver.ts b/src/omnisharp/OmniSharpDotnetResolver.ts index 136329f89..104309f8a 100644 --- a/src/omnisharp/OmniSharpDotnetResolver.ts +++ b/src/omnisharp/OmniSharpDotnetResolver.ts @@ -22,7 +22,7 @@ export class OmniSharpDotnetResolver implements IHostExecutableResolver { const dotnet = this.platformInfo.isWindows() ? 'dotnet.exe' : 'dotnet'; const env = { ...process.env }; - if (options.dotnetPath) { + if (options.dotnetPath.length > 0) { env['PATH'] = options.dotnetPath + path.delimiter + env['PATH']; } diff --git a/src/omnisharp/OmniSharpMonoResolver.ts b/src/omnisharp/OmniSharpMonoResolver.ts index 53bfd8e6e..71c566795 100644 --- a/src/omnisharp/OmniSharpMonoResolver.ts +++ b/src/omnisharp/OmniSharpMonoResolver.ts @@ -19,7 +19,7 @@ export class OmniSharpMonoResolver implements IHostExecutableResolver { private async configureEnvironmentAndGetInfo(options: Options): Promise { let env = { ...process.env }; let monoPath: string; - if (options.useGlobalMono !== "never" && options.monoPath !== undefined) { + if (options.useGlobalMono !== "never" && options.monoPath.length > 0) { env['PATH'] = path.join(options.monoPath, 'bin') + path.delimiter + env['PATH']; env['MONO_GAC_PREFIX'] = options.monoPath; monoPath = options.monoPath; @@ -40,7 +40,7 @@ export class OmniSharpMonoResolver implements IHostExecutableResolver { if (options.useGlobalMono === "always") { let isMissing = monoInfo.version === undefined; if (isMissing) { - const suggestedAction = options.monoPath + const suggestedAction = options.monoPath.length > 0 ? "Update the \"omnisharp.monoPath\" setting to point to the folder containing Mono's '/bin' folder." : "Ensure that Mono's '/bin' folder is added to your environment's PATH variable."; throw new Error(`Unable to find Mono. ${suggestedAction}`); @@ -61,4 +61,3 @@ export class OmniSharpMonoResolver implements IHostExecutableResolver { return undefined; } } - diff --git a/src/omnisharp/OmnisharpManager.ts b/src/omnisharp/OmnisharpManager.ts index 1253283d4..c2e6947ca 100644 --- a/src/omnisharp/OmnisharpManager.ts +++ b/src/omnisharp/OmnisharpManager.ts @@ -22,8 +22,8 @@ export class OmnisharpManager { private platformInfo: PlatformInformation) { } - public async GetOmniSharpLaunchInfo(defaultOmnisharpVersion: string, omnisharpPath: string | undefined, useFramework: boolean, serverUrl: string, latestVersionFileServerPath: string, installPath: string, extensionPath: string): Promise { - if (omnisharpPath === undefined || omnisharpPath.length === 0) { + public async GetOmniSharpLaunchInfo(defaultOmnisharpVersion: string, omnisharpPath: string, useFramework: boolean, serverUrl: string, latestVersionFileServerPath: string, installPath: string, extensionPath: string): Promise { + if (omnisharpPath.length === 0) { // If omnisharpPath was not specified, return the default path. const basePath = path.resolve(extensionPath, '.omnisharp', defaultOmnisharpVersion + (useFramework ? '' : `-net${modernNetVersion}`)); return this.GetLaunchInfo(this.platformInfo, useFramework, basePath); diff --git a/src/omnisharp/options.ts b/src/omnisharp/options.ts index bd7e63ff4..99610bd3f 100644 --- a/src/omnisharp/options.ts +++ b/src/omnisharp/options.ts @@ -7,7 +7,7 @@ import { vscode, WorkspaceConfiguration } from '../vscodeAdapter'; export class Options { constructor( - public path: string | undefined, + public path: string, public useModernNet: boolean, public useGlobalMono: string, public waitForDebugger: boolean, @@ -48,13 +48,13 @@ export class Options { public inlayHintsForImplicitVariableTypes: boolean, public inlayHintsForLambdaParameterTypes: boolean, public inlayHintsForImplicitObjectCreation: boolean, - public razorPluginPath: string | undefined, - public defaultLaunchSolution: string | undefined, - public monoPath: string | undefined, - public dotnetPath: string | undefined, + public razorPluginPath: string, + public defaultLaunchSolution: string, + public monoPath: string, + public dotnetPath: string, public excludePaths: string[], public maxProjectFileCountForDiagnosticAnalysis: number, - public testRunSettings: string | undefined) { + public testRunSettings: string) { } public static Read(vscode: vscode): Options { @@ -72,8 +72,13 @@ export class Options { const path = Options.readPathOption(csharpConfig, omnisharpConfig); const useModernNet = omnisharpConfig.get("useModernNet", false); const useGlobalMono = Options.readUseGlobalMonoOption(omnisharpConfig, csharpConfig); - const monoPath = omnisharpConfig.get('monoPath'); - const dotnetPath = omnisharpConfig.get('dotnetPath'); + + // VS Code coerces unset string settings to the empty string. + // Thus, to avoid dealing with the empty string AND undefined, + // explicitly pass in the empty string as the fallback if the setting + // isn't defined in package.json (which should never happen). + const monoPath = omnisharpConfig.get('monoPath', ''); + const dotnetPath = omnisharpConfig.get('dotnetPath', ''); const waitForDebugger = omnisharpConfig.get('waitForDebugger', false); @@ -87,7 +92,7 @@ export class Options { const projectLoadTimeout = omnisharpConfig.get('projectLoadTimeout', 60); const maxProjectResults = omnisharpConfig.get('maxProjectResults', 250); - const defaultLaunchSolution = omnisharpConfig.get('defaultLaunchSolution'); + const defaultLaunchSolution = omnisharpConfig.get('defaultLaunchSolution', ''); const useEditorFormattingSettings = omnisharpConfig.get('useEditorFormattingSettings', true); const enableRoslynAnalyzers = omnisharpConfig.get('enableRoslynAnalyzers', false); @@ -132,11 +137,11 @@ export class Options { const razorDisabled = razorConfig?.get('disabled', false) ?? false; const razorDevMode = razorConfig?.get('devmode', false) ?? false; - const razorPluginPath = razorConfig?.get('plugin.path') ?? undefined; + const razorPluginPath = razorConfig?.get('plugin.path', '') ?? ''; const maxProjectFileCountForDiagnosticAnalysis = csharpConfig.get('maxProjectFileCountForDiagnosticAnalysis', 1000); - const testRunSettings = omnisharpConfig.get('testRunSettings'); + const testRunSettings = omnisharpConfig.get('testRunSettings', ''); const excludePaths = this.getExcludedPaths(vscode); @@ -204,29 +209,25 @@ export class Options { return excludePaths; function getExcludes(config: WorkspaceConfiguration, option: string): string[] { - const optionValue = config.get<{ [i: string]: boolean }>(option); - if (optionValue === undefined) { - return []; - } - + const optionValue = config.get<{ [i: string]: boolean }>(option, {}); return Object.entries(optionValue) .filter(([key, value]) => value) .map(([key, value]) => key); } } - private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string | undefined { + private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string { if (omnisharpConfig.has('path')) { // If 'omnisharp.path' setting was found, use it. - return omnisharpConfig.get('path'); + return omnisharpConfig.get('path', ''); } else if (csharpConfig.has('omnisharp')) { // BACKCOMPAT: If 'csharp.omnisharp' setting was found, use it. - return csharpConfig.get('omnisharp'); + return csharpConfig.get('omnisharp', ''); } else { - // Otherwise, undefined. - return undefined; + // Otherwise, the empty string. + return ''; } } diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 29fed75c5..b7dda20ce 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -342,7 +342,7 @@ export class OmniSharpServer { if (!options.razorDisabled) { // Razor support only exists for certain platforms, so only load the plugin if present - const razorPluginPath = options.razorPluginPath ?? path.join( + const razorPluginPath = options.razorPluginPath.length > 0 ? options.razorPluginPath : path.join( this.extensionPath, '.razor', 'OmniSharpPlugin', @@ -427,7 +427,7 @@ export class OmniSharpServer { const launchResult = await launchOmniSharp(cwd, args, launchInfo, this.platformInfo, options, this.monoResolver, this.dotnetResolver); this.eventStream.post(new ObservableEvents.OmnisharpLaunch(launchResult.hostVersion, launchResult.hostPath, launchResult.hostIsMono, launchResult.command, launchResult.process.pid)); - if (!options.razorDisabled && options.razorPluginPath !== undefined) { + if (!options.razorDisabled && options.razorPluginPath.length > 0) { if (fs.existsSync(options.razorPluginPath)) { this.eventStream.post(new ObservableEvents.RazorPluginPathSpecified(options.razorPluginPath)); } else { diff --git a/test/unitTests/OmnisharpManager.test.ts b/test/unitTests/OmnisharpManager.test.ts index 9ae0bafb7..2723f4fa5 100644 --- a/test/unitTests/OmnisharpManager.test.ts +++ b/test/unitTests/OmnisharpManager.test.ts @@ -135,24 +135,6 @@ suite(OmnisharpManager.name, () => { expect(launchInfo.LaunchPath).to.be.equal(tmpFile.name); }); - test('Returns the default path if the omnisharp path is not set', async () => { - const launchInfo = await manager.GetOmniSharpLaunchInfo(defaultVersion, undefined, useFramework, server.baseUrl, latestfilePath, installPath, extensionPath); - if (useFramework) { - expect(launchInfo.LaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion + suffix, elem.executable)); - if (elem.platformInfo.isWindows()) { - expect(launchInfo.MonoLaunchPath).to.be.undefined; - } - else { - expect(launchInfo.MonoLaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion, "omnisharp", "OmniSharp.exe")); - } - } - else { - expect(launchInfo.LaunchPath).to.be.undefined; - expect(launchInfo.MonoLaunchPath).to.be.undefined; - expect(launchInfo.DotnetLaunchPath).to.be.equal(path.join(extensionPath, ".omnisharp", defaultVersion + suffix, elem.executable)); - } - }); - test('Returns the default path if the omnisharp path is empty', async () => { const launchInfo = await manager.GetOmniSharpLaunchInfo(defaultVersion, "", useFramework, server.baseUrl, latestfilePath, installPath, extensionPath); if (useFramework) { diff --git a/test/unitTests/optionStream.test.ts b/test/unitTests/optionStream.test.ts index 4af2fd414..ea7b7cf0a 100644 --- a/test/unitTests/optionStream.test.ts +++ b/test/unitTests/optionStream.test.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { should, expect } from 'chai'; +import { should } from 'chai'; import { ConfigurationChangeEvent, vscode } from "../../src/vscodeAdapter"; import { getVSCodeWithConfig, updateConfig } from "./testAssets/Fakes"; import Disposable from "../../src/Disposable"; @@ -36,7 +36,7 @@ suite('OptionStream', () => { }); test('Returns the default options if there is no change', () => { - expect(options.path).to.be.undefined; + options.path.should.equal(""); options.useGlobalMono.should.equal("auto"); options.waitForDebugger.should.equal(false); options.loggingLevel.should.equal("information"); @@ -56,11 +56,11 @@ suite('OptionStream', () => { options.enableDecompilationSupport.should.equal(false); options.enableImportCompletion.should.equal(false); options.enableAsyncCompletion.should.equal(false); - expect(options.defaultLaunchSolution).to.be.undefined; + options.defaultLaunchSolution.should.equal(""); }); test('Gives the changed option when the omnisharp config changes', () => { - expect(options.path).to.be.undefined; + options.path.should.equal(""); let changingConfig = "omnisharp"; updateConfig(vscode, changingConfig, 'path', "somePath"); listenerFunction.forEach(listener => listener(GetConfigChangeEvent(changingConfig))); diff --git a/test/unitTests/options.test.ts b/test/unitTests/options.test.ts index 809b308fb..c3d9d25d5 100644 --- a/test/unitTests/options.test.ts +++ b/test/unitTests/options.test.ts @@ -13,9 +13,9 @@ suite("Options tests", () => { test('Verify defaults', () => { const vscode = getVSCodeWithConfig(); const options = Options.Read(vscode); - expect(options.path).to.be.undefined; + options.path.should.equal(""); options.useGlobalMono.should.equal("auto"); - expect(options.monoPath).to.be.undefined; + options.monoPath.should.equal(""); options.waitForDebugger.should.equal(false); options.loggingLevel.should.equal("information"); options.autoStart.should.equal(true); @@ -35,8 +35,8 @@ suite("Options tests", () => { options.enableDecompilationSupport.should.equal(false); options.enableImportCompletion.should.equal(false); options.analyzeOpenDocumentsOnly.should.equal(false); - expect(options.testRunSettings).to.be.undefined; - expect(options.defaultLaunchSolution).to.be.undefined; + options.testRunSettings.should.equal(""); + options.defaultLaunchSolution.should.equal(""); }); test('Verify return no excluded paths when files.exclude empty', () => {