fix(compiler-cli): use numeric comparison for TypeScript version (#22705)

Fixes #22593

PR Close #22705

Author: benbraou

packages/compiler-cli/src/diagnostics/typescript_symbols.ts

@@ -12,7 +12,7 @@ import * as path from 'path';
 import * as ts from 'typescript';
 
 import {BuiltinType, DeclarationKind, Definition, PipeInfo, Pipes, Signature, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols';
-
+import {isVersionBetween} from './typescript_version';
 
 // In TypeScript 2.1 these flags moved
 // These helpers work for both 2.0 and 2.1.
@@ -394,21 +394,33 @@ class SignatureResultOverride implements Signature {
   get result(): Symbol { return this.resultType; }
 }
 
-const toSymbolTable: (symbols: ts.Symbol[]) => ts.SymbolTable = isTypescriptVersion('2.2') ?
-    (symbols => {
-      const result = new Map<string, ts.Symbol>();
-      for (const symbol of symbols) {
-        result.set(symbol.name, symbol);
-      }
-      return <ts.SymbolTable>(result as any);
-    }) :
-    (symbols => {
-      const result = <any>{};
-      for (const symbol of symbols) {
-        result[symbol.name] = symbol;
-      }
-      return result as ts.SymbolTable;
-    });
+/**
+ * Indicates the lower bound TypeScript version supporting `SymbolTable` as an ES6 `Map`.
+ * For lower versions, `SymbolTable` is implemented as a dictionary
+ */
+const MIN_TS_VERSION_SUPPORTING_MAP = '2.2';
+
+export const toSymbolTableFactory = (tsVersion: string) => (symbols: ts.Symbol[]) => {
+  if (isVersionBetween(tsVersion, MIN_TS_VERSION_SUPPORTING_MAP)) {
+    // ∀ Typescript version >= 2.2, `SymbolTable` is implemented as an ES6 `Map`
+    const result = new Map<string, ts.Symbol>();
+    for (const symbol of symbols) {
+      result.set(symbol.name, symbol);
+    }
+    // First, tell the compiler that `result` is of type `any`. Then, use a second type assertion
+    // to `ts.SymbolTable`.
+    // Otherwise, `Map<string, ts.Symbol>` and `ts.SymbolTable` will be considered as incompatible
+    // types by the compiler
+    return <ts.SymbolTable>(<any>result);
+  }
+
+  // ∀ Typescript version < 2.2, `SymbolTable` is implemented as a dictionary
+  const result: {[name: string]: ts.Symbol} = {};
+  for (const symbol of symbols) {
+    result[symbol.name] = symbol;
+  }
+  return <ts.SymbolTable>(<any>result);
+};
 
 function toSymbols(symbolTable: ts.SymbolTable | undefined): ts.Symbol[] {
   if (!symbolTable) return [];
@@ -442,6 +454,7 @@ class SymbolTableWrapper implements SymbolTable {
 
     if (Array.isArray(symbols)) {
       this.symbols = symbols;
+      const toSymbolTable = toSymbolTableFactory(ts.version);
       this.symbolTable = toSymbolTable(symbols);
     } else {
       this.symbols = toSymbols(symbols);
@@ -855,22 +868,3 @@ function getFromSymbolTable(symbolTable: ts.SymbolTable, key: string): ts.Symbol
 
   return symbol;
 }
-
-function toNumbers(value: string | undefined): number[] {
-  return value ? value.split('.').map(v => +v) : [];
-}
-
-function compareNumbers(a: number[], b: number[]): -1|0|1 {
-  for (let i = 0; i < a.length && i < b.length; i++) {
-    if (a[i] > b[i]) return 1;
-    if (a[i] < b[i]) return -1;
-  }
-  return 0;
-}
-
-function isTypescriptVersion(low: string, high?: string): boolean {
-  const tsNumbers = toNumbers(ts.version);
-
-  return compareNumbers(toNumbers(low), tsNumbers) <= 0 &&
-      compareNumbers(toNumbers(high), tsNumbers) >= 0;
-}

packages/compiler-cli/src/diagnostics/typescript_version.ts

@@ -0,0 +1,85 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+/**
+ * Converts a `string` version into an array of numbers
+ * @example
+ * toNumbers('2.0.1'); // returns [2, 0, 1]
+ */
+export function toNumbers(value: string): number[] {
+  return value.split('.').map(Number);
+}
+
+/**
+ * Compares two arrays of positive numbers with lexicographical order in mind.
+ *
+ * However - unlike lexicographical order - for arrays of different length we consider:
+ * [1, 2, 3] = [1, 2, 3, 0] instead of [1, 2, 3] < [1, 2, 3, 0]
+ *
+ * @param a The 'left hand' array in the comparison test
+ * @param b The 'right hand' in the comparison test
+ * @returns {-1|0|1} The comparison result: 1 if a is greater, -1 if b is greater, 0 is the two
+ * arrays are equals
+ */
+export function compareNumbers(a: number[], b: number[]): -1|0|1 {
+  const max = Math.max(a.length, b.length);
+  const min = Math.min(a.length, b.length);
+
+  for (let i = 0; i < min; i++) {
+    if (a[i] > b[i]) return 1;
+    if (a[i] < b[i]) return -1;
+  }
+
+  if (min !== max) {
+    const longestArray = a.length === max ? a : b;
+
+    // The result to return in case the to arrays are considered different (1 if a is greater,
+    // -1 if b is greater)
+    const comparisonResult = a.length === max ? 1 : -1;
+
+    // Check that at least one of the remaining elements is greater than 0 to consider that the two
+    // arrays are different (e.g. [1, 0] and [1] are considered the same but not [1, 0, 1] and [1])
+    for (let i = min; i < max; i++) {
+      if (longestArray[i] > 0) {
+        return comparisonResult;
+      }
+    }
+  }
+
+  return 0;
+}
+
+/**
+ * Checks if a TypeScript version is:
+ * - greater or equal than the provided `low` version,
+ * - lower or equal than an optional `high` version.
+ *
+ * @param version The TypeScript version
+ * @param low The minimum version
+ * @param high The maximum version
+ */
+export function isVersionBetween(version: string, low: string, high?: string): boolean {
+  const tsNumbers = toNumbers(version);
+  if (high !== undefined) {
+    return compareNumbers(toNumbers(low), tsNumbers) <= 0 &&
+        compareNumbers(toNumbers(high), tsNumbers) >= 0;
+  }
+  return compareNumbers(toNumbers(low), tsNumbers) <= 0;
+}
+
+/**
+ * Compares two versions
+ *
+ * @param v1 The 'left hand' version in the comparison test
+ * @param v2 The 'right hand' version in the comparison test
+ * @returns {-1|0|1} The comparison result: 1 if v1 is greater, -1 if v2 is greater, 0 is the two
+ * versions are equals
+ */
+export function compareVersions(v1: string, v2: string): -1|0|1 {
+  return compareNumbers(toNumbers(v1), toNumbers(v2));
+}

packages/compiler-cli/src/transformers/program.ts

@@ -13,6 +13,7 @@ import * as path from 'path';
 import * as ts from 'typescript';
 
 import {TypeCheckHost, translateDiagnostics} from '../diagnostics/translate_diagnostics';
+import {compareVersions} from '../diagnostics/typescript_version';
 import {MetadataCollector, ModuleMetadata, createBundleIndexHost} from '../metadata/index';
 
 import {CompilerHost, CompilerOptions, CustomTransformers, DEFAULT_ERROR_CODE, Diagnostic, DiagnosticMessageChain, EmitFlags, LazyRoute, LibrarySummary, Program, SOURCE, TsEmitArguments, TsEmitCallback, TsMergeEmitResultsCallback} from './api';
@@ -95,6 +96,19 @@ const defaultEmitCallback: TsEmitCallback =
         program.emit(
             targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers);
 
+/**
+ * Minimum supported TypeScript version
+ * ∀ supported typescript version v, v >= MIN_TS_VERSION
+ */
+const MIN_TS_VERSION = '2.7.2';
+
+/**
+ * Supremum of supported TypeScript versions
+ * ∀ supported typescript version v, v < MAX_TS_VERSION
+ * MAX_TS_VERSION is not considered as a supported TypeScript version
+ */
+const MAX_TS_VERSION = '2.8.0';
+
 class AngularCompilerProgram implements Program {
   private rootNames: string[];
   private metadataCache: MetadataCache;
@@ -124,10 +138,7 @@ class AngularCompilerProgram implements Program {
       private host: CompilerHost, oldProgram?: Program) {
     this.rootNames = [...rootNames];
 
-    if ((ts.version < '2.7.2' || ts.version >= '2.8.0') && !options.disableTypeScriptVersionCheck) {
-      throw new Error(
-          `The Angular Compiler requires TypeScript >=2.7.2 and <2.8.0 but ${ts.version} was found instead.`);
-    }
+    checkVersion(ts.version, MIN_TS_VERSION, MAX_TS_VERSION, options.disableTypeScriptVersionCheck);
 
     this.oldTsProgram = oldProgram ? oldProgram.getTsProgram() : undefined;
     if (oldProgram) {
@@ -847,6 +858,34 @@ class AngularCompilerProgram implements Program {
   }
 }
 
+/**
+ * Checks whether a given version ∈ [minVersion, maxVersion[
+ * An error will be thrown if the following statements are simultaneously true:
+ * - the given version ∉ [minVersion, maxVersion[,
+ * - the result of the version check is not meant to be bypassed (the parameter disableVersionCheck
+ * is false)
+ *
+ * @param version The version on which the check will be performed
+ * @param minVersion The lower bound version. A valid version needs to be greater than minVersion
+ * @param maxVersion The upper bound version. A valid version needs to be strictly less than
+ * maxVersion
+ * @param disableVersionCheck Indicates whether version check should be bypassed
+ *
+ * @throws Will throw an error if the following statements are simultaneously true:
+ * - the given version ∉ [minVersion, maxVersion[,
+ * - the result of the version check is not meant to be bypassed (the parameter disableVersionCheck
+ * is false)
+ */
+export function checkVersion(
+    version: string, minVersion: string, maxVersion: string,
+    disableVersionCheck: boolean | undefined) {
+  if ((compareVersions(version, minVersion) < 0 || compareVersions(version, maxVersion) >= 0) &&
+      !disableVersionCheck) {
+    throw new Error(
+        `The Angular Compiler requires TypeScript >=${minVersion} and <${maxVersion} but ${version} was found instead.`);
+  }
+}
+
 export function createProgram({rootNames, options, host, oldProgram}: {
   rootNames: ReadonlyArray<string>,
   options: CompilerOptions,

…li/test/diagnostics/symbol_query_spec.ts …t/diagnostics/typescript_symbols_spec.tspackages/compiler-cli/test/diagnostics/symbol_query_spec.ts renamed to packages/compiler-cli/test/diagnostics/typescript_symbols_spec.ts packages/compiler-cli/test/diagnostics/symbol_query_spec.ts renamed to packages/compiler-cli/test/diagnostics/typescript_symbols_spec.ts

@@ -13,7 +13,7 @@ import {ReflectorHost} from '@angular/language-service/src/reflector_host';
 import * as ts from 'typescript';
 
 import {Symbol, SymbolQuery, SymbolTable} from '../../src/diagnostics/symbols';
-import {getSymbolQuery} from '../../src/diagnostics/typescript_symbols';
+import {getSymbolQuery, toSymbolTableFactory} from '../../src/diagnostics/typescript_symbols';
 import {CompilerOptions} from '../../src/transformers/api';
 import {Directory} from '../mocks';
 
@@ -57,6 +57,19 @@ describe('symbol query', () => {
   });
 });
 
+describe('toSymbolTableFactory(tsVersion)', () => {
+  it('should return a Map for versions of TypeScript >= 2.2 and a dictionary otherwise', () => {
+    const a = { name: 'a' } as ts.Symbol;
+    const b = { name: 'b' } as ts.Symbol;
+
+    expect(toSymbolTableFactory('2.1')([a, b]) instanceof Map).toEqual(false);
+    expect(toSymbolTableFactory('2.4')([a, b]) instanceof Map).toEqual(true);
+
+    // Check that for the lower bound version `2.2`, toSymbolTableFactory('2.2') returns a map
+    expect(toSymbolTableFactory('2.2')([a, b]) instanceof Map).toEqual(true);
+  });
+});
+
 function appComponentSource(template: string): string {
   return `
     import {Component} from '@angular/core';

packages/compiler-cli/test/diagnostics/typescript_version_spec.ts

@@ -0,0 +1,70 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {compareNumbers, compareVersions, isVersionBetween, toNumbers} from '../../src/diagnostics/typescript_version';
+
+
+describe('toNumbers', () => {
+  it('should handle strings', () => {
+    expect(toNumbers('2')).toEqual([2]);
+    expect(toNumbers('2.1')).toEqual([2, 1]);
+    expect(toNumbers('2.0.1')).toEqual([2, 0, 1]);
+  });
+});
+
+describe('compareNumbers', () => {
+
+  it('should handle empty arrays', () => { expect(compareNumbers([], [])).toEqual(0); });
+
+  it('should handle arrays of same length', () => {
+    expect(compareNumbers([1], [3])).toEqual(-1);
+    expect(compareNumbers([3], [1])).toEqual(1);
+
+    expect(compareNumbers([1, 0], [1, 0])).toEqual(0);
+
+    expect(compareNumbers([1, 1], [1, 0])).toEqual(1);
+    expect(compareNumbers([1, 0], [1, 1])).toEqual(-1);
+
+    expect(compareNumbers([1, 0, 9], [1, 1, 0])).toEqual(-1);
+    expect(compareNumbers([1, 1, 0], [1, 0, 9])).toEqual(1);
+  });
+
+  it('should handle arrays of different length', () => {
+    expect(compareNumbers([2], [2, 1])).toEqual(-1);
+    expect(compareNumbers([2, 1], [2])).toEqual(1);
+
+    expect(compareNumbers([0, 9], [1])).toEqual(-1);
+    expect(compareNumbers([1], [0, 9])).toEqual(1);
+
+    expect(compareNumbers([2], [])).toEqual(1);
+    expect(compareNumbers([], [2])).toEqual(-1);
+
+    expect(compareNumbers([1, 0], [1, 0, 0, 0])).toEqual(0);
+  });
+});
+
+describe('isVersionBetween', () => {
+  it('should correctly check if a typescript version is within a given range', () => {
+    expect(isVersionBetween('2.7.0', '2.40')).toEqual(false);
+    expect(isVersionBetween('2.40', '2.7.0')).toEqual(true);
+
+    expect(isVersionBetween('2.7.2', '2.7.0', '2.8.0')).toEqual(true);
+
+    expect(isVersionBetween('2.7.2', '2.7.7', '2.8.0')).toEqual(false);
+  });
+});
+
+describe('compareVersions', () => {
+  it('should correctly compare versions', () => {
+    expect(compareVersions('2.7.0', '2.40')).toEqual(-1);
+    expect(compareVersions('2.40', '2.7.0')).toEqual(1);
+    expect(compareVersions('2.40', '2.40')).toEqual(0);
+    expect(compareVersions('2.40', '2.41')).toEqual(-1);
+    expect(compareVersions('2', '2.1')).toEqual(-1);
+  });
+});

packages/compiler-cli/test/transformers/program_spec.ts

@@ -13,7 +13,7 @@ import * as ts from 'typescript';
 
 import {formatDiagnostics} from '../../src/perform_compile';
 import {CompilerHost, EmitFlags, LazyRoute} from '../../src/transformers/api';
-import {createSrcToOutPathMapper} from '../../src/transformers/program';
+import {checkVersion, createSrcToOutPathMapper} from '../../src/transformers/program';
 import {GENERATED_FILES, StructureIsReused, tsStructureIsReused} from '../../src/transformers/util';
 import {TestSupport, expectNoDiagnosticsInProgram, isInBazel, setup} from '../test_support';
 
@@ -1048,4 +1048,35 @@ describe('ng program', () => {
              .toContain('Function expressions are not supported');
        });
   });
+
+  describe('checkVersion', () => {
+    const MIN_TS_VERSION = '2.7.2';
+    const MAX_TS_VERSION = '2.8.0';
+
+    const versionError = (version: string) =>
+        `The Angular Compiler requires TypeScript >=${MIN_TS_VERSION} and <${MAX_TS_VERSION} but ${version} was found instead.`;
+
+    it('should not throw when a supported TypeScript version is used', () => {
+      expect(() => checkVersion('2.7.2', MIN_TS_VERSION, MAX_TS_VERSION, undefined)).not.toThrow();
+      expect(() => checkVersion('2.7.2', MIN_TS_VERSION, MAX_TS_VERSION, false)).not.toThrow();
+      expect(() => checkVersion('2.7.2', MIN_TS_VERSION, MAX_TS_VERSION, true)).not.toThrow();
+    });
+
+    it('should handle a TypeScript version < the minimum supported one', () => {
+      expect(() => checkVersion('2.4.1', MIN_TS_VERSION, MAX_TS_VERSION, undefined))
+          .toThrowError(versionError('2.4.1'));
+      expect(() => checkVersion('2.4.1', MIN_TS_VERSION, MAX_TS_VERSION, false))
+          .toThrowError(versionError('2.4.1'));
+      expect(() => checkVersion('2.4.1', MIN_TS_VERSION, MAX_TS_VERSION, true)).not.toThrow();
+    });
+
+    it('should handle a TypeScript version > the maximum supported one', () => {
+      expect(() => checkVersion('2.9.0', MIN_TS_VERSION, MAX_TS_VERSION, undefined))
+          .toThrowError(versionError('2.9.0'));
+      expect(() => checkVersion('2.9.0', MIN_TS_VERSION, MAX_TS_VERSION, false))
+          .toThrowError(versionError('2.9.0'));
+      expect(() => checkVersion('2.9.0', MIN_TS_VERSION, MAX_TS_VERSION, true)).not.toThrow();
+    });
+  });
+
 });