Fixed bugs that occured when transpiling files in other directories (d…

…art-archive#71) The paths for library files weren't being resolved properly when they started with ../ or ./ so TS names weren't being converted to Dart properly (ex. Array and List) Files weren't being transpiled if their paths began with ./ because SourceFile.fileName strips leading ./ The generator was already writing files to subdirectories but was still generating imports as if all files were being output to the same directory
derekxu16 · Nov 27, 2019 · ea26154 · ea26154
1 parent 7c8962b
commit ea26154
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 43 deletions.
diff --git a/index.js b/index.js
@@ -3,11 +3,12 @@
 const main = require('./build/lib/main.js');
 
 var args = require('minimist')(process.argv.slice(2), {
-  base: 'string',
+  string: ['base-path'],
   boolean: [
     'semantic-diagnostics', 'generate-html', 'rename-conflicting-types', 'explicit-static',
     'trust-js-types'
   ],
+  default: {'base-path': ''},
   alias: {
     'base-path': 'basePath',
     'semantic-diagnostics': 'semanticDiagnostics',

diff --git a/lib/base.ts b/lib/base.ts
@@ -443,25 +443,26 @@ export class TranspilerBase {
   }
 
   /**
-   * Return resolved named possibly including a prefix for the identifier.
+   * Return resolved name possibly including a prefix for the identifier.
    */
   resolveImportForSourceFile(sourceFile: ts.SourceFile, context: ts.SourceFile, identifier: string):
       string {
-    if (sourceFile === context) return identifier;
+    if (sourceFile === context) {
+      return identifier;
+    }
     if (sourceFile.hasNoDefaultLib) {
       // We don't want to emit imports to default lib libraries as we replace with Dart equivalents
       // such as dart:html, etc.
       return identifier;
     }
-    let relativePath = path.relative(path.dirname(context.fileName), sourceFile.fileName);
-    // TODO(jacobr): actually handle imports in different directories. We assume for now that all
-    // files in a library will be output to a single directory for codegen simplicity.
-    let parts = this.getDartFileName(relativePath).split('/');
-    let fileName = parts[parts.length - 1];
-    let identifierParts = identifier.split('.');
+    const relativePath = path.relative(path.dirname(context.fileName), sourceFile.fileName);
+    const fileName = this.getDartFileName(relativePath);
+    const identifierParts = identifier.split('.');
     identifier = identifierParts[identifierParts.length - 1];
-    let summary = this.addImport(this.transpiler.getDartFileName(fileName), identifier);
-    if (summary.asPrefix) return summary.asPrefix + '.' + identifier;
+    const summary = this.addImport(this.transpiler.getDartFileName(fileName), identifier);
+    if (summary.asPrefix) {
+      return summary.asPrefix + '.' + identifier;
+    }
     return identifier;
   }
 

diff --git a/lib/dart_libraries_for_browser_types.ts b/lib/dart_libraries_for_browser_types.ts
@@ -974,7 +974,7 @@ const STDLIB_TYPE_REPLACEMENTS = new Map(Object.entries({
 }));
 
 export const TS_TO_DART_TYPENAMES = new Map(Object.entries({
-  'typescript/lib/lib.es5': STDLIB_TYPE_REPLACEMENTS,
-  'typescript/lib/lib.es6': STDLIB_TYPE_REPLACEMENTS,
-  'typescript/lib/lib.dom': STDLIB_TYPE_REPLACEMENTS
+  'lib.es5': STDLIB_TYPE_REPLACEMENTS,
+  'lib.es6': STDLIB_TYPE_REPLACEMENTS,
+  'lib.dom': STDLIB_TYPE_REPLACEMENTS
 }));
diff --git a/lib/declaration.ts b/lib/declaration.ts
@@ -460,7 +460,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase {
         const moduleDecl = <ts.ModuleDeclaration>node;
         const moduleName = base.getModuleName(moduleDecl);
         const moduleBlock = base.getModuleBlock(moduleDecl);
-        if (moduleName.slice(0, 2) === '..') {
+        if (moduleName.startsWith('..')) {
           this.emit(
               '\n// Library augmentation not allowed by Dart. Ignoring augmentation of ' +
               moduleDecl.name.text + '\n');

diff --git a/lib/facade_converter.ts b/lib/facade_converter.ts
@@ -1,3 +1,4 @@
+import {relative} from 'path';
 import * as ts from 'typescript';
 
 import * as base from './base';
@@ -288,7 +289,7 @@ export class FacadeConverter extends base.TranspilerBase {
   }
 
   private extractPropertyNames(m: Map<string, Map<string, string>>, candidates: Set<string>) {
-    for (let fileName of m.keys()) {
+    for (const fileName of m.keys()) {
       const file = m.get(fileName);
       if (file === undefined) {
         return;
@@ -739,12 +740,12 @@ export class FacadeConverter extends base.TranspilerBase {
         return null;
       }
 
-      const fileAndName = this.getFileAndName(identifier, symbol);
+      const fileAndName = this.getLibFileAndName(identifier, symbol);
 
       if (fileAndName) {
         let fileSubs = TS_TO_DART_TYPENAMES.get(fileAndName.fileName);
         if (fileSubs) {
-          let name = fileAndName.qname;
+          const name = fileAndName.qname;
           let dartBrowserType = DART_LIBRARIES_FOR_BROWSER_TYPES.has(name);
           if (fileSubs.has(name)) {
             let subName = fileSubs.get(name);
@@ -979,14 +980,16 @@ export class FacadeConverter extends base.TranspilerBase {
         */
   }
 
-  private getFileAndName(n: ts.Node, originalSymbol: ts.Symbol): {fileName: string, qname: string} {
+  private getLibFileAndName(n: ts.Node, originalSymbol: ts.Symbol):
+      {fileName: string, qname: string} {
     let symbol = originalSymbol;
-    while (symbol.flags & ts.SymbolFlags.Alias) symbol = this.tc.getAliasedSymbol(symbol);
+    while (symbol.flags & ts.SymbolFlags.Alias) {
+      symbol = this.tc.getAliasedSymbol(symbol);
+    }
     const decl = this.getTypeDeclarationOfSymbol(symbol, n);
 
-    const fileName = decl.getSourceFile().fileName;
-    const canonicalFileName = this.getRelativeFileName(fileName)
-                                  .replace(/(\.d)?\.ts$/, '')
+    const fileName = relative('./node_modules/typescript/lib', decl.getSourceFile().fileName);
+    const canonicalFileName = fileName.replace(/(\.d)?\.ts$/, '')
                                   .replace(FACADE_NODE_MODULES_PREFIX, '')
                                   .replace(this.typingsRootRegex, '');
 
@@ -996,7 +999,10 @@ export class FacadeConverter extends base.TranspilerBase {
     if (symbol.flags & (ts.SymbolFlags.Class | ts.SymbolFlags.Function | ts.SymbolFlags.Variable)) {
       qname = symbol.getName();
     }
-    if (FACADE_DEBUG) console.error('fn:', fileName, 'cfn:', canonicalFileName, 'qn:', qname);
+
+    if (FACADE_DEBUG) {
+      console.error('fn:', fileName, 'cfn:', canonicalFileName, 'qn:', qname);
+    }
     return {fileName: canonicalFileName, qname};
   }
 }
diff --git a/lib/main.ts b/lib/main.ts
@@ -114,8 +114,7 @@ export class Transpiler {
   /* Number of nested levels of type arguments the current expression is within. */
   private typeArgumentDepth = 0;
 
-  constructor(private options: TranspilerOptions) {
-    this.options = this.options || {};
+  constructor(private options: TranspilerOptions = {}) {
     this.fc = new FacadeConverter(this, options.typingsRoot, options.generateHTML);
     this.declarationTranspiler = new DeclarationTranspiler(
         this, this.fc, options.enforceUnderscoreConventions, options.promoteFunctionLikeMembers,
@@ -130,13 +129,20 @@ export class Transpiler {
   /**
    * Transpiles the given files to Dart.
    * @param fileNames The input files.
-   * @param destination Location to write files to. Creates files next to their sources if absent.
+   * @param destination Location to write files to. Outputs file contents to stdout if absent.
    */
   transpile(fileNames: string[], destination?: string): void {
     if (this.options.basePath) {
       this.options.basePath = this.normalizeSlashes(path.resolve(this.options.basePath));
     }
-    fileNames = fileNames.map((f) => this.normalizeSlashes(f));
+    fileNames = fileNames.map((name: string) => {
+      const normalizedName = this.normalizeSlashes(name);
+      if (normalizedName.startsWith('./')) {
+        // The fileName property of SourceFiles omits ./ for files in the current directory
+        return normalizedName.substring(2);
+      }
+      return normalizedName;
+    });
     const host = this.createCompilerHost();
     const program = ts.createProgram(fileNames, this.getCompilerOptions(), host);
     this.fc.setTypeChecker(program.getTypeChecker());
@@ -230,11 +236,10 @@ export class Transpiler {
   }
 
   getCompilerOptions() {
-    let opts: ts.CompilerOptions = {};
-    for (let k of Object.keys(COMPILER_OPTIONS)) opts[k] = COMPILER_OPTIONS[k];
+    const opts: ts.CompilerOptions = Object.assign({}, COMPILER_OPTIONS);
     opts.rootDir = this.options.basePath;
     if (this.options.generateHTML) {
-      // Prevent the TypeScript DOM library files from being compiled
+      // Prevent the TypeScript DOM library files from being compiled.
       opts.lib = ['lib.es2015.d.ts', 'lib.scripthost.d.ts'];
     }
     return opts;
@@ -243,22 +248,22 @@ export class Transpiler {
   private createCompilerHost(): ts.CompilerHost {
     const compilerOptions = this.getCompilerOptions();
     const compilerHost = ts.createCompilerHost(compilerOptions);
-    let defaultLibFileName = ts.getDefaultLibFileName(compilerOptions);
-    defaultLibFileName = this.normalizeSlashes(defaultLibFileName);
+    const defaultLibFileName = this.normalizeSlashes(ts.getDefaultLibFileName(compilerOptions));
     compilerHost.getSourceFile = (sourceName) => {
       let sourcePath = sourceName;
       if (sourceName === defaultLibFileName) {
         sourcePath = ts.getDefaultLibFilePath(compilerOptions);
       }
-      if (!fs.existsSync(sourcePath)) return undefined;
-      let contents = fs.readFileSync(sourcePath, 'utf-8');
+      if (!fs.existsSync(sourcePath)) {
+        return undefined;
+      }
+      const contents = fs.readFileSync(sourcePath, 'utf-8');
       return ts.createSourceFile(sourceName, contents, COMPILER_OPTIONS.target, true);
     };
     compilerHost.writeFile = (name, text, writeByteOrderMark) => {
       fs.writeFile(name, text, undefined);
     };
     compilerHost.useCaseSensitiveFileNames = () => true;
-    compilerHost.getCanonicalFileName = (filename) => filename;
     compilerHost.getCurrentDirectory = () => '';
     compilerHost.getNewLine = () => '\n';
     compilerHost.resolveModuleNames = getModuleResolver(compilerHost);
@@ -268,7 +273,7 @@ export class Transpiler {
 
   // Visible for testing.
   getOutputPath(filePath: string, destinationRoot: string): string {
-    let relative = this.getDartFileName(filePath);
+    const relative = this.getDartFileName(filePath);
     return this.normalizeSlashes(path.join(destinationRoot, relative));
   }
 
@@ -380,20 +385,23 @@ export class Transpiler {
    * @param filePath Optional path to relativize, defaults to the current file's path.
    */
   getRelativeFileName(filePath?: string): string {
-    if (filePath === undefined) filePath = path.resolve(this.currentFile.fileName);
-    // TODO(jacobr): Use path.isAbsolute on node v0.12.
-    if (this.normalizeSlashes(path.resolve('/x/', filePath)) !== filePath) {
+    if (filePath === undefined) {
+      filePath = path.resolve(this.currentFile.fileName);
+    }
+    if (!path.isAbsolute(filePath)) {
       return filePath;  // already relative.
     }
-    let basePath = this.options.basePath || '';
+    const basePath = this.options.basePath || '';
     if (filePath.indexOf(basePath) !== 0 && !filePath.match(/\.d\.ts$/)) {
       throw new Error(`Files must be located under base, got ${filePath} vs ${basePath}`);
     }
     return this.normalizeSlashes(path.relative(basePath, filePath));
   }
 
   getDartFileName(filePath?: string): string {
-    if (filePath === undefined) filePath = path.resolve(this.currentFile.fileName);
+    if (filePath === undefined) {
+      filePath = path.resolve(this.currentFile.fileName);
+    }
     filePath = this.normalizeSlashes(filePath);
     filePath = filePath.replace(/\.(js|es6|d\.ts|ts)$/, '.dart');
     // Normalize from node module file path pattern to

diff --git a/test/facade_converter_test.ts b/test/facade_converter_test.ts
@@ -182,7 +182,14 @@ external List<String> /*ReadonlyArray<String>*/ f();`);
 
   describe('error detection', () => {
     it('supports imports', () => {
-      expectWithTypes(`import {X} from "other";\nlet x:X;`).to.equal(`import "other.dart" show X;
+      // In all tests, the main code has a fake location of FAKE_MAIN, which is declared
+      // to be '/demo/some/main.ts' within test_support.ts. At the top of this file, the 'other'
+      // module is declared to have a fake path of '/other.ts'. So, the correct import path for the
+      // other module is '../../other.dart'
+      expectWithTypes(`
+import {X} from "other";
+declare let x:X;
+`).to.equal(`import "../../other.dart" show X;
 
 @JS()
 external X get x;