fix(typescript): correctly handle absolute paths for imported modules

Previously, it was assumed that all `ts.SourceFile#fileName` properties
(and the thereof derived `ts.Symbol#name` properties) corresponding to
imported modules would be paths relative to the project root. This used
to be true with older versions of TypeScript (v3.x), but it stopped
being true with the recent update to TypeScript v4.x. This would result
in different values for things like `FileInfo#relativePath` and
`ModuleDoc#id`, leading to errors. An example error is failing to match
`module` doc IDs to special elements `relativePath`s [in angular.io][1].

This commit restores the previous behavior (i.e. ensuring that
`FileInfo#relativePath` and `ModuleDoc#id` remain the same as with older
versions) by making the following changes:

- Explicitly compute the relative path from the project root to a source
  file's path for `FileInfo#relativePath` (instead of assuming that
  `ts.SourceFile#fileName` is relative).

- Explicitly compute the relative path from the project root to a
  symbol's name, which is [based on `ts.SourceFile#fileName`][2]
  (instead of assuming that `ts.Symbol#name` is based on a relative
  path).

- Remove unnecessary logic for keeping `fileName` relative to the
  project root when calling `ts.createSourceFile()` in
  `CustomCompilerHost#getSourceFile()`. Since the `ts.SourceFile`s
  created for files that TypeScript discovers by following imports in
  other files will not have a relative `fileName`, there is no gain in
  trying to retain that property for `CustomCompilerHost`.

[1]: https://github.com/angular/angular/blob/8ebc946c0e7bf80d26ec8268acb4ff0af9e5c34a/aio/tools/transforms/angular-api-package/processors/processSpecialElements.js#L8-L21
[2]: https://github.com/microsoft/TypeScript/blob/1fe9bfdd0ee0ecf9dff23a40c0066645719e46b9/src/compiler/binder.ts#L2762

typescript/mocks/readTypeScriptModules/imports.ts

@@ -0,0 +1,3 @@
+import { X } from './modules';
+
+export const Y = !X;

typescript/src/api-doc-types/ModuleDoc.ts

@@ -1,4 +1,6 @@
 import { Declaration, TypeChecker } from 'typescript';
+import * as path from 'canonical-path';
+
 import { ModuleSymbol } from '../services/TsParser';
 import { FileInfo } from '../services/TsParser/FileInfo';
 
@@ -11,7 +13,7 @@ import { ExportDoc } from './ExportDoc';
  */
 export class ModuleDoc implements ApiDoc {
   docType = 'module';
-  id = this.symbol.name.replace(/^"|"$/g, '').replace(/\/index$/, '');
+  id = ensureRelative(this.basePath, this.symbol.name.replace(/^"|"$/g, '').replace(/\/index$/, ''));
   name = this.id.split('/').pop()!;
   declaration: Declaration = this.symbol.valueDeclaration!;
   aliases = [this.id, this.name];
@@ -28,3 +30,14 @@ export class ModuleDoc implements ApiDoc {
               public hidePrivateMembers: boolean,
               public typeChecker: TypeChecker) {}
 }
+
+/**
+ * Convert a potentially absolute path to relative.
+ *
+ * If `toPath` is already relative, it is practically returned unchanged. If it is an absolute path,
+ * it is resolved relative to `fromPath` (which should always be an absolute path).
+ */
+function ensureRelative(absoluteFromPath: string, toPath: string): string {
+  const absoluteToPath = path.resolve(absoluteFromPath, toPath);
+  return path.relative(absoluteFromPath, absoluteToPath);
+}

typescript/src/processors/readTypeScriptModules/index.spec.ts

@@ -273,7 +273,16 @@ describe('readTypeScriptModules', () => {
       expect(typeAliasDoc.docType).toEqual('class');
       expect(typeAliasDoc.typeParams).toEqual('<T = any>');
     });
-  })
+
+    it('should correctly compute doc IDs for imported modules', () => {
+      // NOTE: The order of files is important here. We want `modules.ts` to be first discovered by
+      //       TypeScript as an import in `imports.ts`, therefore, `imports.ts` has to come first.
+      processor.sourceFiles = ['imports.ts', 'modules.ts'];
+      const docs: DocCollection = [];
+      processor.$process(docs);
+      expect(docs.map(d => d.id)).toEqual(['imports', 'imports/Y', 'modules', 'modules/X']);
+    });
+  });
 
   describe('exported functions', () => {
     it('should include type parameters', () => {

typescript/src/services/TsParser/CustomCompilerHost.spec.ts

@@ -20,36 +20,36 @@ describe('CustomCompilerHost', () => {
   });
 
   describe('getSourceFile', () => {
-    it('should return a SourceFile object for a given path', () => {
+    it('should return a SourceFile object for a given relative path', () => {
       const sourceFile = host.getSourceFile('testSrc.ts', 0)!;
-      expect(sourceFile.fileName).toEqual('testSrc.ts');
+      expect(sourceFile.fileName).toEqual(resolve(baseDir, 'testSrc.ts'));
       expect(sourceFile.pos).toEqual(0);
       expect(sourceFile.text).toEqual(jasmine.any(String));
     });
 
-    it('should return a SourceFile object for a given path, with fileName relative to baseDir', () => {
+    it('should return a SourceFile object for a given absolute path', () => {
       const sourceFile = host.getSourceFile(resolve(baseDir, 'testSrc.ts'), 0)!;
-      expect(sourceFile.fileName).toEqual('testSrc.ts');
+      expect(sourceFile.fileName).toEqual(resolve(baseDir, 'testSrc.ts'));
       expect(sourceFile.pos).toEqual(0);
       expect(sourceFile.text).toEqual(jasmine.any(String));
     });
 
     it('should try each of the configured extensions and update the filename to the correct extension', () => {
       let sourceFile = host.getSourceFile('testSrc.js', 0)!;
-      expect(sourceFile.fileName).toEqual('testSrc.ts');
+      expect(sourceFile.fileName).toEqual(resolve(baseDir, 'testSrc.ts'));
 
       sourceFile = host.getSourceFile('../mockPackage.ts', 0)!;
-      expect(sourceFile.fileName).toEqual('../mockPackage.js');
+      expect(sourceFile.fileName).toEqual(resolve(baseDir, '../mockPackage.js'));
     });
 
     it('should cope with folders with names that look like source files', () => {
       const sourceFile = host.getSourceFile('zone.js', 0)!;
-      expect(sourceFile.fileName).toEqual('zone.js/index.ts');
+      expect(sourceFile.fileName).toEqual(resolve(baseDir, 'zone.js/index.ts'));
     });
 
     it('should cope with "invalid" relative references to node_modules, which are actually outside the baseDir', () => {
       const sourceFile = host.getSourceFile('node_modules/@types/jasmine/index.d.ts', 0)!;
-      expect(sourceFile.fileName).toEqual('../../../node_modules/@types/jasmine/index.d.ts');
+      expect(sourceFile.fileName).toEqual(resolve(baseDir, '../../../node_modules/@types/jasmine/index.d.ts'));
     });
   });
 

typescript/src/services/TsParser/CustomCompilerHost.ts

@@ -11,7 +11,6 @@ export class CustomCompilerHost implements CompilerHost {
 
   getSourceFile(fileName: string, languageVersion: ScriptTarget, onError?: (message: string) => void): SourceFile {
     let text;
-    let baseFilePath;
     let resolvedPath;
     let resolvedPathWithExt;
 
@@ -20,7 +19,7 @@ export class CustomCompilerHost implements CompilerHost {
       resolvedPath = path.resolve(this.baseDir, fileName);
       text = fs.readFileSync(resolvedPath, { encoding: this.options.charset as BufferEncoding });
       this.log.debug('found source file:', fileName);
-      return createSourceFile(path.relative(this.baseDir, resolvedPath), text, languageVersion);
+      return createSourceFile(resolvedPath, text, languageVersion);
     } catch (e) {
       // if it is a folder then try loading the index file of that folder
       if ((e as NodeJS.ErrnoException).code === 'EISDIR') {
@@ -44,7 +43,7 @@ export class CustomCompilerHost implements CompilerHost {
           resolvedPath = path.resolve(this.baseDir, maybe);
           text = fs.readFileSync(resolvedPath, { encoding: this.options.charset as BufferEncoding });
           this.log.debug('found source file:', fileName);
-          return createSourceFile(path.relative(this.baseDir, resolvedPath), text, languageVersion);
+          return createSourceFile(resolvedPath, text, languageVersion);
         } catch (e) {
           // ignore the error and move on to the next maybe...
         }
@@ -53,9 +52,7 @@ export class CustomCompilerHost implements CompilerHost {
     }
 
     // Strip off the extension and resolve relative to the baseDir
-    baseFilePath = fileName.replace(/\.[^./]+$/, '');
-    resolvedPath = path.resolve(this.baseDir, baseFilePath);
-    baseFilePath = path.relative(this.baseDir, resolvedPath);
+    resolvedPath = path.resolve(this.baseDir, fileName).replace(/\.[^./]+$/, '');
 
     // Iterate through each possible extension and return the first source file that is actually found
     for (const extension of this.extensions) {
@@ -66,7 +63,7 @@ export class CustomCompilerHost implements CompilerHost {
         this.log.silly('getSourceFile:', resolvedPathWithExt);
         text = fs.readFileSync(resolvedPathWithExt, { encoding: this.options.charset as BufferEncoding });
         this.log.debug('found source file:', fileName, resolvedPathWithExt);
-        return createSourceFile(baseFilePath + extension, text, languageVersion);
+        return createSourceFile(resolvedPathWithExt, text, languageVersion);
       } catch (e) {
         // Try again if the file simply did not exist, otherwise report the error as a warning
         if ((e as NodeJS.ErrnoException)?.code !== 'ENOENT') {

typescript/src/services/TsParser/FileInfo.ts

@@ -7,12 +7,12 @@ const path = require('canonical-path');
  * The file (and the location in the file) from where an API doc element was sourced.
  */
 export class FileInfo {
-  relativePath = this.declaration.getSourceFile().fileName;
   location = new Location(this.declaration);
-  filePath = path.resolve(this.basePath, this.relativePath);
+  filePath = path.resolve(this.basePath, this.declaration.getSourceFile().fileName);
   baseName = path.basename(this.filePath, path.extname(this.filePath));
   extension = path.extname(this.filePath).replace(/^\./, '');
-  projectRelativePath = path.relative(this.basePath, this.filePath);
+  relativePath = path.relative(this.basePath, this.filePath);
+  projectRelativePath = this.relativePath;
   realFilePath = this.getRealFilePath(this.filePath);
   realProjectRelativePath = path.relative(this.basePath, this.realFilePath);