Sourcemap corrections

clarify --sourceroot option meaning.

Correct relativePath to work for dist/js/filename case.
Add test.

Don't mark the placeholder parser source as input source

Fixes google#1685

Review URL: https://api.github.com/repos/google/traceur-compiler/issues/1750

Closes google#1750, closes google#1685.

.gitignore

@@ -24,12 +24,13 @@ src/syntax/trees/ParseTreeType.js
 src/syntax/trees/ParseTrees.js
 test/unit/codegeneration/PlaceholderParser.generated.js
 test/unit/tools/SourceMapMapping.generated.js
-test/unit/node/resources/compile-amd
-test/unit/node/resources/compile-cjs
-test/unit/node/resources/compile-cjs-maps
+test/unit/node/out/compile-amd
+test/unit/node/out/compile-cjs
+test/unit/node/out/compile-cjs-maps
 test/amd-compiled
 test/bench/esprima
 test/commonjs-compiled
 test/errsfile.json
 test/test-list.js
 test/wiki/CompilingOffline/out/*
+test/wiki/CompilingOffline/deepDirectory/dist/*

src/Compiler.js

@@ -220,7 +220,6 @@ export class Compiler {
           skipValidation: true
         }),
         basepath: basePath(outputName),
-        sourceRoot: sourceRoot,
         inputSourceMap: this.options_.inputSourceMap
       };
     }
@@ -320,7 +319,8 @@ export class Compiler {
       }
     }
     path = path || 'unamed.js';
-    return path.replace(/\.[^.]+$/, '.map');
+    path = path.split('/').pop();
+    return path + '.map';
   }
 
   sourceNameFromTree(tree) {

src/Options.js

@@ -523,7 +523,7 @@ export function addOptions(flags, commandOptions) {
     (to) => { return commandOptions.sourceMaps = to; }
   );
   flags.option('--source-root <true|false|string>',
-    'absolute path to source at execution time. false to omit, ' +
+    'sourcemap sourceRoot value. false to omit, ' +
         'true for directory of output file.',
     (to) => {
       if (to === 'false')

src/codegeneration/PlaceholderParser.js

@@ -140,8 +140,7 @@ function insertPlaceholderIdentifiers(sourceLiterals) {
 var counter = 0;
 
 function getParser(source, errorReporter) {
-  var file = new SourceFile(
-      '@traceur/generated/TemplateParser/' + counter++, source);
+  var file = new SourceFile(null, source);
   var options = new Options();
   // Enable internal code to always use all experimental features.
   options.experimental = true;

src/node/NodeCompiler.js

@@ -41,7 +41,10 @@ NodeCompiler.prototype = {
     if (this.options_.sourceMaps === 'file') {
       var sourcemap = this.getSourceMap();
       if (sourcemap) {
-        writeFile(this.sourceMappingURL(filename), sourcemap);
+        var mapName = this.sourceMappingURL(filename);
+        // Write the map file next to the output file.
+        mapName = path.resolve(path.dirname(filename), mapName);
+        writeFile(mapName, sourcemap);
       }
     }
 

src/node/recursiveModuleCompile.js

@@ -44,7 +44,6 @@ function recursiveModuleCompileToSingleFile(outputFile, includes, options) {
 
   mkdirRecursive(outputDir);
   process.chdir(outputDir);
-
   // Make includes relative to output dir so that sourcemap paths are correct.
   resolvedIncludes = resolvedIncludes.map(function(include) {
     include.name = normalizePath(path.relative(outputDir, include.name));

src/outputgeneration/ParseTreeMapWriter.js

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 import {ParseTreeWriter} from './ParseTreeWriter.js';
+import {StringSet} from '../util/StringSet.js';
 
 /**
  * Converts a ParseTree to text and a source Map.
@@ -25,11 +26,11 @@ export class ParseTreeMapWriter extends ParseTreeWriter {
   constructor(sourceMapConfiguration, options = undefined) {
     super(options);
     this.sourceMapGenerator_ = sourceMapConfiguration.sourceMapGenerator;
-    this.sourceRoot_ = sourceMapConfiguration.sourceRoot;
     this.lowResolution_ = sourceMapConfiguration.lowResolution;
     this.basepath_ = sourceMapConfiguration.basepath;
     this.outputLineCount_ = 1;
     this.isFirstMapping_ = true;
+    this.sourcesInMap_ = new StringSet();
   }
 
   //
@@ -116,11 +117,11 @@ export class ParseTreeMapWriter extends ParseTreeWriter {
       line: line,
       column: position.column || 0  // source map uses zero based columns
     };
-    if (position.source.name !== this.sourceName_) {
-      this.sourceName_ = position.source.name;
+    if (position.source.name && !this.sourcesInMap_.has(position.source.name)) {
+      this.sourcesInMap_.add(position.source.name);
       this.relativeSourceName_ = relativePath(position.source.name,
               this.basepath_);
-      this.sourceMapGenerator_.setSourceContent(position.source.name,
+      this.sourceMapGenerator_.setSourceContent(this.relativeSourceName_,
           position.source.contents);
     }
     this.flushMappings();
@@ -171,17 +172,21 @@ export function relativePath(name, sourceRoot) {
 
   var nameSegments = name.split('/');
   var rootSegments = sourceRoot.split('/');
-  // Handle dir name without /
+
   if (rootSegments[rootSegments.length - 1]) {
-    rootSegments.push('');
+    // We can't patch this up because we can't know whether the caller sent
+    // a file path or a directory w/o a slash by mistake
+    throw new Error('rootPath must end in /');
   }
   var commonSegmentsLength = 0;
   var uniqueSegments = [];
+  var foundUnique = false;
   nameSegments.forEach((segment, index)  => {
-    if (segment === rootSegments[index]) {
+    if (!foundUnique && segment === rootSegments[index]) {
       commonSegmentsLength++;
-      return false;
+      return;
     }
+    foundUnique = true;
     uniqueSegments.push(segment);
   });
 

test/unit/codegeneration/SourceMap.js

@@ -54,8 +54,10 @@ suite('SourceMap.js', function() {
     assert.equal(relativePath('/w/t/src/foo.js', '/w/t/src/'),
         'foo.js', 'same directory  ');
 
-    assert.equal(relativePath('/w/t/src/foo.js', '/w/t/out'),
-        '../src/foo.js', 'missing trailing slash');
+    assert.throws(() => relativePath('/w/t/src/foo.js', '/w/t/out'));
+
+    assert.equal(relativePath('/w/t/t/w/c/d/s/js/greeting.js',
+      '/w/t/t/w/c/d/d/js/'), '../../s/js/greeting.js', 'src/js bug');
   });
 
   test('SourceMap', function() {
@@ -67,7 +69,7 @@ suite('SourceMap.js', function() {
     var generatedLines = generatedSource.split('\n');
 
     // Check that the generated code did not change since we analyzed the map.
-    var expectedFilledColumnsZeroThrough = [16, 10, 0, 9, 0, -1, 37, -1];
+    var expectedFilledColumnsZeroThrough = [16, 10, 0, 9, 0, -1, 40, -1];
     generatedLines.forEach(function(line, index) {
       assert.equal(line.length - 1, expectedFilledColumnsZeroThrough[index]);
     });
@@ -106,7 +108,7 @@ suite('SourceMap.js', function() {
     var generatedLines = generatedSource.split('\n');
 
     // Check that the generated code did not change since we analyzed the map.
-    var expectedFilledColumnsZeroThrough = [16, 10, 0, 9, 0, -1, 37, -1];
+    var expectedFilledColumnsZeroThrough = [16, 10, 0, 9, 0, -1, 40, -1];
     generatedLines.forEach(function(line, index) {
       assert.equal(line.length - 1, expectedFilledColumnsZeroThrough[index]);
     });
@@ -165,7 +167,7 @@ suite('SourceMap.js', function() {
     var generatedLines = generatedSource.split('\n');
 
     // Check that the generated code did not change since we analyzed the map.
-    var expectedFilledColumnsZeroThrough = [15, 10, 0, 9, 0, -1, 37, -1];
+    var expectedFilledColumnsZeroThrough = [15, 10, 0, 9, 0, -1, 40, -1];
     generatedLines.forEach(function(line, index) {
       assert.equal(line.length - 1, expectedFilledColumnsZeroThrough[index]);
     });
@@ -209,7 +211,7 @@ suite('SourceMap.js', function() {
     var outFilename = 'out.js';
     var outFileContents = scriptCompiler.write(tree, outFilename);
 
-    var expected = 'alert(a);\nalert(b);\nalert(c);\n\n//# sourceMappingURL=out.map\n';
+    var expected = 'alert(a);\nalert(b);\nalert(c);\n\n//# sourceMappingURL=out.js.map\n';
     assert.equal(expected, outFileContents);
 
     var map = JSON.parse(scriptCompiler.getSourceMap(outFilename));
@@ -224,7 +226,7 @@ suite('SourceMap.js', function() {
     var tree = moduleCompiler.parse(src, filename);
     var actual = moduleCompiler.write(tree);
     // The sourceMappingURL should be relativel
-    assert.notEqual(actual.indexOf('//# sourceMappingURL=unnamed.map'), -1);
+    assert.notEqual(actual.indexOf('//# sourceMappingURL=unnamed.js.map'), -1);
 
     var sourceMap = moduleCompiler.getSourceMap(filename);
     assert.equal(JSON.parse(sourceMap).sources[0], filename);

test/unit/node/generated-code-dependencies.js

@@ -129,7 +129,7 @@ suite('context test', function() {
     exec(executable + ' --source-maps --out ' + tempFileName + ' -- ' + inputFilename,
         function(error, stdout, stderr) {
           assert.isNull(error);
-          tempMapName = tempFileName.replace('.js','') + '.map';
+          tempMapName = tempFileName + '.map';
           var map = fs.readFileSync(tempMapName, 'utf-8');
           assert(map, 'contains a source map');
           done();
@@ -148,10 +148,8 @@ suite('context test', function() {
       var m = /\/\/#\s*sourceMappingURL=(.*)/.exec(transcoded);
       assert(m, 'sourceMappingURL appears in the output');
       var sourceMappingURL = m[1];
-      var expected = forwardSlash(path.resolve(path.dirname(tempFileName),
-          'sourceroot-test.map'));
-      assert.equal(sourceMappingURL, expected);
-      tempMapName = tempFileName.replace('.js','') + '.map';
+      assert.equal(sourceMappingURL, 'sourceroot-test.js.map');
+      tempMapName = tempFileName + '.map';
       var map = JSON.parse(fs.readFileSync(tempMapName, 'utf-8'));
       var actualSourceRoot = map.sourceRoot;
       // Trailing slash as in the example,
@@ -186,10 +184,8 @@ suite('context test', function() {
       assert(m, 'sourceMappingURL appears in the output');
       var sourceMappingURL = m[1];
       var basepath = path.dirname(tempFileName);
-      var expected = forwardSlash(
-          path.resolve(basepath, 'sourceroot-test.map'));
-      assert.equal(sourceMappingURL, expected);
-      tempMapName = tempFileName.replace('.js','') + '.map';
+      assert.equal(sourceMappingURL, 'sourceroot-test.js.map');
+      tempMapName = tempFileName + '.map';
       var map = JSON.parse(fs.readFileSync(tempMapName, 'utf-8'));
       var actualSourceRoot = map.sourceRoot;
       // Trailing slash as in the example,
@@ -223,10 +219,8 @@ suite('context test', function() {
       assert(m, 'sourceMappingURL appears in the output');
       var sourceMappingURL = m[1];
       var basepath = path.dirname(tempFileName);
-      var expected = forwardSlash(
-          path.resolve(basepath, 'sourceroot-test.map'));
-      assert.equal(sourceMappingURL, expected);
-      tempMapName = tempFileName.replace('.js','') + '.map';
+      assert.equal(sourceMappingURL, 'sourceroot-test.js.map');
+      tempMapName = tempFileName + '.map';
       var map = JSON.parse(fs.readFileSync(tempMapName, 'utf-8'));
       var actualSourceRoot = map.sourceRoot;
       // Trailing slash as in the example,
@@ -275,6 +269,49 @@ suite('context test', function() {
     });
   });
 
+  test('sourceMappingURL from --source-maps=true and deep directories', function(done){
+    var deepDirectory = 'test/wiki/CompilingOffline/deepDirectory/';
+    var pwd = process.cwd();
+    process.chdir(deepDirectory);
+    var executable = '../../../../traceur';
+    var inputFilename = './src/js/app.js';
+    var cmd = executable + ' --source-maps ' +
+        '--source-root=false --out ./dist/js/bundle.js ' + inputFilename;
+    var child = exec(cmd, function(error, stdout, stderr) {
+      assert.isNull(error);
+      process.chdir(pwd);
+      var tempFileName = resolve(deepDirectory + '/dist/js/bundle.js');
+      var transcoded = fs.readFileSync(tempFileName, 'utf-8');
+      var m = /\/\/#\s*sourceMappingURL=(.*)/.exec(transcoded);
+      assert(m, 'sourceMappingURL appears in the output');
+      var sourceMappingURL = m[1];
+      var basepath = path.dirname(tempFileName);
+      var expected = 'bundle.js.map';
+      assert.equal(sourceMappingURL, expected);
+      // The map file should be in the same dir as the output file, with
+      // the name given in the URL
+      tempMapName = path.resolve(path.dirname(tempFileName), sourceMappingURL);
+      var map = JSON.parse(fs.readFileSync(tempMapName, 'utf-8'));
+      var actualSourceRoot = map.sourceRoot;
+      // Trailing slash as in the example,
+      // https://github.com/mozilla/source-map
+      assert.equal(undefined, actualSourceRoot, 'has undefined sourceroot');
+      var absoluteInputFilename = path.resolve(deepDirectory, inputFilename);
+      var inputRelativeToOutput =
+          path.relative(path.dirname(tempFileName), absoluteInputFilename);
+      var foundInput = map.sources.some(function(name) {
+        if (inputRelativeToOutput === name) {
+          // The 'sources' entry is relative
+          assert.equal(name.indexOf('.'), 0);
+          return true;
+        }
+      });
+      assert(foundInput, 'the inputFilename '+ inputRelativeToOutput +
+          ' is one of the sourcemap sources ' + map.sources.join(','));
+      done();
+    });
+  });
+
   test('compiled modules sourcemaps=memory', function(done) {
     var inputFilename = resolve('test/unit/runtime/resources/CallsThrowsError.js');
     var executable = 'node ' + resolve('src/node/command.js');
@@ -441,7 +478,7 @@ suite('context test', function() {
     var executable = 'node ' + resolve('src/node/command.js');
     var inputDir = './test/unit/node/resources/compile-dir';
     var goldenDir = './test/unit/node/resources/golden-amd';
-    var outDir = './test/unit/node/resources/compile-amd';
+    var outDir = './test/unit/node/out/compile-amd';
     var cmd = executable + ' --dir ' + inputDir + ' ' + outDir + ' --modules=amd';
     exec(cmd, function(error, stdout, stderr) {
       assert.isNull(error);
@@ -455,7 +492,7 @@ suite('context test', function() {
     var executable = 'node ' + resolve('src/node/command.js');
     var inputDir = './test/unit/node/resources/compile-dir';
     var goldenDir = './test/unit/node/resources/golden-cjs';
-    var outDir = './test/unit/node/resources/compile-cjs';
+    var outDir = './test/unit/node/out/compile-cjs';
     exec(executable + ' --dir ' + inputDir + ' ' + outDir + ' --modules=commonjs', function(error, stdout, stderr) {
       assert.isNull(error);
       checkFile(outDir, goldenDir, 'file.js');
@@ -513,14 +550,14 @@ suite('context test', function() {
   test('compile module dir option CommonJS with source-maps', function(done) {
     var executable = 'node ' + resolve('src/node/command.js');
     var inputDir = './test/unit/node/resources/compile-dir';
-    var outDir = './test/unit/node/resources/compile-cjs-maps';
+    var outDir = './test/unit/node/out/compile-cjs-maps';
     var cmd = executable + ' --source-maps=file --dir ' + inputDir + ' ' + outDir + ' --modules=commonjs';
     exec(cmd, function(error, stdout, stderr) {
       assert.isNull(error);
-      var fileContents = fs.readFileSync(path.resolve(outDir, 'file.map'));
-      var depContents = fs.readFileSync(path.resolve(outDir, 'dep.map'));
-      assert(fileContents + '');
-      assert(depContents + '')
+      var fileMapContents = fs.readFileSync(path.resolve(outDir, 'file.js.map'));
+      var depMapContents = fs.readFileSync(path.resolve(outDir, 'dep.js.map'));
+      assert(fileMapContents + '');
+      assert(depMapContents + '')
       done();
     });
   });
@@ -533,4 +570,5 @@ suite('context test', function() {
       done();
     });
   });
+
 });

test/wiki/CompilingOffline/deepDirectory/src/js/app.js

@@ -0,0 +1,4 @@
+import {Greeter} from './greeter.js';
+
+var greeter = new Greeter();
+greeter.sayHi();

test/wiki/CompilingOffline/deepDirectory/src/js/greeter.js

@@ -0,0 +1,4 @@
+// greeter.js
+export class Greeter {
+  sayHi() { console.log('Hi!'); }
+}