From 92045683147b18632f3b2171d015defc1228b949 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:10:55 -0700 Subject: [PATCH 01/16] Update *Adapter types --- src/statusBarItemAdapter.ts | 10 +++++----- src/textEditorAdapter.ts | 4 ++-- src/vscodeAdapter.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/statusBarItemAdapter.ts b/src/statusBarItemAdapter.ts index 1bf6ddbcf..e1b078ca2 100644 --- a/src/statusBarItemAdapter.ts +++ b/src/statusBarItemAdapter.ts @@ -12,7 +12,7 @@ export class StatusBarItemAdapter implements vscodeAdapter.StatusBarItem { return this.statusBarItem.alignment; } - get priority(): number { + get priority(): number | undefined { return this.statusBarItem.priority; } @@ -40,19 +40,19 @@ export class StatusBarItemAdapter implements vscodeAdapter.StatusBarItem { this.statusBarItem.color = value; } - get command(): string | vscode.Command { + get command(): string | vscode.Command | undefined { return this.statusBarItem.command; } - set command(value: string | vscode.Command) { + set command(value: string | vscode.Command | undefined) { this.statusBarItem.command = value; } - get name(): string { + get name(): string | undefined { return this.statusBarItem.name; } - set name(value: string) { + set name(value: string | undefined) { this.statusBarItem.name = value; } diff --git a/src/textEditorAdapter.ts b/src/textEditorAdapter.ts index 2ff110dc8..a604e2579 100644 --- a/src/textEditorAdapter.ts +++ b/src/textEditorAdapter.ts @@ -8,10 +8,10 @@ import * as vscode from 'vscode'; export class TextEditorAdapter implements vscodeAdapter.TextEditor { - get document(): any { + get document(): vscode.TextDocument { return this.textEditor.document; } constructor(private textEditor: vscode.TextEditor) { } -} \ No newline at end of file +} diff --git a/src/vscodeAdapter.ts b/src/vscodeAdapter.ts index ba8b96deb..1cb0940e0 100644 --- a/src/vscodeAdapter.ts +++ b/src/vscodeAdapter.ts @@ -234,7 +234,7 @@ export interface StatusBarItem { * The priority of this item. Higher value means the item should * be shown more to the left. */ - readonly priority: number; + readonly priority: number | undefined; /** * The text to show for the entry. You can embed icons in the text by leveraging the syntax: @@ -988,4 +988,4 @@ export interface vscode { sessionId: string; openExternal(target: Uri): Thenable; }; -} \ No newline at end of file +} From dfcc3b8b4d40cbcb9cef994c735caa87ec5c4fa1 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:30:05 -0700 Subject: [PATCH 02/16] Rewrite platform.ts to avoid nullability issues --- src/platform.ts | 115 ++++++++++++++++-------------------------------- 1 file changed, 37 insertions(+), 78 deletions(-) diff --git a/src/platform.ts b/src/platform.ts index 59eae240f..0daeb63e6 100644 --- a/src/platform.ts +++ b/src/platform.ts @@ -19,8 +19,7 @@ const unknown = 'unknown'; export class LinuxDistribution { public constructor( public name: string, - public version: string, - public idLike?: string[]) { } + public version: string) { } public static async GetCurrent(): Promise { // Try /etc/os-release and fallback to /usr/lib/os-release per the synopsis @@ -78,7 +77,6 @@ export class LinuxDistribution { public static FromReleaseInfo(releaseInfo: string, eol: string = os.EOL): LinuxDistribution { let name = unknown; let version = unknown; - let idLike: string[] = null; const lines = releaseInfo.split(eol); for (let line of lines) { @@ -100,25 +98,22 @@ export class LinuxDistribution { else if (key === 'VERSION_ID') { version = value; } - else if (key === 'ID_LIKE') { - idLike = value.split(" "); - } - if (name !== unknown && version !== unknown && idLike !== null) { + if (name !== unknown && version !== unknown) { break; } } } - return new LinuxDistribution(name, version, idLike); + return new LinuxDistribution(name, version); } } export class PlatformInformation { public constructor( - public platform: string, + public platform: NodeJS.Platform, public architecture: string, - public distribution: LinuxDistribution = null) { + public distribution?: LinuxDistribution) { } public isWindows(): boolean { @@ -134,87 +129,51 @@ export class PlatformInformation { } public toString(): string { - let result = this.platform; - - if (this.architecture) { - if (result) { - result += ', '; - } - - result += this.architecture; - } - - if (this.distribution) { - if (result) { - result += ', '; - } - - result += this.distribution.toString(); + let result = `${this.platform}, ${this.architecture}`; + if (this.distribution !== undefined) { + result += `, ${this.distribution.toString()}`; } return result; } public static async GetCurrent(): Promise { - let platform = os.platform(); - let architecturePromise: Promise; - let distributionPromise: Promise; - - switch (platform) { - case 'win32': - architecturePromise = PlatformInformation.GetWindowsArchitecture(); - distributionPromise = Promise.resolve(null); - break; - - case 'darwin': - architecturePromise = PlatformInformation.GetUnixArchitecture(); - distributionPromise = Promise.resolve(null); - break; - - case 'linux': - architecturePromise = PlatformInformation.GetUnixArchitecture(); - distributionPromise = LinuxDistribution.GetCurrent(); - break; - - default: - throw new Error(`Unsupported platform: ${platform}`); + const platform = os.platform(); + if (platform === 'win32') { + return new PlatformInformation(platform, PlatformInformation.GetWindowsArchitecture()); + } + else if (platform === 'darwin') { + return new PlatformInformation(platform, await PlatformInformation.GetUnixArchitecture()); + } + else if (platform === 'linux') { + const [architecture, distribution] = await Promise.all([ + PlatformInformation.GetUnixArchitecture(), + LinuxDistribution.GetCurrent() + ]); + return new PlatformInformation(platform, architecture, distribution); } - const platformData: [string, LinuxDistribution] = await Promise.all([architecturePromise, distributionPromise]); - - return new PlatformInformation(platform, platformData[0], platformData[1]); + throw new Error(`Unsupported platform: ${platform}`); } - private static async GetWindowsArchitecture(): Promise { - return new Promise((resolve, reject) => { - if (process.env.PROCESSOR_ARCHITECTURE === 'x86' && process.env.PROCESSOR_ARCHITEW6432 === undefined) { - resolve('x86'); - } - else if (process.env.PROCESSOR_ARCHITECTURE === 'ARM64' && process.env.PROCESSOR_ARCHITEW6432 === undefined) { - resolve('arm64'); - } - else { - resolve('x86_64'); - } - }); + private static GetWindowsArchitecture(): string { + if (process.env.PROCESSOR_ARCHITECTURE === 'x86' && process.env.PROCESSOR_ARCHITEW6432 === undefined) { + return 'x86'; + } + else if (process.env.PROCESSOR_ARCHITECTURE === 'ARM64' && process.env.PROCESSOR_ARCHITEW6432 === undefined) { + return 'arm64'; + } + else { + return 'x86_64'; + } } private static async GetUnixArchitecture(): Promise { - return util.execChildProcess('uname -m') - .then(architecture => { - if (architecture) { - architecture = architecture.trim(); - - switch (architecture) { - case "aarch64": - return "arm64"; - default: - return architecture; - } - } - - return null; - }); + const architecture = await util.execChildProcess('uname -m'); + if (architecture === "aarch64") { + return "arm64"; + } + return architecture; } public isValidPlatformForMono(): boolean { From 05bebccf86741359095e1f1bad2cd321aed593c1 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:34:27 -0700 Subject: [PATCH 03/16] Always pass in a string for proxy --- src/NetworkSettings.ts | 4 ++-- src/packageManager/proxy.ts | 2 +- src/tools/UpdatePackageDependencies.ts | 4 ++-- tasks/offlinePackagingTasks.ts | 4 ++-- test/unitTests/OmnisharpDownloader.test.ts | 2 +- test/unitTests/OmnisharpManager.test.ts | 2 +- test/unitTests/Packages/FileDownloader.test.ts | 4 +--- test/unitTests/Packages/downloadAndInstallPackages.test.ts | 2 +- 8 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/NetworkSettings.ts b/src/NetworkSettings.ts index 1ae77fe0b..16e3a49b5 100644 --- a/src/NetworkSettings.ts +++ b/src/NetworkSettings.ts @@ -17,8 +17,8 @@ export interface NetworkSettingsProvider { export function vscodeNetworkSettingsProvider(vscode: vscode): NetworkSettingsProvider { return () => { const config = vscode.workspace.getConfiguration(); - const proxy = config.get('http.proxy'); + const proxy = config.get('http.proxy', ''); const strictSSL = config.get('http.proxyStrictSSL', true); return new NetworkSettings(proxy, strictSSL); }; -} \ No newline at end of file +} diff --git a/src/packageManager/proxy.ts b/src/packageManager/proxy.ts index a33f03aea..0cf45c886 100644 --- a/src/packageManager/proxy.ts +++ b/src/packageManager/proxy.ts @@ -20,7 +20,7 @@ function getSystemProxyURL(requestURL: Url): string { } export function getProxyAgent(requestURL: Url, proxy: string, strictSSL: boolean): Agent { - const proxyURL = proxy || getSystemProxyURL(requestURL); + const proxyURL = proxy.length > 0 ? proxy : getSystemProxyURL(requestURL); if (!proxyURL) { return null; diff --git a/src/tools/UpdatePackageDependencies.ts b/src/tools/UpdatePackageDependencies.ts index ecb5f665b..7d0d893fd 100644 --- a/src/tools/UpdatePackageDependencies.ts +++ b/src/tools/UpdatePackageDependencies.ts @@ -61,7 +61,7 @@ export async function updatePackageDependencies(): Promise { break; } }); - const networkSettingsProvider: NetworkSettingsProvider = () => new NetworkSettings(/*proxy:*/ null, /*stringSSL:*/ true); + const networkSettingsProvider: NetworkSettingsProvider = () => new NetworkSettings(/*proxy:*/ '', /*stringSSL:*/ true); const downloadAndGetHash = async (url: string): Promise => { console.log(`Downloading from '${url}'`); @@ -253,4 +253,4 @@ function getLowercaseFileNameFromUrl(url: string): string { let versionIndex = fileName.indexOf(versions[0]); //remove the dash before the version number return fileName.substr(0, versionIndex - 1).toLowerCase(); -} \ No newline at end of file +} diff --git a/tasks/offlinePackagingTasks.ts b/tasks/offlinePackagingTasks.ts index 147e361a6..757aca4b3 100644 --- a/tasks/offlinePackagingTasks.ts +++ b/tasks/offlinePackagingTasks.ts @@ -100,7 +100,7 @@ async function install(platformInfo: PlatformInformation, packageJSON: any, isFr let runTimeDependencies = getRuntimeDependenciesPackages(packageJSON) .filter(dep => dep.isFramework === undefined || dep.isFramework === isFramework); let packagesToInstall = await getAbsolutePathPackagesToInstall(runTimeDependencies, platformInfo, codeExtensionPath); - let provider = () => new NetworkSettings(undefined, undefined); + let provider = () => new NetworkSettings('', undefined); if (!(await downloadAndInstallPackages(packagesToInstall, provider, eventStream, isValidDownload, isFramework))) { throw Error("Failed to download package."); } @@ -150,4 +150,4 @@ async function createPackageAsync(packageName: string, outputFolder: string, vsc throw new Error(`vsce failed to create: '${packagePath}'`); } } -} \ No newline at end of file +} diff --git a/test/unitTests/OmnisharpDownloader.test.ts b/test/unitTests/OmnisharpDownloader.test.ts index 2a0c46dba..454fd9d84 100644 --- a/test/unitTests/OmnisharpDownloader.test.ts +++ b/test/unitTests/OmnisharpDownloader.test.ts @@ -21,7 +21,7 @@ import { modernNetVersion } from "../../src/omnisharp/OmnisharpPackageCreator"; [true, false].forEach(useFramework => { suite(`OmnisharpDownloader (useFramework: ${useFramework})`, () => { - const networkSettingsProvider = () => new NetworkSettings(undefined, false); + const networkSettingsProvider = () => new NetworkSettings('', false); let eventStream: EventStream; const installPath = "somePath"; let platformInfo = new PlatformInformation("win32", "x86"); diff --git a/test/unitTests/OmnisharpManager.test.ts b/test/unitTests/OmnisharpManager.test.ts index 2723f4fa5..e6a918970 100644 --- a/test/unitTests/OmnisharpManager.test.ts +++ b/test/unitTests/OmnisharpManager.test.ts @@ -211,6 +211,6 @@ suite(OmnisharpManager.name, () => { }); function GetTestOmniSharpManager(platformInfo: PlatformInformation, eventStream: EventStream, extensionPath: string): OmnisharpManager { - let downloader = new OmnisharpDownloader(() => new NetworkSettings(undefined, false), eventStream, testPackageJSON, platformInfo, extensionPath); + let downloader = new OmnisharpDownloader(() => new NetworkSettings('', false), eventStream, testPackageJSON, platformInfo, extensionPath); return new OmnisharpManager(downloader, platformInfo); } diff --git a/test/unitTests/Packages/FileDownloader.test.ts b/test/unitTests/Packages/FileDownloader.test.ts index 35c150f1c..35f8a6cfc 100644 --- a/test/unitTests/Packages/FileDownloader.test.ts +++ b/test/unitTests/Packages/FileDownloader.test.ts @@ -20,7 +20,7 @@ suite("FileDownloader", () => { const correctUrlPath = `/resource`; const redirectUrlPath = '/redirectResource'; const errorUrlPath = '/errorResource'; - const networkSettingsProvider = () => new NetworkSettings(undefined, false); + const networkSettingsProvider = () => new NetworkSettings('', false); const eventStream = new EventStream(); let eventBus: TestEventBus; const getPrimaryURLEvents = () => { @@ -124,5 +124,3 @@ suite("FileDownloader", () => { return `${server.baseUrl}${urlPath}`; } }); - - diff --git a/test/unitTests/Packages/downloadAndInstallPackages.test.ts b/test/unitTests/Packages/downloadAndInstallPackages.test.ts index 0eb1447e0..1e9e07d8b 100644 --- a/test/unitTests/Packages/downloadAndInstallPackages.test.ts +++ b/test/unitTests/Packages/downloadAndInstallPackages.test.ts @@ -35,7 +35,7 @@ suite(`${downloadAndInstallPackages.name}`, () => { let downloadValidator: DownloadValidator = () => true; const packageDescription = "Test Package"; - const networkSettingsProvider = () => new NetworkSettings(undefined, false); + const networkSettingsProvider = () => new NetworkSettings('', false); setup(async () => { eventStream = new EventStream(); From b0877cfb30b1507077b88a925ea44553d57fa06c Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:35:27 -0700 Subject: [PATCH 04/16] Make NestedError's err optional --- src/NestedError.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NestedError.ts b/src/NestedError.ts index 26de1e87d..c5ff7e6eb 100644 --- a/src/NestedError.ts +++ b/src/NestedError.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ export class NestedError extends Error { - constructor(public message: string, public err: Error = null) { + constructor(public message: string, public err?: Error) { super(message); } -} \ No newline at end of file +} From 48af514e8be6c8354261341532fab5118fdf2c85 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:40:30 -0700 Subject: [PATCH 05/16] Use context.extension instead of looking it up --- src/main.ts | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/main.ts b/src/main.ts index 0e3d9d5ec..ff81396c1 100644 --- a/src/main.ts +++ b/src/main.ts @@ -46,15 +46,13 @@ import { isValidDownload } from './packageManager/isValidDownload'; import { BackgroundWorkStatusBarObserver } from './observers/BackgroundWorkStatusBarObserver'; import { getDotnetPackApi } from './DotnetPack'; -export async function activate(context: vscode.ExtensionContext): Promise { +export async function activate(context: vscode.ExtensionContext): Promise { - const extensionId = CSharpExtensionId; - const extension = vscode.extensions.getExtension(extensionId); - const extensionVersion = extension.packageJSON.version; - const aiKey = extension.packageJSON.contributes.debuggers[0].aiKey; - const reporter = new TelemetryReporter(extensionId, extensionVersion, aiKey); + const extensionVersion = context.extension.packageJSON.version; + const aiKey = context.extension.packageJSON.contributes.debuggers[0].aiKey; + const reporter = new TelemetryReporter(CSharpExtensionId, extensionVersion, aiKey); - util.setExtensionPath(extension.extensionPath); + util.setExtensionPath(context.extension.extensionPath); const eventStream = new EventStream(); const optionStream = createOptionStream(vscode); @@ -123,28 +121,25 @@ export async function activate(context: vscode.ExtensionContext): Promise"; - let errorMessage: string = `The C# extension for Visual Studio Code (powered by OmniSharp) is incompatible on ${platform} ${architecture}`; - const messageOptions: vscode.MessageOptions = { - }; + let errorMessage: string = `The C# extension for Visual Studio Code (powered by OmniSharp) is incompatible on ${platformInfo.platform} ${platformInfo.architecture}`; // Check to see if VS Code is running remotely - if (extension.extensionKind === vscode.ExtensionKind.Workspace) { + if (context.extension.extensionKind === vscode.ExtensionKind.Workspace) { const setupButton: string = "How to setup Remote Debugging"; errorMessage += ` with the VS Code Remote Extensions. To see avaliable workarounds, click on '${setupButton}'.`; - await vscode.window.showErrorMessage(errorMessage, messageOptions, setupButton).then((selectedItem: string) => { + await vscode.window.showErrorMessage(errorMessage, setupButton).then(selectedItem => { if (selectedItem === setupButton) { - let remoteDebugInfoURL = 'https://github.com/OmniSharp/omnisharp-vscode/wiki/Remote-Debugging-On-Linux-Arm'; + const remoteDebugInfoURL = 'https://github.com/OmniSharp/omnisharp-vscode/wiki/Remote-Debugging-On-Linux-Arm'; vscode.env.openExternal(vscode.Uri.parse(remoteDebugInfoURL)); } }); } else { - await vscode.window.showErrorMessage(errorMessage, messageOptions); + await vscode.window.showErrorMessage(errorMessage); } // Unsupported platform @@ -160,10 +155,10 @@ export async function activate(context: vscode.ExtensionContext): Promise downloadAndInstallPackages(dependencies, networkSettingsProvider, eventStream, isValidDownload, useFramework); - let runtimeDependenciesExist = await ensureRuntimeDependencies(extension, eventStream, platformInfo, installDependencies, useFramework); + let runtimeDependenciesExist = await ensureRuntimeDependencies(context.extension, eventStream, platformInfo, installDependencies, useFramework); // activate language services - let langServicePromise = OmniSharp.activate(context, extension.packageJSON, platformInfo, networkSettingsProvider, eventStream, optionProvider, extension.extensionPath); + let langServicePromise = OmniSharp.activate(context, context.extension.packageJSON, platformInfo, networkSettingsProvider, eventStream, optionProvider, context.extension.extensionPath); // register JSON completion & hover providers for project.json context.subscriptions.push(addJSONProviders()); @@ -177,7 +172,7 @@ export async function activate(context: vscode.ExtensionContext): Promise Date: Mon, 16 May 2022 20:41:46 -0700 Subject: [PATCH 06/16] Mark _prefix as optional --- src/logger.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index f3a46585c..fdddb5a61 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -11,7 +11,7 @@ export function SubscribeToAllLoggers(subscriber: (message: string) => void) { export class Logger { private _writer: (message: string) => void; - private _prefix: string; + private _prefix: string | undefined; private _indentLevel: number = 0; private _indentSize: number = 4; @@ -29,7 +29,7 @@ export class Logger { this.write(indent); } - if (this._prefix) { + if (this._prefix !== undefined) { this.write(`[${this._prefix}] `); } @@ -49,13 +49,11 @@ export class Logger { } } - public append(message?: string): void { - message = message || ""; + public append(message: string = ""): void { this._appendCore(message); } - public appendLine(message?: string): void { - message = message || ""; + public appendLine(message: string = ""): void { this._appendCore(message + '\n'); this._atLineStart = true; } @@ -67,4 +65,4 @@ export class Logger { Subscriber(message); } } -} \ No newline at end of file +} From 69cae0aa248aff497451362933c3114b9cee14de Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:46:13 -0700 Subject: [PATCH 07/16] Use implicit NaNs --- src/json.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/json.ts b/src/json.ts index e30210a7f..38799180b 100644 --- a/src/json.ts +++ b/src/json.ts @@ -81,24 +81,19 @@ function cleanJsonText(text: string) { let index = 0; let length = text.length; - function next(): number | undefined { + function next(): number { const result = peek(); index++; return result; } - function peek(offset: number = 0): number | undefined { - if ((index + offset) < length) { - return text.charCodeAt(index + offset); - } - else { - return undefined; - } + function peek(offset: number = 0): number { + return text.charCodeAt(index + offset); } - function peekPastWhitespace(): number | undefined { + function peekPastWhitespace(): number { let pos = index; - let code = undefined; + let code = NaN; do { code = text.charCodeAt(pos); From bfe6ee675fd6b3b74932393aa4086f4c12c19d71 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:47:36 -0700 Subject: [PATCH 08/16] filteredPackages is non-null --- src/InstallRuntimeDependencies.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/InstallRuntimeDependencies.ts b/src/InstallRuntimeDependencies.ts index 03d4cc900..e96dce7d9 100644 --- a/src/InstallRuntimeDependencies.ts +++ b/src/InstallRuntimeDependencies.ts @@ -16,7 +16,7 @@ export async function installRuntimeDependencies(packageJSON: any, extensionPath const packagesToInstall = await getAbsolutePathPackagesToInstall(runTimeDependencies, platformInfo, extensionPath); const filteredPackages = filterOmniSharpPackage(packagesToInstall, useFramework); - if (filteredPackages && filteredPackages.length > 0) { + if (filteredPackages.length > 0) { eventStream.post(new PackageInstallation("C# dependencies")); // Display platform information and RID eventStream.post(new LogPlatformInfo(platformInfo)); @@ -36,5 +36,5 @@ export async function installRuntimeDependencies(packageJSON: any, extensionPath function filterOmniSharpPackage(packages: AbsolutePathPackage[], useFramework: boolean) { // Since we will have more than one OmniSharp package defined for some platforms, we need // to filter out the one that doesn't match which dotnet runtime is being used. - return packages.filter(pkg => pkg.id != "OmniSharp" || pkg.isFramework === useFramework); + return packages.filter(pkg => pkg.id !== "OmniSharp" || pkg.isFramework === useFramework); } From 7a3a589b3c71861ca6c243763aeb41ca06c90694 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:49:39 -0700 Subject: [PATCH 09/16] Mark getDotnetPackApi as nullable --- src/DotnetPack.ts | 8 ++++---- src/main.ts | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/DotnetPack.ts b/src/DotnetPack.ts index 088a32cd4..2e6447e00 100644 --- a/src/DotnetPack.ts +++ b/src/DotnetPack.ts @@ -10,10 +10,10 @@ export interface DotnetPackExtensionExports { getDotnetPath(version?: string): Promise; } -export async function getDotnetPackApi(): Promise { +export async function getDotnetPackApi(): Promise { const dotnetExtension = vscode.extensions.getExtension(dotnetPackExtensionId); - if (!dotnetExtension) { - return null; + if (dotnetExtension === undefined) { + return undefined; } if (!dotnetExtension.isActive) { @@ -21,4 +21,4 @@ export async function getDotnetPackApi(): Promise { } return dotnetExtension.exports; -} \ No newline at end of file +} diff --git a/src/main.ts b/src/main.ts index ff81396c1..2743f5ffa 100644 --- a/src/main.ts +++ b/src/main.ts @@ -227,10 +227,9 @@ async function ensureRuntimeDependencies(extension: vscode.Extension { const dotnetPackApi = await getDotnetPackApi(); - if (!dotnetPackApi) { - return null; + if (dotnetPackApi !== undefined) { + await dotnetPackApi.getDotnetPath(); } - return await dotnetPackApi.getDotnetPath(); } From 6e417dde9074554bd13d30dad1ecee8d40c0844e Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 20:57:05 -0700 Subject: [PATCH 10/16] Nullability fixes for configurationProvider --- src/configurationProvider.ts | 70 ++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/src/configurationProvider.ts b/src/configurationProvider.ts index 6cb08e41c..54252c659 100644 --- a/src/configurationProvider.ts +++ b/src/configurationProvider.ts @@ -32,28 +32,25 @@ export class CSharpConfigurationProvider implements vscode.DebugConfigurationPro */ private async checkWorkspaceInformationMatchesWorkspaceFolder(folder: vscode.WorkspaceFolder): Promise { - const solutionPathOrFolder: string = this.server.getSolutionPathOrFolder(); + const solutionPathOrFolder = this.server.getSolutionPathOrFolder(); // Make sure folder, folder.uri, and solutionPathOrFolder are defined. - if (!solutionPathOrFolder) { + if (solutionPathOrFolder === undefined) { return Promise.resolve(false); } - let serverFolder = solutionPathOrFolder; // If its a .sln or .slnf file, get the folder of the solution. - return fs.lstat(solutionPathOrFolder).then(stat => { - return stat.isFile(); - }).then(isFile => { - if (isFile) { - serverFolder = path.dirname(solutionPathOrFolder); - } + let serverFolder = solutionPathOrFolder; + const isFile = (await fs.lstat(solutionPathOrFolder)).isFile(); + if (isFile) { + serverFolder = path.dirname(solutionPathOrFolder); + } - // Get absolute paths of current folder and server folder. - const currentFolder = path.resolve(folder.uri.fsPath); - serverFolder = path.resolve(serverFolder); + // Get absolute paths of current folder and server folder. + const currentFolder = path.resolve(folder.uri.fsPath); + serverFolder = path.resolve(serverFolder); - return currentFolder && folder.uri && isSubfolderOf(serverFolder, currentFolder); - }); + return isSubfolderOf(serverFolder, currentFolder); } /** @@ -119,26 +116,22 @@ export class CSharpConfigurationProvider implements vscode.DebugConfigurationPro * Parse envFile and add to config.env */ private parseEnvFile(envFile: string, config: vscode.DebugConfiguration): vscode.DebugConfiguration { - if (envFile) { - try { - const parsedFile: ParsedEnvironmentFile = ParsedEnvironmentFile.CreateFromFile(envFile, config["env"]); - - // show error message if single lines cannot get parsed - if (parsedFile.Warning) { - CSharpConfigurationProvider.showFileWarningAsync(parsedFile.Warning, envFile); - } + try { + const parsedFile = ParsedEnvironmentFile.CreateFromFile(envFile, config["env"]); - config.env = parsedFile.Env; - } - catch (e) { - throw new Error(`Can't parse envFile ${envFile} because of ${e}`); + // show error message if single lines cannot get parsed + if (parsedFile.Warning) { + CSharpConfigurationProvider.showFileWarningAsync(parsedFile.Warning, envFile); } + + config.env = parsedFile.Env; + } + catch (e) { + throw new Error(`Can't parse envFile ${envFile} because of ${e}`); } // remove envFile from config after parsing - if (config.envFile) { - delete config.envFile; - } + delete config.envFile; return config; } @@ -155,14 +148,13 @@ export class CSharpConfigurationProvider implements vscode.DebugConfigurationPro if (config.request === "launch") { if (!config.cwd && !config.pipeTransport) { - config.cwd = folder?.uri?.fsPath; // Workspace folder - } - if (!config.internalConsoleOptions) { - config.internalConsoleOptions = "openOnSessionStart"; + config.cwd = folder?.uri.fsPath; // Workspace folder } + config.internalConsoleOptions ??= "openOnSessionStart"; + // read from envFile and set config.env - if (config.envFile) { + if (config.envFile !== undefined && config.envFile.length > 0) { config = this.parseEnvFile(config.envFile, config); } } @@ -172,12 +164,10 @@ export class CSharpConfigurationProvider implements vscode.DebugConfigurationPro private static async showFileWarningAsync(message: string, fileName: string) { const openItem: MessageItem = { title: 'Open envFile' }; - let result: MessageItem = await vscode.window.showWarningMessage(message, openItem); - if (result && result.title === openItem.title) { - let doc: vscode.TextDocument = await vscode.workspace.openTextDocument(fileName); - if (doc) { - vscode.window.showTextDocument(doc); - } + const result = await vscode.window.showWarningMessage(message, openItem); + if (result?.title === openItem.title) { + const doc = await vscode.workspace.openTextDocument(fileName); + await vscode.window.showTextDocument(doc); } } } From 92a8053712b140e2b419d93e3cab997607c612a2 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 21:10:16 -0700 Subject: [PATCH 11/16] Clear most warnings in assets.ts --- src/assets.ts | 117 +++++++++++++++++++++----------------------------- 1 file changed, 49 insertions(+), 68 deletions(-) diff --git a/src/assets.ts b/src/assets.ts index caf093bd1..be02561c6 100644 --- a/src/assets.ts +++ b/src/assets.ts @@ -5,7 +5,7 @@ import * as fs from 'fs-extra'; import * as jsonc from 'jsonc-parser'; -import { FormattingOptions, ModificationOptions } from 'jsonc-parser'; +import { FormattingOptions, ModificationOptions } from 'jsonc-parser'; import * as os from 'os'; import * as path from 'path'; import * as protocol from './omnisharp/protocol'; @@ -23,32 +23,19 @@ export class AssetGenerator { public tasksJsonPath: string; public launchJsonPath: string; - private executeableProjects: protocol.MSBuildProject[]; + private executableProjects: protocol.MSBuildProject[]; private startupProject: protocol.MSBuildProject | undefined; - private fallbackBuildProject: protocol.MSBuildProject; + private fallbackBuildProject: protocol.MSBuildProject | undefined; - public constructor(workspaceInfo: protocol.WorkspaceInformationResponse, workspaceFolder: vscode.WorkspaceFolder = undefined) { + public constructor(workspaceInfo: protocol.WorkspaceInformationResponse, workspaceFolder?: vscode.WorkspaceFolder) { if (workspaceFolder) { this.workspaceFolder = workspaceFolder; } else { - let resourcePath: string = undefined; - - if (!resourcePath && workspaceInfo.Cake) { - resourcePath = workspaceInfo.Cake.Path; - } - - if (!resourcePath && workspaceInfo.ScriptCs) { - resourcePath = workspaceInfo.ScriptCs.Path; - } - - if (!resourcePath && workspaceInfo.DotNet && workspaceInfo.DotNet.Projects.length > 0) { - resourcePath = workspaceInfo.DotNet.Projects[0].Path; - } - - if (!resourcePath && workspaceInfo.MsBuild) { - resourcePath = workspaceInfo.MsBuild.SolutionPath; - } + const resourcePath = workspaceInfo.Cake?.Path ?? + workspaceInfo.ScriptCs?.Path ?? + workspaceInfo.DotNet?.Projects?.[0].Path ?? + workspaceInfo.MsBuild?.SolutionPath; this.workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(resourcePath)); } @@ -57,29 +44,22 @@ export class AssetGenerator { this.tasksJsonPath = path.join(this.vscodeFolder, 'tasks.json'); this.launchJsonPath = path.join(this.vscodeFolder, 'launch.json'); - this.startupProject = undefined; - this.fallbackBuildProject = undefined; - - if (workspaceInfo.MsBuild && workspaceInfo.MsBuild.Projects.length > 0) { - this.executeableProjects = protocol.findExecutableMSBuildProjects(workspaceInfo.MsBuild.Projects); - if (this.executeableProjects.length === 0) { + if (workspaceInfo.MsBuild !== undefined && workspaceInfo.MsBuild.Projects.length > 0) { + this.executableProjects = protocol.findExecutableMSBuildProjects(workspaceInfo.MsBuild.Projects); + if (this.executableProjects.length === 0) { this.fallbackBuildProject = workspaceInfo.MsBuild.Projects[0]; } } else { - this.executeableProjects = []; + this.executableProjects = []; } } public hasExecutableProjects(): boolean { - return this.executeableProjects.length > 0; + return this.executableProjects.length > 0; } public isStartupProjectSelected(): boolean { - if (this.startupProject) { - return true; - } else { - return false; - } + return this.startupProject !== undefined; } public async selectStartupProject(selectedIndex?: number): Promise { @@ -87,21 +67,21 @@ export class AssetGenerator { throw new Error("No executable projects"); } - if (this.executeableProjects.length === 1) { - this.startupProject = this.executeableProjects[0]; + if (this.executableProjects.length === 1) { + this.startupProject = this.executableProjects[0]; return true; } else { const mapItemNameToProject: { [key: string]: protocol.MSBuildProject } = {}; const itemNames: string[] = []; - this.executeableProjects.forEach(project => { + this.executableProjects.forEach(project => { const itemName = `${path.basename(project.Path, ".csproj")} (${project.Path})`; itemNames.push(itemName); mapItemNameToProject[itemName] = project; }); - let selectedItem: string; - if (selectedIndex != null) { + let selectedItem: string | undefined; + if (selectedIndex !== undefined) { selectedItem = itemNames[selectedIndex]; } else { @@ -110,7 +90,8 @@ export class AssetGenerator { placeHolder: "Select the project to launch" }); } - if (!selectedItem || !mapItemNameToProject[selectedItem]) { + + if (selectedItem === undefined || mapItemNameToProject[selectedItem] === undefined) { return false; } @@ -121,11 +102,11 @@ export class AssetGenerator { // This method is used by the unit tests instead of selectStartupProject public setStartupProject(index: number): void { - if (index >= this.executeableProjects.length) { + if (index >= this.executableProjects.length) { throw new Error("Invalid project index"); } - this.startupProject = this.executeableProjects[index]; + this.startupProject = this.executableProjects[index]; } public hasWebServerDependency(): boolean { @@ -463,9 +444,9 @@ async function getOperations(generator: AssetGenerator): Promise td.group === 'build'); } @@ -474,18 +455,18 @@ function getBuildTasks(tasksConfiguration: tasks.TaskConfiguration): tasks.TaskD } } - findBuildTask(tasksConfiguration.version, tasksConfiguration.tasks); + findBuildTask(tasksConfiguration.tasks); if (tasksConfiguration.windows) { - findBuildTask(tasksConfiguration.version, tasksConfiguration.windows.tasks); + findBuildTask(tasksConfiguration.windows.tasks); } if (tasksConfiguration.osx) { - findBuildTask(tasksConfiguration.version, tasksConfiguration.osx.tasks); + findBuildTask(tasksConfiguration.osx.tasks); } if (tasksConfiguration.linux) { - findBuildTask(tasksConfiguration.version, tasksConfiguration.linux.tasks); + findBuildTask(tasksConfiguration.linux.tasks); } return result; @@ -583,8 +564,8 @@ function getBuildAssetsNotificationSetting() { return csharpConfig.get('supressBuildAssetsNotification'); } -export function getFormattingOptions(): FormattingOptions { - const editorConfig = vscode.workspace.getConfiguration('editor'); +export function getFormattingOptions(): FormattingOptions { + const editorConfig = vscode.workspace.getConfiguration('editor'); const tabSize = editorConfig.get('tabSize') ?? 4; const insertSpaces = editorConfig.get('insertSpaces') ?? true; @@ -609,20 +590,20 @@ export async function addTasksJsonIfNecessary(generator: AssetGenerator, operati } const formattingOptions = getFormattingOptions(); - + const tasksJson = generator.createTasksConfiguration(); let text: string; if (!fs.pathExistsSync(generator.tasksJsonPath)) { // when tasks.json does not exist create it and write all the content directly const tasksJsonText = JSON.stringify(tasksJson); - const tasksJsonTextFormatted = jsonc.applyEdits(tasksJsonText, jsonc.format(tasksJsonText, null, formattingOptions)); + const tasksJsonTextFormatted = jsonc.applyEdits(tasksJsonText, jsonc.format(tasksJsonText, undefined, formattingOptions)); text = tasksJsonTextFormatted; } else { // when tasks.json exists just update the tasks node - const ourConfigs = tasksJson.tasks; - const content = fs.readFileSync(generator.tasksJsonPath).toString(); + const ourConfigs = tasksJson.tasks ?? []; + const content = fs.readFileSync(generator.tasksJsonPath, { encoding: 'utf8' }); const updatedJson = updateJsonWithComments(content, ourConfigs, 'tasks', 'label', formattingOptions); text = updatedJson; } @@ -658,12 +639,12 @@ async function addLaunchJsonIfNecessary(generator: AssetGenerator, operations: A "configurations": ${configurationsMassaged} }`; - text = jsonc.applyEdits(launchJsonText, jsonc.format(launchJsonText, null, formattingOptions)); - } + text = jsonc.applyEdits(launchJsonText, jsonc.format(launchJsonText, undefined, formattingOptions)); + } else { // when launch.json exists replace or append our configurations - const ourConfigs = jsonc.parse(launchJsonConfigurations); - const content = fs.readFileSync(generator.launchJsonPath).toString(); + const ourConfigs = jsonc.parse(launchJsonConfigurations) ?? []; + const content = fs.readFileSync(generator.launchJsonPath, { encoding: 'utf8' }); const updatedJson = updateJsonWithComments(content, ourConfigs, 'configurations', 'name', formattingOptions); text = updatedJson; } @@ -751,18 +732,18 @@ async function getExistingAssets(generator: AssetGenerator) { assets = assets.concat(tasks); } - if(fs.pathExistsSync(generator.launchJsonPath)) { + if(fs.pathExistsSync(generator.launchJsonPath)) { const content = fs.readFileSync(generator.launchJsonPath).toString(); let configurationNames = [ - ".NET Core Launch (console)", + ".NET Core Launch (console)", ".NET Core Launch (web)", ".NET Core Attach", - "Launch and Debug Standalone Blazor WebAssembly App", + "Launch and Debug Standalone Blazor WebAssembly App", ]; const configurations = jsonc.parse(content)?.configurations?. map((t: { name: string; }) => t.name). filter((n: string) => configurationNames.includes(n)); - + assets = assets.concat(configurations); } @@ -773,7 +754,7 @@ async function getExistingAssets(generator: AssetGenerator) { async function shouldGenerateAssets(generator: AssetGenerator): Promise { return new Promise((resolve, reject) => { getExistingAssets(generator).then(res => { - if (res && res.length) { + if (res.length > 0) { const yesItem = { title: 'Yes' }; const cancelItem = { title: 'Cancel', isCloseAffordance: true }; vscode.window.showWarningMessage('Replace existing build and debug assets?', cancelItem, yesItem) @@ -833,22 +814,22 @@ export function replaceCommentPropertiesWithComments(text: string) { // replacing dummy properties OS-COMMENT with the normal comment syntax let regex = /["']OS-COMMENT\d*["']\s*\:\s*["'](.*)["']\s*?,/gi; let withComments = text.replace(regex, '// $1'); - + return withComments; } -export function updateJsonWithComments(text: string, replacements: any[], nodeName: string, keyName: string, formattingOptions: FormattingOptions) : string { +export function updateJsonWithComments(text: string, replacements: any[], nodeName: string, keyName: string, formattingOptions: FormattingOptions) : string { let modificationOptions : ModificationOptions = { formattingOptions }; - + // parse using jsonc because there are comments // only use this to determine what to change // we will modify it as text to keep existing comments let parsed = jsonc.parse(text); let items = parsed[nodeName]; let itemKeys : string[] = items.map((i: { [x: string]: string; }) => i[keyName]); - + let modified = text; // count how many items we inserted to ensure we are putting items at the end // in the same order as they are in the replacements array @@ -866,6 +847,6 @@ export function updateJsonWithComments(text: string, replacements: any[], nodeNa // changes one by one modified = updated; }); - + return replaceCommentPropertiesWithComments(modified); } From 60ae06afe1f33c4e12f47995d809baae082daac6 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Mon, 16 May 2022 21:18:43 -0700 Subject: [PATCH 12/16] Fix tests --- src/coreclr-debug/util.ts | 10 +---- .../InstallRuntimeDependencies.test.ts | 4 +- .../Packages/PackageFilterer.test.ts | 38 +++++++++---------- .../coreclr-debug/targetArchitecture.test.ts | 10 +---- .../logging/CsharpLoggerObserver.test.ts | 4 +- .../logging/TelemetryObserver.test.ts | 2 +- 6 files changed, 27 insertions(+), 41 deletions(-) diff --git a/src/coreclr-debug/util.ts b/src/coreclr-debug/util.ts index 3d7f49742..4ffc78b48 100644 --- a/src/coreclr-debug/util.ts +++ b/src/coreclr-debug/util.ts @@ -121,13 +121,7 @@ export class CoreClrDebugUtil { const MINIMUM_SUPPORTED_OSX_ARM64_DOTNET_CLI: string = '6.0.0'; -export function getTargetArchitecture(platformInfo: PlatformInformation, launchJsonTargetArchitecture: string, dotnetInfo: DotnetInfo): string -{ - if (!platformInfo) - { - throw new Error(`Unable to retrieve 'TargetArchitecture' without platformInfo.`); - } - +export function getTargetArchitecture(platformInfo: PlatformInformation, launchJsonTargetArchitecture: string, dotnetInfo: DotnetInfo): string { let targetArchitecture = ""; // On Apple M1 Machines, we need to determine if we need to use the 'x86_64' or 'arm64' debugger. @@ -175,4 +169,4 @@ export function getTargetArchitecture(platformInfo: PlatformInformation, launchJ } return targetArchitecture; -} \ No newline at end of file +} diff --git a/test/unitTests/InstallRuntimeDependencies.test.ts b/test/unitTests/InstallRuntimeDependencies.test.ts index 2668875cd..5359eb2e8 100644 --- a/test/unitTests/InstallRuntimeDependencies.test.ts +++ b/test/unitTests/InstallRuntimeDependencies.test.ts @@ -24,7 +24,7 @@ suite(`${installRuntimeDependencies.name}`, () => { let installDependencies: IInstallDependencies; let eventStream: EventStream; let eventBus: TestEventBus; - let platformInfo = new PlatformInformation("platform1", "architecture1"); + let platformInfo = new PlatformInformation("linux", "architecture1"); const useFramework = true; setup(() => { @@ -91,4 +91,4 @@ suite(`${installRuntimeDependencies.name}`, () => { expect(installed).to.be.false; }); }); -}); \ No newline at end of file +}); diff --git a/test/unitTests/Packages/PackageFilterer.test.ts b/test/unitTests/Packages/PackageFilterer.test.ts index 2248f4b11..b49220dc4 100644 --- a/test/unitTests/Packages/PackageFilterer.test.ts +++ b/test/unitTests/Packages/PackageFilterer.test.ts @@ -17,39 +17,39 @@ suite(`${getNotInstalledPackagesForPlatform.name}`, () => { let extensionPath = "/ExtensionPath"; const packages = [ { - "description": "Platfrom1-Architecture1 uninstalled package", - "platforms": ["platform1"], + "description": "linux-Architecture1 uninstalled package", + "platforms": ["linux"], "architectures": ["architecture1"], "installPath": "path1" }, { //already installed package - "description": "Platfrom1-Architecture1 installed package", - "platforms": ["platform1"], + "description": "linux-Architecture1 installed package", + "platforms": ["linux"], "architectures": ["architecture1"], "installPath": "path5" }, { - "description": "Platfrom2-Architecture2 uninstalled package", - "platforms": ["platform2"], + "description": "win32-Architecture2 uninstalled package", + "platforms": ["win32"], "architectures": ["architecture2"], "installPath": "path2" }, { - "description": "Platfrom1-Architecture2 uninstalled package", - "platforms": ["platform1"], + "description": "linux-Architecture2 uninstalled package", + "platforms": ["linux"], "architectures": ["architecture2"], "installPath": "path3" }, { - "description": "Platfrom2-Architecture1 uninstalled package", - "platforms": ["platform2"], + "description": "win32-Architecture1 uninstalled package", + "platforms": ["win32"], "architectures": ["architecture1"], "installPath": "path4" }, { - "description": "Platfrom1-Architecture2 uninstalled package", - "platforms": ["platform1"], + "description": "linux-Architecture2 uninstalled package", + "platforms": ["linux"], "architectures": ["architecture2"], "installPath": "path3" }, @@ -65,24 +65,24 @@ suite(`${getNotInstalledPackagesForPlatform.name}`, () => { }); test('Filters the packages based on Platform Information', async () => { - let platformInfo = new PlatformInformation("platform2", "architecture2"); + let platformInfo = new PlatformInformation("win32", "architecture2"); let filteredPackages = await getNotInstalledPackagesForPlatform(absolutePathPackages, platformInfo); expect(filteredPackages.length).to.be.equal(1); - expect(filteredPackages[0].description).to.be.equal("Platfrom2-Architecture2 uninstalled package"); - expect(filteredPackages[0].platforms[0]).to.be.equal("platform2"); + expect(filteredPackages[0].description).to.be.equal("win32-Architecture2 uninstalled package"); + expect(filteredPackages[0].platforms[0]).to.be.equal("win32"); expect(filteredPackages[0].architectures[0]).to.be.equal("architecture2"); }); test('Returns only the packages where install.Lock is not present', async () => { - let platformInfo = new PlatformInformation("platform1", "architecture1"); + let platformInfo = new PlatformInformation("linux", "architecture1"); let filteredPackages = await getNotInstalledPackagesForPlatform(absolutePathPackages, platformInfo); expect(filteredPackages.length).to.be.equal(1); - expect(filteredPackages[0].description).to.be.equal("Platfrom1-Architecture1 uninstalled package"); - expect(filteredPackages[0].platforms[0]).to.be.equal("platform1"); + expect(filteredPackages[0].description).to.be.equal("linux-Architecture1 uninstalled package"); + expect(filteredPackages[0].platforms[0]).to.be.equal("linux"); expect(filteredPackages[0].architectures[0]).to.be.equal("architecture1"); }); teardown(() => { mock.restore(); }); -}); \ No newline at end of file +}); diff --git a/test/unitTests/coreclr-debug/targetArchitecture.test.ts b/test/unitTests/coreclr-debug/targetArchitecture.test.ts index 969f104ed..0c49ddb8d 100644 --- a/test/unitTests/coreclr-debug/targetArchitecture.test.ts +++ b/test/unitTests/coreclr-debug/targetArchitecture.test.ts @@ -12,17 +12,9 @@ import { DotnetInfo } from '../../../src/utils/getDotnetInfo'; suite("getTargetArchitecture Tests", () => { suiteSetup(() => should()); - suite("Error", () => { - test("Invalid Call", () => { - let fn = function () { getTargetArchitecture(null, null, null); }; - - expect(fn).to.throw("Unable to retrieve 'TargetArchitecture' without platformInfo."); - }); - }); - suite("Windows", () => { test("Windows x86_64", () => { - const platformInfo: PlatformInformation = new PlatformInformation("windows", "x86_64", null); + const platformInfo: PlatformInformation = new PlatformInformation("win32", "x86_64", null); const targetArchitecture = getTargetArchitecture(platformInfo, null, null); diff --git a/test/unitTests/logging/CsharpLoggerObserver.test.ts b/test/unitTests/logging/CsharpLoggerObserver.test.ts index 8a6a1dc50..8951b2cfd 100644 --- a/test/unitTests/logging/CsharpLoggerObserver.test.ts +++ b/test/unitTests/logging/CsharpLoggerObserver.test.ts @@ -24,9 +24,9 @@ suite("CsharpLoggerObserver", () => { }); test('PlatformInfo: Logs contain the Platform and Architecture', () => { - let event = new Event.LogPlatformInfo(new PlatformInformation("MyPlatform", "MyArchitecture")); + let event = new Event.LogPlatformInfo(new PlatformInformation("linux", "MyArchitecture")); observer.post(event); - expect(logOutput).to.contain("MyPlatform"); + expect(logOutput).to.contain("linux"); expect(logOutput).to.contain("MyArchitecture"); }); diff --git a/test/unitTests/logging/TelemetryObserver.test.ts b/test/unitTests/logging/TelemetryObserver.test.ts index 971a22e7f..0e45b5df3 100644 --- a/test/unitTests/logging/TelemetryObserver.test.ts +++ b/test/unitTests/logging/TelemetryObserver.test.ts @@ -15,7 +15,7 @@ chai.use(require('chai-arrays')); suite('TelemetryReporterObserver', () => { suiteSetup(() => should()); - let platformInfo = new PlatformInformation("platform", "architecture"); + let platformInfo = new PlatformInformation("linux", "architecture"); let name = ""; let property: { [key: string]: string } = null; let measure: { [key: string]: number }[] = []; From 4fbfce19e07cbafc8ea4a1119732f6fd579f3466 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sat, 21 May 2022 00:21:47 -0700 Subject: [PATCH 13/16] Clear nullability warnings in assets.ts --- src/assets.ts | 54 +++++++++++++++++++++++--------------- src/omnisharp/extension.ts | 11 ++++---- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/assets.ts b/src/assets.ts index be02561c6..82917ec6d 100644 --- a/src/assets.ts +++ b/src/assets.ts @@ -18,28 +18,15 @@ import { OmniSharpServer } from './omnisharp/server'; import { tolerantParse } from './json'; export class AssetGenerator { - public workspaceFolder: vscode.WorkspaceFolder; public vscodeFolder: string; public tasksJsonPath: string; public launchJsonPath: string; - private executableProjects: protocol.MSBuildProject[]; + private executableProjects: protocol.MSBuildProject[] = []; private startupProject: protocol.MSBuildProject | undefined; private fallbackBuildProject: protocol.MSBuildProject | undefined; - public constructor(workspaceInfo: protocol.WorkspaceInformationResponse, workspaceFolder?: vscode.WorkspaceFolder) { - if (workspaceFolder) { - this.workspaceFolder = workspaceFolder; - } - else { - const resourcePath = workspaceInfo.Cake?.Path ?? - workspaceInfo.ScriptCs?.Path ?? - workspaceInfo.DotNet?.Projects?.[0].Path ?? - workspaceInfo.MsBuild?.SolutionPath; - - this.workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(resourcePath)); - } - + public constructor(workspaceInfo: protocol.WorkspaceInformationResponse, private workspaceFolder: vscode.WorkspaceFolder) { this.vscodeFolder = path.join(this.workspaceFolder.uri.fsPath, '.vscode'); this.tasksJsonPath = path.join(this.vscodeFolder, 'tasks.json'); this.launchJsonPath = path.join(this.vscodeFolder, 'launch.json'); @@ -49,8 +36,6 @@ export class AssetGenerator { if (this.executableProjects.length === 0) { this.fallbackBuildProject = workspaceInfo.MsBuild.Projects[0]; } - } else { - this.executableProjects = []; } } @@ -145,7 +130,14 @@ export class AssetGenerator { const startupProjectDir = path.dirname(this.startupProject.Path); const relativeProjectDir = path.join('${workspaceFolder}', path.relative(this.workspaceFolder.uri.fsPath, startupProjectDir)); const configurationName = 'Debug'; - const targetFramework = protocol.findNetCoreTargetFramework(this.startupProject); + + // We know targetFramework is non-null for the following reasons: + // 1. startupProject is non-null. + // 2. In order for startupProject to be non-null, there must be at least one executable project. + // 3. For a project to be executable, it must either be .NET Core or Blazor WASM standalone. + // 4. Blazor was officially released starting with .NET Core 3.0. + // Therefore, we know that findNetCoreTargetFramework will always return a framework. + const targetFramework = protocol.findNetCoreTargetFramework(this.startupProject)!; const result = path.join(relativeProjectDir, `bin/${configurationName}/${targetFramework.ShortName}/${this.startupProject.AssemblyName}.dll`); return result; } @@ -690,7 +682,21 @@ export async function addAssetsIfNecessary(server: OmniSharpServer): Promise { - const generator = new AssetGenerator(info); + const resourcePath = info.Cake?.Path ?? + info.ScriptCs?.Path ?? + info.DotNet?.Projects?.[0].Path ?? + info.MsBuild?.SolutionPath; + if (resourcePath === undefined) { + // This shouldn't happen, but it's a cheap check. + return resolve(AddAssetResult.NotApplicable); + } + + const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(resourcePath)); + if (workspaceFolder === undefined) { + return resolve(AddAssetResult.NotApplicable); + } + + const generator = new AssetGenerator(info, workspaceFolder); // If there aren't executable projects, we will not prompt if (generator.hasExecutableProjects()) { return getOperations(generator).then(operations => { @@ -698,7 +704,7 @@ export async function addAssetsIfNecessary(server: OmniSharpServer): Promise { + promptToAddAssets(workspaceFolder).then(result => { if (result === PromptResult.Disable) { return resolve(AddAssetResult.Disable); } @@ -781,7 +787,13 @@ export async function generateAssets(server: OmniSharpServer, selectedIndex?: nu try { let workspaceInformation = await serverUtils.requestWorkspaceInformation(server); if (workspaceInformation.MsBuild && workspaceInformation.MsBuild.Projects.length > 0) { - const generator = new AssetGenerator(workspaceInformation); + const resourcePath = workspaceInformation.MsBuild.SolutionPath; + const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(resourcePath)); + if (workspaceFolder === undefined) { + return; + } + + const generator = new AssetGenerator(workspaceInformation, workspaceFolder); let doGenerateAssets = await shouldGenerateAssets(generator); if (!doGenerateAssets) { return; // user cancelled diff --git a/src/omnisharp/extension.ts b/src/omnisharp/extension.ts index 2d7885fe9..9e08ce93d 100644 --- a/src/omnisharp/extension.ts +++ b/src/omnisharp/extension.ts @@ -136,13 +136,12 @@ export async function activate(context: vscode.ExtensionContext, packageJSON: an disposables.add(registerCommands(context, server, platformInfo, eventStream, optionProvider, omnisharpMonoResolver, packageJSON, extensionPath)); if (!context.workspaceState.get('assetPromptDisabled')) { - disposables.add(server.onServerStart(() => { + disposables.add(server.onServerStart(async () => { // Update or add tasks.json and launch.json - addAssetsIfNecessary(server).then(result => { - if (result === AddAssetResult.Disable) { - context.workspaceState.update('assetPromptDisabled', true); - } - }); + const result = await addAssetsIfNecessary(server); + if (result === AddAssetResult.Disable) { + context.workspaceState.update('assetPromptDisabled', true); + } })); } From 503bde863cd645317b60f660a09f776a5ba08d8e Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sun, 22 May 2022 21:54:37 -0700 Subject: [PATCH 14/16] WIP --- src/assets.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/assets.ts b/src/assets.ts index 82917ec6d..4450a78ec 100644 --- a/src/assets.ts +++ b/src/assets.ts @@ -135,9 +135,13 @@ export class AssetGenerator { // 1. startupProject is non-null. // 2. In order for startupProject to be non-null, there must be at least one executable project. // 3. For a project to be executable, it must either be .NET Core or Blazor WASM standalone. - // 4. Blazor was officially released starting with .NET Core 3.0. + // 4. This code path is not called if the project is Blazor WASM standalone. // Therefore, we know that findNetCoreTargetFramework will always return a framework. const targetFramework = protocol.findNetCoreTargetFramework(this.startupProject)!; + + // TODO: Could we rewrite this to use this.startupPath.OutputPath, so we don't need a nullability assertion above? + // const relativeOutputDir = path.relative(this.workspaceFolder.uri.fsPath, this.startupProject.OutputPath); + // const result = path.join('${workspaceFolder}', relativeOutputDir, `${this.startupProject.AssemblyName}.dll`); const result = path.join(relativeProjectDir, `bin/${configurationName}/${targetFramework.ShortName}/${this.startupProject.AssemblyName}.dll`); return result; } From 96c684a7e422d44829cee6a0da6f2212867df4c9 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Thu, 26 May 2022 19:13:28 -0700 Subject: [PATCH 15/16] Refactor selectStartupProject a bit --- src/assets.ts | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/assets.ts b/src/assets.ts index 50d0c5aed..da928d344 100644 --- a/src/assets.ts +++ b/src/assets.ts @@ -52,35 +52,30 @@ export class AssetGenerator { throw new Error("No executable projects"); } + if (selectedIndex !== undefined) { + this.startupProject = this.executableProjects[selectedIndex]; + return true; + } + if (this.executableProjects.length === 1) { this.startupProject = this.executableProjects[0]; return true; } else { - const mapItemNameToProject: { [key: string]: protocol.MSBuildProject } = {}; - const itemNames: string[] = []; - - this.executableProjects.forEach(project => { - const itemName = `${path.basename(project.Path, ".csproj")} (${project.Path})`; - itemNames.push(itemName); - mapItemNameToProject[itemName] = project; + const items = this.executableProjects.map(project => ({ + label: `${path.basename(project.Path, ".csproj")} (${project.Path})`, + project, + })); + + const selectedItem = await vscode.window.showQuickPick(items, { + matchOnDescription: true, + placeHolder: "Select the project to launch" }); - let selectedItem: string | undefined; - if (selectedIndex !== undefined) { - selectedItem = itemNames[selectedIndex]; - } - else { - selectedItem = await vscode.window.showQuickPick(itemNames, { - matchOnDescription: true, - placeHolder: "Select the project to launch" - }); - } - - if (selectedItem === undefined || mapItemNameToProject[selectedItem] === undefined) { + if (selectedItem === undefined) { return false; } - this.startupProject = mapItemNameToProject[selectedItem]; + this.startupProject = selectedItem.project; return true; } } From 461618045f947020f795f625a51782b5b7e7cf20 Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sat, 28 May 2022 22:49:38 -0700 Subject: [PATCH 16/16] Emulate is_musl_based_distro --- src/platform.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform.ts b/src/platform.ts index 64ab75c0e..c6f6b315f 100644 --- a/src/platform.ts +++ b/src/platform.ts @@ -177,12 +177,13 @@ export class PlatformInformation { return architecture; } + // Emulates https://github.com/dotnet/install-scripts/blob/3c6cc06/src/dotnet-install.sh#L187-L189. private static async GetIsMusl(): Promise { try { const output = await util.execChildProcess('ldd --version'); return output.includes('musl'); } catch (err) { - return false; + return err instanceof Error ? err.message.includes('musl') : false; } }