fix: use same logic to resolve typescript in banner and server

This commit refactors version_provider.ts to resolve a node module with
any minimum version, not just major version.

The same logic is now used in server.ts and banner.ts to resolve
tsserver. This enables us to accurately report the version of the
module require-d.

Author: Keen Yee Liau

server/src/banner.ts

@@ -1,4 +1,5 @@
 import {parseCommandLine} from './cmdline_utils';
+import {resolveTsServer} from './version_provider';
 
 /**
  * This method provides a custom implementation for the AMD loader to resolve
@@ -7,17 +8,14 @@ import {parseCommandLine} from './cmdline_utils';
  * @param cb function to invoke with resolved modules
  */
 export function define(modules: string[], cb: (...modules: any[]) => void) {
-  function resolve(packageName: string, paths: string[]) {
-    try {
-      return require.resolve(packageName, {paths});
-    } catch {
-    }
-  }
   const TSSERVER = 'typescript/lib/tsserverlibrary';
   const resolvedModules = modules.map(m => {
-    if (m === TSSERVER || m === 'typescript') {
+    if (m === 'typescript') {
+      throw new Error(`Import '${TSSERVER}' instead of 'typescript'`);
+    }
+    if (m === TSSERVER) {
       const {tsProbeLocations} = parseCommandLine(process.argv);
-      m = resolve(TSSERVER, tsProbeLocations) || TSSERVER;
+      m = resolveTsServer(tsProbeLocations).resolvedPath;
     }
     return require(m);
   });

server/src/server.ts

@@ -10,7 +10,7 @@ import {generateHelpMessage, parseCommandLine} from './cmdline_utils';
 import {createLogger} from './logger';
 import {ServerHost} from './server_host';
 import {Session} from './session';
-import {resolveWithMinMajor} from './version_provider';
+import {resolveNgLangSvc, resolveTsServer} from './version_provider';
 
 // Parse command line arguments
 const options = parseCommandLine(process.argv);
@@ -26,9 +26,8 @@ const logger = createLogger({
   logVerbosity: options.logVerbosity,
 });
 
-const {tsProbeLocations, ngProbeLocations} = options;
-const ts = resolveWithMinMajor('typescript', 3, tsProbeLocations);
-const ng = resolveWithMinMajor('@angular/language-service', 9, ngProbeLocations);
+const ts = resolveTsServer(options.tsProbeLocations);
+const ng = resolveNgLangSvc(options.ngProbeLocations);
 
 // ServerHost provides native OS functionality
 const host = new ServerHost();

server/src/tests/version_provider_spec.ts

@@ -6,23 +6,69 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {resolveWithMinMajor} from '../version_provider';
+import {resolveNgLangSvc, resolveTsServer, Version} from '../version_provider';
 
-describe('resolveWithMinMajor', () => {
+describe('Node Module Resolver', () => {
   const probeLocations = [__dirname];
 
-  it('should find typescript >= v2', () => {
-    const result = resolveWithMinMajor('typescript', 2, probeLocations);
-    expect(result.version).toBe('3.6.4');
+  it('should be able to resolve tsserver', () => {
+    const result = resolveTsServer(probeLocations);
+    expect(result).toBeDefined();
+    expect(result.resolvedPath).toMatch(/typescript\/lib\/tsserverlibrary.js$/);
   });
 
-  it('should find typescript v3', () => {
-    const result = resolveWithMinMajor('typescript', 3, probeLocations);
-    expect(result.version).toBe('3.6.4');
+  it('should be able to resolve Angular language service', () => {
+    const result = resolveNgLangSvc(probeLocations);
+    expect(result).toBeDefined();
+    expect(result.resolvedPath).toMatch(/language-service.umd.js$/);
   });
+});
+
+describe('Version', () => {
+  it('should parse version string correctly', () => {
+    const cases: Array<[string, number, number, number]> = [
+      // version string | major | minor | patch
+      ['1', 1, 0, 0],
+      ['1.2', 1, 2, 0],
+      ['1.2.3', 1, 2, 3],
+      ['9.0.0-rc.1+126.sha-0c38aae.with-local-changes', 9, 0, 0],
+    ];
+    for (const [versionStr, major, minor, patch] of cases) {
+      const v = new Version(versionStr);
+      expect(v.major).toBe(major);
+      expect(v.minor).toBe(minor);
+      expect(v.patch).toBe(patch);
+    }
+  });
+
+  it('should compare versions correctly', () => {
+    const cases: Array<[string, string, boolean]> = [
+      // lhs | rhs | result
+      ['1', '1', true],
+      ['1', '2', false],
+      ['2', '2.0', true],
+      ['2', '2.1', false],
+      ['2', '2.0.0', true],
+      ['2', '2.0.1', false],
+
+      ['1.2', '1', true],
+      ['1.2', '2', false],
+      ['2.2', '2.1', true],
+      ['2.2', '2.7', false],
+      ['3.2', '3.2.0', true],
+      ['3.2', '3.2.1', false],
 
-  it('should fail to find typescript v4', () => {
-    expect(() => resolveWithMinMajor('typescript', 4, probeLocations))
-        .toThrowError(/^Failed to resolve 'typescript'/);
+      ['1.2.3', '1', true],
+      ['1.2.3', '2', false],
+      ['2.2.3', '2.1', true],
+      ['2.2.3', '2.3', false],
+      ['3.2.3', '3.2.2', true],
+      ['3.2.3', '3.2.4', false],
+    ];
+    for (const [s1, s2, result] of cases) {
+      const v1 = new Version(s1);
+      const v2 = new Version(s2);
+      expect(v1.greaterThanOrEqual(v2)).toBe(result, `Expect ${v1} >= ${v2}`);
+    }
   });
 });

server/src/version_provider.ts

@@ -6,65 +6,136 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import * as path from 'path';
+const MIN_TS_VERSION = '3.6';
+const MIN_NG_VERSION = '9.0';
 
 /**
  * Represents a valid node module that has been successfully resolved.
  */
-export interface NodeModule {
+interface NodeModule {
   resolvedPath: string;
-  version?: string;
+  version: Version;
 }
 
-function resolve(packageName: string, paths: string[]): NodeModule|undefined {
+function resolve(packageName: string, location: string, rootPackage?: string): NodeModule|
+    undefined {
+  rootPackage = rootPackage || packageName;
   try {
-    // Here, use native NodeJS require instead of ServerHost.require because
-    // we want the full path of the resolution provided by native
-    // `require.resolve()`, which ServerHost does not provide.
-    const resolvedPath = require.resolve(`${packageName}/package.json`, {paths});
-    const packageJson = require(resolvedPath);
+    const packageJsonPath = require.resolve(`${rootPackage}/package.json`, {
+      paths: [location],
+    });
+    const packageJson = require(packageJsonPath);
+    const resolvedPath = require.resolve(packageName, {
+      paths: [location],
+    });
     return {
-      resolvedPath: path.dirname(resolvedPath),
-      version: packageJson.version,
+      resolvedPath,
+      version: new Version(packageJson.version),
     };
   } catch {
   }
 }
 
-function minVersion(nodeModule: NodeModule, minMajor: number): boolean {
-  if (!nodeModule.version) {
-    return false;
-  }
-  const [majorStr] = nodeModule.version.split('.');
-  if (!majorStr) {
-    return false;
+/**
+ * Resolve the node module with the specified `packageName` that satisfies
+ * the specified minimum version.
+ * @param packageName name of package to be resolved
+ * @param minVersionStr minimum version
+ * @param probeLocations locations to initiate node module resolution
+ * @param rootPackage location of package.json, if different from `packageName`
+ */
+function resolveWithMinVersion(
+    packageName: string, minVersionStr: string, probeLocations: string[],
+    rootPackage?: string): NodeModule {
+  if (rootPackage && !packageName.startsWith(rootPackage)) {
+    throw new Error(`${packageName} must be in the root package`);
   }
-  const major = Number(majorStr);
-  if (isNaN(major)) {
-    return false;
+  const minVersion = new Version(minVersionStr);
+  for (const location of probeLocations) {
+    const nodeModule = resolve(packageName, location, rootPackage);
+    if (nodeModule && nodeModule.version.greaterThanOrEqual(minVersion)) {
+      return nodeModule;
+    }
   }
-  return major >= minMajor;
+  throw new Error(
+      `Failed to resolve '${packageName}' with minimum version '${minVersion}' from ` +
+      JSON.stringify(probeLocations, null, 2));
 }
 
 /**
- * Resolve the node module with the specified `packageName` that satisfies
- * the specified minimum major version.
- * @param packageName
- * @param minMajor
+ * Resolve `typescript/lib/tsserverlibrary` from the given locations.
  * @param probeLocations
  */
-export function resolveWithMinMajor(
-    packageName: string, minMajor: number, probeLocations: string[]): NodeModule {
-  for (const location of probeLocations) {
-    const nodeModule = resolve(packageName, [location]);
-    if (!nodeModule) {
-      continue;
+export function resolveTsServer(probeLocations: string[]): NodeModule {
+  const tsserver = 'typescript/lib/tsserverlibrary';
+  return resolveWithMinVersion(tsserver, MIN_TS_VERSION, probeLocations, 'typescript');
+}
+
+/**
+ * Resolve `@angular/language-service` from the given locations.
+ * @param probeLocations
+ */
+export function resolveNgLangSvc(probeLocations: string[]): NodeModule {
+  const nglangsvc = '@angular/language-service';
+  return resolveWithMinVersion(nglangsvc, MIN_NG_VERSION, probeLocations);
+}
+
+/**
+ * Converts the specified string `a` to non-negative integer.
+ * Returns -1 if the result is NaN.
+ * @param a
+ */
+function parseNonNegativeInt(a: string): number {
+  // parseInt() will try to convert as many as possible leading characters that
+  // are digits. This means a string like "123abc" will be converted to 123.
+  // For our use case, this is sufficient.
+  const i = parseInt(a, 10 /* radix */);
+  return isNaN(i) ? -1 : i;
+}
+
+export class Version {
+  readonly major: number;
+  readonly minor: number;
+  readonly patch: number;
+
+  constructor(private readonly versionStr: string) {
+    const [major, minor, patch] = Version.parseVersionStr(versionStr);
+    this.major = major;
+    this.minor = minor;
+    this.patch = patch;
+  }
+
+  greaterThanOrEqual(other: Version): boolean {
+    if (this.major < other.major) {
+      return false;
     }
-    if (minVersion(nodeModule, minMajor)) {
-      return nodeModule;
+    if (this.major > other.major) {
+      return true;
     }
+    if (this.minor < other.minor) {
+      return false;
+    }
+    if (this.minor > other.minor) {
+      return true;
+    }
+    return this.patch >= other.patch;
+  }
+
+  toString(): string {
+    return this.versionStr;
+  }
+
+  /**
+   * Converts the specified `versionStr` to its number constituents. Invalid
+   * number value is represented as negative number.
+   * @param versionStr
+   */
+  static parseVersionStr(versionStr: string): [number, number, number] {
+    const [major, minor, patch] = versionStr.split('.').map(parseNonNegativeInt);
+    return [
+      major === undefined ? 0 : major,
+      minor === undefined ? 0 : minor,
+      patch === undefined ? 0 : patch,
+    ];
   }
-  throw new Error(
-      `Failed to resolve '${packageName}' with minimum major version '${minMajor}' from ` +
-      JSON.stringify(probeLocations, null, 2));
 }