Merge pull request #5255 from 50Wliu/users/winstonliu/misc-nullability

Package manager nullability fixes

Author: JoeRobich

package-lock.json

@@ -91,12 +91,12 @@
     "@types/gulp": "4.0.5",
     "@types/minimist": "1.2.1",
     "@types/mocha": "5.2.5",
-    "@types/node": "10.12.24",
+    "@types/node": "16.11.38",
     "@types/semver": "5.5.0",
     "@types/tmp": "0.0.33",
     "@types/unzipper": "^0.9.1",
     "@types/vscode": "1.66.0",
-    "@types/yauzl": "2.9.1",
+    "@types/yauzl": "2.10.0",
     "@vscode/test-electron": "2.1.3",
     "archiver": "5.3.0",
     "chai": "4.3.4",
@@ -230,6 +230,9 @@
       "platforms": [
         "darwin"
       ],
+      "architectures": [
+        "x86_64"
+      ],
       "binaries": [
         "./mono.osx",
         "./run"

package.json

@@ -33,10 +33,6 @@ export function getUnixTempDirectory(){
     return envTmp;
 }
 
-export function isBoolean(obj: any): obj is boolean {
-    return obj === true || obj === false;
-}
-
 export function sum<T>(arr: T[], selector: (item: T) => number): number {
     return arr.reduce((prev, curr) => prev + selector(curr), 0);
 }

src/common.ts

@@ -48,7 +48,7 @@ async function checkIsValidArchitecture(platformInformation: PlatformInformation
             }
 
             // Validate we are on compatiable macOS version if we are x86_64
-            if ((platformInformation.architecture !== "x86_64") || 
+            if ((platformInformation.architecture !== "x86_64") ||
                 (platformInformation.architecture === "x86_64" && !CoreClrDebugUtil.isMacOSSupported())) {
                 eventStream.post(new DebuggerPrerequisiteFailure("[ERROR] The debugger cannot be installed. The debugger requires macOS 10.12 (Sierra) or newer."));
                 return false;
@@ -149,8 +149,8 @@ export class DebugAdapterExecutableFactory implements vscode.DebugAdapterDescrip
 
             // install.Lock does not exist, need to wait for packages to finish downloading.
             let installLock = false;
-            let debuggerPackage = getRuntimeDependencyPackageWithId("Debugger", this.packageJSON, this.platformInfo, this.extensionPath);
-            if (debuggerPackage && debuggerPackage.installPath) {
+            const debuggerPackage = getRuntimeDependencyPackageWithId("Debugger", this.packageJSON, this.platformInfo, this.extensionPath);
+            if (debuggerPackage?.installPath) {
                 installLock = await common.installFileExists(debuggerPackage.installPath, common.InstallFileType.Lock);
             }
 

src/coreclr-debug/activate.ts

@@ -82,10 +82,7 @@ export class TelemetryObserver {
         if (event.error instanceof PackageError) {
             // we can log the message in a PackageError to telemetry as we do not put PII in PackageError messages
             telemetryProps['error.message'] = event.error.message;
-
-            if (event.error.pkg) {
-                telemetryProps['error.packageUrl'] = event.error.pkg.url;
-            }
+            telemetryProps['error.packageUrl'] = event.error.pkg.url;
         }
 
         this.reporter.sendTelemetryEvent('AcquisitionFailed', telemetryProps);

src/observers/TelemetryObserver.ts

@@ -28,7 +28,7 @@ export class OmnisharpDownloader {
         let runtimeDependencies = getRuntimeDependenciesPackages(this.packageJSON);
         let omniSharpPackages = GetPackagesFromVersion(version, useFramework, runtimeDependencies, serverUrl, installPath);
         let packagesToInstall = await getAbsolutePathPackagesToInstall(omniSharpPackages, this.platformInfo, this.extensionPath);
-        if (packagesToInstall && packagesToInstall.length > 0) {
+        if (packagesToInstall.length > 0) {
             this.eventStream.post(new PackageInstallation(`OmniSharp Version = ${version}`));
             this.eventStream.post(new LogPlatformInfo(this.platformInfo));
             if (await downloadAndInstallPackages(packagesToInstall, this.networkSettingsProvider, this.eventStream, isValidDownload, useFramework)) {

src/omnisharp/OmnisharpDownloader.ts

@@ -8,18 +8,11 @@ import { Package } from "../packageManager/Package";
 export const modernNetVersion = "6.0";
 
 export function GetPackagesFromVersion(version: string, useFramework: boolean, runTimeDependencies: Package[], serverUrl: string, installPath: string): Package[] {
-    if (!version) {
-        throw new Error('Invalid version');
-    }
-
-    let versionPackages = new Array<Package>();
-    for (let inputPackage of runTimeDependencies) {
-        if (inputPackage.platformId && inputPackage.isFramework === useFramework) {
-            versionPackages.push(SetBinaryAndGetPackage(inputPackage, useFramework, serverUrl, version, installPath));
-        }
-    }
-
-    return versionPackages;
+    return runTimeDependencies
+        .filter(inputPackage =>
+            inputPackage.platformId !== undefined && inputPackage.isFramework === useFramework)
+        .map(inputPackage =>
+            SetBinaryAndGetPackage(inputPackage, useFramework, serverUrl, version, installPath));
 }
 
 export function SetBinaryAndGetPackage(inputPackage: Package, useFramework: boolean, serverUrl: string, version: string, installPath: string): Package {
@@ -28,7 +21,7 @@ export function SetBinaryAndGetPackage(inputPackage: Package, useFramework: bool
         // .NET 6 packages use system `dotnet OmniSharp.dll`
         installBinary = 'OmniSharp.dll';
     }
-    else if (inputPackage.platformId === 'win-x86' || inputPackage.platformId === 'win-x64' || inputPackage.platformId === 'win-arm64') {
+    else if (inputPackage.platforms.includes('win32')) {
         installBinary = 'OmniSharp.exe';
     }
     else {
@@ -39,25 +32,13 @@ export function SetBinaryAndGetPackage(inputPackage: Package, useFramework: bool
 }
 
 function GetPackage(inputPackage: Package, useFramework: boolean, serverUrl: string, version: string, installPath: string, installBinary: string): Package {
-    if (!version) {
-        throw new Error('Invalid version');
-    }
-
     const packageSuffix = useFramework ? '' : `-net${modernNetVersion}`;
 
-    let versionPackage: Package = {
-        id: inputPackage.id,
+    return {
+        ...inputPackage,
         description: `${inputPackage.description}, Version = ${version}`,
         url: `${serverUrl}/releases/${version}/omnisharp-${inputPackage.platformId}${packageSuffix}.zip`,
         installPath: `${installPath}/${version}${packageSuffix}`,
         installTestPath: `./${installPath}/${version}${packageSuffix}/${installBinary}`,
-        platforms: inputPackage.platforms,
-        architectures: inputPackage.architectures,
-        binaries: inputPackage.binaries,
-        platformId: inputPackage.platformId,
-        isFramework: useFramework
     };
-
-    return versionPackage;
 }
-

src/omnisharp/OmnisharpPackageCreator.ts

@@ -3,16 +3,16 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { isAbsolute, resolve } from "path";
+import * as path from "path";
 
 export class AbsolutePath {
     constructor(public value: string) {
-        if (!isAbsolute(value)) {
+        if (!path.isAbsolute(value)) {
             throw new Error("The path must be absolute");
         }
     }
 
     public static getAbsolutePath(...pathSegments: string[]): AbsolutePath {
-        return new AbsolutePath(resolve(...pathSegments));
+        return new AbsolutePath(path.resolve(...pathSegments));
     }
-}
+}

src/packageManager/AbsolutePath.ts

@@ -13,8 +13,8 @@ export class AbsolutePathPackage implements IPackage {
         public url: string,
         public platforms: string[],
         public architectures: string[],
-        public binaries: AbsolutePath[],
-        public installPath?: AbsolutePath,
+        public installPath: AbsolutePath,
+        public binaries?: AbsolutePath[],
         public installTestPath?: AbsolutePath,
         public fallbackUrl?: string,
         public platformId?: string,
@@ -29,8 +29,8 @@ export class AbsolutePathPackage implements IPackage {
             pkg.url,
             pkg.platforms,
             pkg.architectures,
-            getAbsoluteBinaries(pkg, extensionPath),
             getAbsoluteInstallPath(pkg, extensionPath),
+            getAbsoluteBinaries(pkg, extensionPath),
             getAbsoluteInstallTestPath(pkg, extensionPath),
             pkg.fallbackUrl,
             pkg.platformId,
@@ -40,27 +40,23 @@ export class AbsolutePathPackage implements IPackage {
     }
 }
 
-function getAbsoluteInstallTestPath(pkg: Package, extensionPath: string): AbsolutePath {
-    if (pkg.installTestPath) {
+function getAbsoluteInstallTestPath(pkg: Package, extensionPath: string): AbsolutePath | undefined {
+    if (pkg.installTestPath !== undefined) {
         return AbsolutePath.getAbsolutePath(extensionPath, pkg.installTestPath);
     }
 
-    return null;
+    return undefined;
 }
 
-function getAbsoluteBinaries(pkg: Package, extensionPath: string): AbsolutePath[] {
-    let basePath = getAbsoluteInstallPath(pkg, extensionPath).value;
-    if (pkg.binaries) {
-        return pkg.binaries.map(value => AbsolutePath.getAbsolutePath(basePath, value));
-    }
-
-    return null;
+function getAbsoluteBinaries(pkg: Package, extensionPath: string): AbsolutePath[] | undefined {
+    const basePath = getAbsoluteInstallPath(pkg, extensionPath).value;
+    return pkg.binaries?.map(value => AbsolutePath.getAbsolutePath(basePath, value));
 }
 
 function getAbsoluteInstallPath(pkg: Package, extensionPath: string): AbsolutePath {
-    if (pkg.installPath) {
+    if (pkg.installPath !== undefined) {
         return AbsolutePath.getAbsolutePath(extensionPath, pkg.installPath);
     }
 
     return new AbsolutePath(extensionPath);
-}
+}

src/packageManager/AbsolutePathPackage.ts

@@ -4,7 +4,6 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as https from 'https';
-import * as util from '../common';
 import { EventStream } from "../EventStream";
 import { DownloadSuccess, DownloadStart, DownloadFallBack, DownloadFailure, DownloadProgress, DownloadSizeObtained } from "../omnisharp/loggingEvents";
 import { NestedError } from "../NestedError";
@@ -24,7 +23,7 @@ export async function DownloadFile(description: string, eventStream: EventStream
         // If the package has a fallback Url, and downloading from the primary Url failed, try again from
         // the fallback. This is used for debugger packages as some users have had issues downloading from
         // the CDN link
-        if (fallbackUrl) {
+        if (fallbackUrl !== undefined) {
             eventStream.post(new DownloadFallBack(fallbackUrl));
             try {
                 let buffer = await downloadFile(description, fallbackUrl, eventStream, networkSettingsProvider);
@@ -51,7 +50,7 @@ async function downloadFile(description: string, urlString: string, eventStream:
         path: url.path,
         agent: getProxyAgent(url, proxy, strictSSL),
         port: url.port,
-        rejectUnauthorized: util.isBoolean(strictSSL) ? strictSSL : true
+        rejectUnauthorized: strictSSL,
     };
 
     let buffers: any[] = [];
@@ -60,13 +59,21 @@ async function downloadFile(description: string, urlString: string, eventStream:
         let request = https.request(options, response => {
             if (response.statusCode === 301 || response.statusCode === 302) {
                 // Redirect - download from new location
+                if (response.headers.location === undefined) {
+                    eventStream.post(new DownloadFailure(`Failed to download from ${urlString}. Redirected without location header`));
+                    return reject(new NestedError('Missing location'));
+                }
                 return resolve(downloadFile(description, response.headers.location, eventStream, networkSettingsProvider));
             }
-
-            else if (response.statusCode != 200) {
+            else if (response.statusCode !== 200) {
                 // Download failed - print error message
                 eventStream.post(new DownloadFailure(`Failed to download from ${urlString}. Error code '${response.statusCode}')`));
-                return reject(new NestedError(response.statusCode.toString()));
+                return reject(new NestedError(response.statusCode!.toString())); // Known to exist because this is from a ClientRequest
+            }
+
+            if (response.headers['content-length'] === undefined) {
+                eventStream.post(new DownloadFailure(`Failed to download from ${urlString}. No content-length header`));
+                return reject(new NestedError('Missing content-length'));
             }
 
             // Downloading - hook up events
@@ -104,4 +111,4 @@ async function downloadFile(description: string, urlString: string, eventStream:
         // Execute the request
         request.end();
     });
-}
+}