fix(compiler-cli): ensure file_system handles mixed Windows drives

The `fs.relative()` method assumed that the file-system is a single tree,
which is not the case in Windows, where you can have multiple drives,
e.g. `C:`, `D:` etc.

This commit changes `fs.relative()` so that it no longer forces the result
to be a `PathSegment` and then flows that refactoring through the rest of
the compiler-cli (and ngcc).  The main difference is that now, in some cases,
we needed to check whether the result is "rooted", i.e an `AbsoluteFsPath`,
rather than a `PathSegment`, before using it.

Fixes angular#36777

packages/compiler-cli/ngcc/src/analysis/util.ts

@@ -5,12 +5,12 @@
  * 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 {AbsoluteFsPath, relative} from '../../../src/ngtsc/file_system';
+import {AbsoluteFsPath, isLocalRelativePath, relative} from '../../../src/ngtsc/file_system';
 import {DependencyTracker} from '../../../src/ngtsc/incremental/api';
 
 export function isWithinPackage(packagePath: AbsoluteFsPath, filePath: AbsoluteFsPath): boolean {
   const relativePath = relative(packagePath, filePath);
-  return !relativePath.startsWith('..') && !relativePath.startsWith('node_modules/');
+  return isLocalRelativePath(relativePath) && !relativePath.startsWith('node_modules/');
 }
 
 class NoopDependencyTracker implements DependencyTracker {

packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts

@@ -9,7 +9,7 @@ import {Statement} from '@angular/compiler';
 import MagicString from 'magic-string';
 import * as ts from 'typescript';
 
-import {absoluteFromSourceFile, AbsoluteFsPath, dirname, relative} from '../../../src/ngtsc/file_system';
+import {absoluteFromSourceFile, AbsoluteFsPath, dirname, relative, toRelativeImport} from '../../../src/ngtsc/file_system';
 import {NOOP_DEFAULT_IMPORT_RECORDER, Reexport} from '../../../src/ngtsc/imports';
 import {Import, ImportManager, translateStatement} from '../../../src/ngtsc/translator';
 import {isDtsPath} from '../../../src/ngtsc/util/src/typescript';
@@ -57,8 +57,9 @@ export class EsmRenderingFormatter implements RenderingFormatter {
 
       if (from) {
         const basePath = stripExtension(from);
-        const relativePath = './' + relative(dirname(entryPointBasePath), basePath);
-        exportFrom = entryPointBasePath !== basePath ? ` from '${relativePath}'` : '';
+        const relativePath = relative(dirname(entryPointBasePath), basePath);
+        const relativeImport = toRelativeImport(relativePath);
+        exportFrom = entryPointBasePath !== basePath ? ` from '${relativeImport}'` : '';
       }
 
       const exportStr = `\nexport {${e.identifier}}${exportFrom};`;
@@ -197,10 +198,10 @@ export class EsmRenderingFormatter implements RenderingFormatter {
       const ngModuleName = info.ngModule.node.name.text;
       const declarationFile = absoluteFromSourceFile(info.declaration.getSourceFile());
       const ngModuleFile = absoluteFromSourceFile(info.ngModule.node.getSourceFile());
+      const relativePath = relative(dirname(declarationFile), ngModuleFile);
+      const relativeImport = toRelativeImport(relativePath);
       const importPath = info.ngModule.ownedByModuleGuess ||
-          (declarationFile !== ngModuleFile ?
-               stripExtension(`./${relative(dirname(declarationFile), ngModuleFile)}`) :
-               null);
+          (declarationFile !== ngModuleFile ? stripExtension(relativeImport) : null);
       const ngModule = generateImportString(importManager, importPath, ngModuleName);
 
       if (info.declaration.type) {

packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts

@@ -6,7 +6,7 @@
  * 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 {absoluteFromSourceFile, AbsoluteFsPath, dirname, FileSystem, join, relative} from '../../../src/ngtsc/file_system';
+import {absoluteFromSourceFile, AbsoluteFsPath, dirname, FileSystem, isLocalRelativePath, join, relative, resolve} from '../../../src/ngtsc/file_system';
 import {isDtsPath} from '../../../src/ngtsc/util/src/typescript';
 import {Logger} from '../logging/logger';
 import {EntryPoint, EntryPointJsonProperty} from '../packages/entry_point';
@@ -70,9 +70,9 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
       bundle: EntryPointBundle, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath) {
     bundle.src.program.getSourceFiles().forEach(sourceFile => {
       const relativePath = relative(packagePath, absoluteFromSourceFile(sourceFile));
-      const isOutsidePackage = relativePath.startsWith('..');
-      if (!sourceFile.isDeclarationFile && !isOutsidePackage) {
-        const newFilePath = join(ngccFolder, relativePath);
+      const isInsidePackage = isLocalRelativePath(relativePath);
+      if (!sourceFile.isDeclarationFile && isInsidePackage) {
+        const newFilePath = resolve(ngccFolder, relativePath);
         this.fs.ensureDir(dirname(newFilePath));
         this.fs.copyFile(absoluteFromSourceFile(sourceFile), newFilePath);
       }
@@ -86,7 +86,7 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
       super.writeFileAndBackup(file);
     } else {
       const relativePath = relative(packagePath, file.path);
-      const newFilePath = join(ngccFolder, relativePath);
+      const newFilePath = resolve(ngccFolder, relativePath);
       this.fs.ensureDir(dirname(newFilePath));
       this.fs.writeFile(newFilePath, file.contents);
     }
@@ -98,7 +98,7 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
       super.revertFileAndBackup(filePath);
     } else if (this.fs.exists(filePath)) {
       const relativePath = relative(packagePath, filePath);
-      const newFilePath = join(packagePath, NGCC_DIRECTORY, relativePath);
+      const newFilePath = resolve(packagePath, NGCC_DIRECTORY, relativePath);
       this.fs.removeFile(newFilePath);
     }
   }
@@ -117,8 +117,9 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
     // All format properties point to the same format-path.
     const oldFormatProp = formatProperties[0]!;
     const oldFormatPath = packageJson[oldFormatProp]!;
-    const oldAbsFormatPath = join(entryPoint.path, oldFormatPath);
-    const newAbsFormatPath = join(ngccFolder, relative(entryPoint.packagePath, oldAbsFormatPath));
+    const oldAbsFormatPath = resolve(entryPoint.path, oldFormatPath);
+    const newAbsFormatPath =
+        resolve(ngccFolder, relative(entryPoint.packagePath, oldAbsFormatPath));
     const newFormatPath = relative(entryPoint.path, newAbsFormatPath);
 
     // Update all properties in `package.json` (both in memory and on disk).

packages/compiler-cli/src/ngtsc/file_system/index.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 export {NgtscCompilerHost} from './src/compiler_host';
-export {absoluteFrom, absoluteFromSourceFile, basename, dirname, getFileSystem, isRoot, isRooted, join, relative, relativeFrom, resolve, setFileSystem} from './src/helpers';
+export {absoluteFrom, absoluteFromSourceFile, basename, dirname, getFileSystem, isLocalRelativePath, isRoot, isRooted, join, relative, relativeFrom, resolve, setFileSystem, toRelativeImport} from './src/helpers';
 export {LogicalFileSystem, LogicalProjectPath} from './src/logical';
 export {NodeJSFileSystem} from './src/node_js_file_system';
 export {AbsoluteFsPath, FileStats, FileSystem, PathSegment, PathString} from './src/types';

packages/compiler-cli/src/ngtsc/file_system/src/helpers.ts

@@ -83,7 +83,7 @@ export function isRooted(path: string): boolean {
 /**
  * Static access to `relative`.
  */
-export function relative<T extends PathString>(from: T, to: T): PathSegment {
+export function relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
   return fs.relative(from, to);
 }
 
@@ -93,3 +93,23 @@ export function relative<T extends PathString>(from: T, to: T): PathSegment {
 export function basename(filePath: PathString, extension?: string): PathSegment {
   return fs.basename(filePath, extension) as PathSegment;
 }
+
+/**
+ * Returns true if the given path is locally relative.
+ *
+ * This is used to work out if the given path is relative (i.e. not absolute) but also is not
+ * escaping the current directory.
+ */
+export function isLocalRelativePath(relativePath: string): boolean {
+  return !isRooted(relativePath) && !relativePath.startsWith('..');
+}
+
+/**
+ * Converts a path to a form suitable for use as a relative module import specifier.
+ *
+ * In other words it adds the `./` to the path if it is locally relative.
+ */
+export function toRelativeImport(relativePath: PathSegment|AbsoluteFsPath): PathSegment|
+    AbsoluteFsPath {
+  return isLocalRelativePath(relativePath) ? `./${relativePath}` as PathSegment : relativePath;
+}

packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts

@@ -82,7 +82,7 @@ export class InvalidFileSystem implements FileSystem {
   isRooted(path: string): boolean {
     throw makeError();
   }
-  relative<T extends PathString>(from: T, to: T): PathSegment {
+  relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
     throw makeError();
   }
   basename(filePath: string, extension?: string): PathSegment {

packages/compiler-cli/src/ngtsc/file_system/src/logical.ts

@@ -7,7 +7,7 @@
  */
 import * as ts from 'typescript';
 
-import {absoluteFrom, dirname, relative, resolve} from './helpers';
+import {absoluteFrom, dirname, isLocalRelativePath, relative, resolve, toRelativeImport} from './helpers';
 import {AbsoluteFsPath, BrandedPath, PathSegment} from './types';
 import {stripExtension} from './util';
 
@@ -29,11 +29,8 @@ export const LogicalProjectPath = {
    * importing from `to`.
    */
   relativePathBetween: function(from: LogicalProjectPath, to: LogicalProjectPath): PathSegment {
-    let relativePath = relative(dirname(resolve(from)), resolve(to));
-    if (!relativePath.startsWith('../')) {
-      relativePath = ('./' + relativePath) as PathSegment;
-    }
-    return relativePath as PathSegment;
+    const relativePath = relative(dirname(resolve(from)), resolve(to));
+    return toRelativeImport(relativePath) as PathSegment;
   },
 };
 
@@ -122,5 +119,5 @@ export class LogicalFileSystem {
  * E.g. `foo/bar/zee` is within `foo/bar` but not within `foo/car`.
  */
 function isWithinBasePath(base: AbsoluteFsPath, path: AbsoluteFsPath): boolean {
-  return !relative(base, path).startsWith('..');
+  return isLocalRelativePath(relative(base, path));
 }

packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts

@@ -9,7 +9,7 @@
 import * as fs from 'fs';
 import * as fsExtra from 'fs-extra';
 import * as p from 'path';
-import {absoluteFrom, relativeFrom} from './helpers';
+import {absoluteFrom} from './helpers';
 import {AbsoluteFsPath, FileStats, FileSystem, PathSegment, PathString} from './types';
 
 /**
@@ -93,8 +93,8 @@ export class NodeJSFileSystem implements FileSystem {
   isRooted(path: string): boolean {
     return p.isAbsolute(path);
   }
-  relative<T extends PathString>(from: T, to: T): PathSegment {
-    return relativeFrom(this.normalize(p.relative(from, to)));
+  relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
+    return this.normalize(p.relative(from, to)) as PathSegment | AbsoluteFsPath;
   }
   basename(filePath: string, extension?: string): PathSegment {
     return p.basename(filePath, extension) as PathSegment;

packages/compiler-cli/src/ngtsc/file_system/src/types.ts

@@ -57,7 +57,14 @@ export interface FileSystem {
   resolve(...paths: string[]): AbsoluteFsPath;
   dirname<T extends PathString>(file: T): T;
   join<T extends PathString>(basePath: T, ...paths: string[]): T;
-  relative<T extends PathString>(from: T, to: T): PathSegment;
+  /**
+   * Compute the relative path between `from` and `to`.
+   *
+   * In file-systems that can have multiple file trees the returned path may not actually be
+   * "relative" (i.e. `PathSegment`). For example, Windows can have multiple drives :
+   * `relative('c:/a/b', 'd:/a/c')` would be `d:/a/c'.
+   */
+  relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath;
   basename(filePath: string, extension?: string): PathSegment;
   realpath(filePath: AbsoluteFsPath): AbsoluteFsPath;
   getDefaultLibLocation(): AbsoluteFsPath;

packages/compiler-cli/src/ngtsc/file_system/src/util.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as ts from 'typescript';
-import {AbsoluteFsPath} from './types';
+import {AbsoluteFsPath, PathString} from './types';
 
 const TS_DTS_JS_EXTENSION = /(?:\.d)?\.ts$|\.js$/;
 
@@ -21,8 +21,8 @@ export function normalizeSeparators(path: string): string {
 /**
  * Remove a .ts, .d.ts, or .js extension from a file name.
  */
-export function stripExtension(path: string): string {
-  return path.replace(TS_DTS_JS_EXTENSION, '');
+export function stripExtension<T extends PathString>(path: T): T {
+  return path.replace(TS_DTS_JS_EXTENSION, '') as T;
 }
 
 export function getSourceFileOrError(program: ts.Program, fileName: AbsoluteFsPath): ts.SourceFile {

packages/compiler-cli/src/ngtsc/file_system/test/node_js_file_system_spec.ts

@@ -7,6 +7,7 @@
  */
 import * as realFs from 'fs';
 import * as fsExtra from 'fs-extra';
+import * as os from 'os';
 import {absoluteFrom, dirname, relativeFrom, setFileSystem} from '../src/helpers';
 import {NodeJSFileSystem} from '../src/node_js_file_system';
 import {AbsoluteFsPath} from '../src/types';
@@ -245,4 +246,13 @@ describe('NodeJSFileSystem', () => {
       expect(fs.isCaseSensitive()).toEqual(isCaseSensitive);
     });
   });
+
+  if (os.platform() === 'win32') {
+    // Only relevant on Windows
+    describe('relative', () => {
+      it('should handle Windows paths on different drives', () => {
+        expect(fs.relative('C:\\a\\b\\c', 'D:\\a\\b\\d')).toEqual(absoluteFrom('D:\\a\\b\\d'));
+      });
+    });
+  }
 });

packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts

@@ -214,7 +214,7 @@ export abstract class MockFileSystem implements FileSystem {
   abstract resolve(...paths: string[]): AbsoluteFsPath;
   abstract dirname<T extends string>(file: T): T;
   abstract join<T extends string>(basePath: T, ...paths: string[]): T;
-  abstract relative<T extends PathString>(from: T, to: T): PathSegment;
+  abstract relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath;
   abstract basename(filePath: string, extension?: string): PathSegment;
   abstract isRooted(path: string): boolean;
   abstract normalize<T extends PathString>(path: T): T;

packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system_native.ts

@@ -30,7 +30,7 @@ export class MockFileSystemNative extends MockFileSystem {
   join<T extends string>(basePath: T, ...paths: string[]): T {
     return NodeJSFileSystem.prototype.join.call(this, basePath, ...paths) as T;
   }
-  relative<T extends PathString>(from: T, to: T): PathSegment {
+  relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
     return NodeJSFileSystem.prototype.relative.call(this, from, to);
   }
 

packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system_posix.ts

@@ -25,8 +25,8 @@ export class MockFileSystemPosix extends MockFileSystem {
     return this.normalize(p.posix.join(basePath, ...paths)) as T;
   }
 
-  relative<T extends PathString>(from: T, to: T): PathSegment {
-    return this.normalize(p.posix.relative(from, to)) as PathSegment;
+  relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
+    return this.normalize(p.posix.relative(from, to)) as PathSegment | AbsoluteFsPath;
   }
 
   basename(filePath: string, extension?: string): PathSegment {

packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system_windows.ts

@@ -25,8 +25,8 @@ export class MockFileSystemWindows extends MockFileSystem {
     return this.normalize(p.win32.join(basePath, ...paths)) as T;
   }
 
-  relative<T extends PathString>(from: T, to: T): PathSegment {
-    return this.normalize(p.win32.relative(from, to)) as PathSegment;
+  relative<T extends PathString>(from: T, to: T): PathSegment|AbsoluteFsPath {
+    return this.normalize(p.win32.relative(from, to)) as PathSegment | AbsoluteFsPath;
   }
 
   basename(filePath: string, extension?: string): PathSegment {

packages/compiler-cli/src/ngtsc/imports/src/emitter.ts

@@ -9,7 +9,7 @@ import {Expression, ExternalExpr, ExternalReference, WrappedNodeExpr} from '@ang
 import * as ts from 'typescript';
 
 import {UnifiedModulesHost} from '../../core/api';
-import {absoluteFromSourceFile, dirname, LogicalFileSystem, LogicalProjectPath, PathSegment, relative} from '../../file_system';
+import {absoluteFromSourceFile, dirname, LogicalFileSystem, LogicalProjectPath, relative, toRelativeImport} from '../../file_system';
 import {stripExtension} from '../../file_system/src/util';
 import {ReflectionHost} from '../../reflection';
 import {getSourceFile, isDeclaration, isTypeDeclaration, nodeNameForError} from '../../util/src/typescript';
@@ -269,11 +269,9 @@ export class RelativePathStrategy implements ReferenceEmitStrategy {
 
   emit(ref: Reference<ts.Node>, context: ts.SourceFile): Expression|null {
     const destSf = getSourceFile(ref.node);
-    let moduleName = stripExtension(
-        relative(dirname(absoluteFromSourceFile(context)), absoluteFromSourceFile(destSf)));
-    if (!moduleName.startsWith('../')) {
-      moduleName = ('./' + moduleName) as PathSegment;
-    }
+    const relativePath =
+        relative(dirname(absoluteFromSourceFile(context)), absoluteFromSourceFile(destSf));
+    const moduleName = toRelativeImport(stripExtension(relativePath));
 
     const name = findExportedNameOfNode(ref.node, destSf, this.reflector);
     return new ExternalExpr({moduleName, name});

packages/compiler-cli/src/ngtsc/util/src/path.ts

@@ -5,23 +5,12 @@
  * 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 {dirname, relative, resolve} from '../../file_system';
-
-const TS_DTS_JS_EXTENSION = /(?:\.d)?\.ts$|\.js$/;
+import {dirname, relative, resolve, toRelativeImport} from '../../file_system';
+import {stripExtension} from '../../file_system/src/util';
 
 export function relativePathBetween(from: string, to: string): string|null {
-  let relativePath = relative(dirname(resolve(from)), resolve(to)).replace(TS_DTS_JS_EXTENSION, '');
-
-  if (relativePath === '') {
-    return null;
-  }
-
-  // path.relative() does not include the leading './'.
-  if (!relativePath.startsWith('.')) {
-    relativePath = `./${relativePath}`;
-  }
-
-  return relativePath;
+  const relativePath = stripExtension(relative(dirname(resolve(from)), resolve(to)));
+  return relativePath !== '' ? toRelativeImport(relativePath) : null;
 }
 
 export function normalizeSeparators(path: string): string {

packages/compiler-cli/test/ngtsc/env.ts

@@ -12,7 +12,7 @@ import * as ts from 'typescript';
 
 import {createCompilerHost, createProgram} from '../../index';
 import {main, mainDiagnosticsForTest, readNgcCommandLineAndConfiguration} from '../../src/main';
-import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, NgtscCompilerHost} from '../../src/ngtsc/file_system';
+import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, NgtscCompilerHost, relativeFrom} from '../../src/ngtsc/file_system';
 import {Folder, MockFileSystem} from '../../src/ngtsc/file_system/testing';
 import {IndexedComponent} from '../../src/ngtsc/indexer';
 import {NgtscProgram} from '../../src/ngtsc/program';
@@ -274,7 +274,8 @@ const ROOT_PREFIX = 'root/';
 
 class FileNameToModuleNameHost extends AugmentedCompilerHost {
   fileNameToModuleName(importedFilePath: string): string {
-    const relativeFilePath = this.fs.relative(this.fs.pwd(), this.fs.resolve(importedFilePath));
+    const relativeFilePath =
+        relativeFrom(this.fs.relative(this.fs.pwd(), this.fs.resolve(importedFilePath)));
     const rootedPath = this.fs.join('root', relativeFilePath);
     return rootedPath.replace(/(\.d)?.ts$/, '');
   }

packages/localize/src/tools/src/translate/asset_files/asset_translation_handler.ts

@@ -17,14 +17,14 @@ import {TranslationBundle, TranslationHandler} from '../translator';
 export class AssetTranslationHandler implements TranslationHandler {
   constructor(private fs: FileSystem) {}
 
-  canTranslate(_relativeFilePath: PathSegment, _contents: Buffer): boolean {
+  canTranslate(_relativeFilePath: PathSegment|AbsoluteFsPath, _contents: Buffer): boolean {
     return true;
   }
 
   translate(
-      diagnostics: Diagnostics, _sourceRoot: AbsoluteFsPath, relativeFilePath: PathSegment,
-      contents: Buffer, outputPathFn: OutputPathFn, translations: TranslationBundle[],
-      sourceLocale?: string): void {
+      diagnostics: Diagnostics, _sourceRoot: AbsoluteFsPath,
+      relativeFilePath: PathSegment|AbsoluteFsPath, contents: Buffer, outputPathFn: OutputPathFn,
+      translations: TranslationBundle[], sourceLocale?: string): void {
     for (const translation of translations) {
       this.writeAssetFile(
           diagnostics, outputPathFn, translation.locale, relativeFilePath, contents);
@@ -36,7 +36,7 @@ export class AssetTranslationHandler implements TranslationHandler {
 
   private writeAssetFile(
       diagnostics: Diagnostics, outputPathFn: OutputPathFn, locale: string,
-      relativeFilePath: PathSegment, contents: Buffer): void {
+      relativeFilePath: PathSegment|AbsoluteFsPath, contents: Buffer): void {
     try {
       const outputPath = absoluteFrom(outputPathFn(locale, relativeFilePath));
       this.fs.ensureDir(this.fs.dirname(outputPath));