improve source map granularity (#3030)

fixes #3023

5 files changed, 184 insertions, 181 deletions

diff --git a/lib/output.js b/lib/output.js
index f60b474c..7aef3b6a 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -1484,47 +1484,52 @@ function OutputStream(options) {
     /* -----[ source map generators ]----- */
 
     function DEFMAP(nodetype, generator) {
-        nodetype.DEFMETHOD("add_source_map", function(stream){
-            generator(this, stream);
+        nodetype.forEach(function(nodetype) {
+            nodetype.DEFMETHOD("add_source_map", generator);
         });
-    };
-
-    // We could easily add info for ALL nodes, but it seems to me that
-    // would be quite wasteful, hence this noop in the base class.
-    DEFMAP(AST_Node, noop);
+    }
 
-    function basic_sourcemap_gen(self, output) {
-        output.add_mapping(self.start);
-    };
+    DEFMAP([
+        // We could easily add info for ALL nodes, but it seems to me that
+        // would be quite wasteful, hence this noop in the base class.
+        AST_Node,
+        // since the label symbol will mark it
+        AST_LabeledStatement,
+        AST_Toplevel,
+    ], noop);
 
     // XXX: I'm not exactly sure if we need it for all of these nodes,
     // or if we should add even more.
-
-    DEFMAP(AST_Directive, basic_sourcemap_gen);
-    DEFMAP(AST_Debugger, basic_sourcemap_gen);
-    DEFMAP(AST_Symbol, basic_sourcemap_gen);
-    DEFMAP(AST_Jump, basic_sourcemap_gen);
-    DEFMAP(AST_StatementWithBody, basic_sourcemap_gen);
-    DEFMAP(AST_LabeledStatement, noop); // since the label symbol will mark it
-    DEFMAP(AST_Lambda, basic_sourcemap_gen);
-    DEFMAP(AST_Switch, basic_sourcemap_gen);
-    DEFMAP(AST_SwitchBranch, basic_sourcemap_gen);
-    DEFMAP(AST_BlockStatement, basic_sourcemap_gen);
-    DEFMAP(AST_Toplevel, noop);
-    DEFMAP(AST_New, basic_sourcemap_gen);
-    DEFMAP(AST_Try, basic_sourcemap_gen);
-    DEFMAP(AST_Catch, basic_sourcemap_gen);
-    DEFMAP(AST_Finally, basic_sourcemap_gen);
-    DEFMAP(AST_Definitions, basic_sourcemap_gen);
-    DEFMAP(AST_Constant, basic_sourcemap_gen);
-    DEFMAP(AST_ObjectSetter, function(self, output){
-        output.add_mapping(self.start, self.key.name);
-    });
-    DEFMAP(AST_ObjectGetter, function(self, output){
-        output.add_mapping(self.start, self.key.name);
-    });
-    DEFMAP(AST_ObjectProperty, function(self, output){
-        output.add_mapping(self.start, self.key);
+    DEFMAP([
+        AST_Array,
+        AST_BlockStatement,
+        AST_Catch,
+        AST_Constant,
+        AST_Debugger,
+        AST_Definitions,
+        AST_Directive,
+        AST_Finally,
+        AST_Jump,
+        AST_Lambda,
+        AST_New,
+        AST_Object,
+        AST_StatementWithBody,
+        AST_Symbol,
+        AST_Switch,
+        AST_SwitchBranch,
+        AST_Try,
+    ], function(output) {
+        output.add_mapping(this.start);
+    });
+
+    DEFMAP([
+        AST_ObjectGetter,
+        AST_ObjectSetter,
+     ], function(output) {
+        output.add_mapping(this.start, this.key.name);
+    });
+
+    DEFMAP([ AST_ObjectProperty ], function(output) {
+        output.add_mapping(this.start, this.key);
     });
-
 })();
diff --git a/test/mocha/minify.js b/test/mocha/minify.js
index 995897da..c8c17778 100644
--- a/test/mocha/minify.js
+++ b/test/mocha/minify.js
@@ -186,99 +186,6 @@ describe("minify", function() {
         });
     });
 
-    describe("inSourceMap", function() {
-        it("Should read the given string filename correctly when sourceMapIncludeSources is enabled (#1236)", function() {
-            var result = Uglify.minify(read("./test/input/issue-1236/simple.js"), {
-                sourceMap: {
-                    content: read("./test/input/issue-1236/simple.js.map"),
-                    filename: "simple.min.js",
-                    includeSources: true
-                }
-            });
-
-            var map = JSON.parse(result.map);
-
-            assert.equal(map.file, 'simple.min.js');
-            assert.equal(map.sourcesContent.length, 1);
-            assert.equal(map.sourcesContent[0],
-                'let foo = x => "foo " + x;\nconsole.log(foo("bar"));');
-        });
-        it("Should process inline source map", function() {
-            var code = Uglify.minify(read("./test/input/issue-520/input.js"), {
-                compress: { toplevel: true },
-                sourceMap: {
-                    content: "inline",
-                    url: "inline"
-                }
-            }).code + "\n";
-            assert.strictEqual(code, readFileSync("test/input/issue-520/output.js", "utf8"));
-        });
-        it("Should warn for missing inline source map", function() {
-            var warn_function = Uglify.AST_Node.warn_function;
-            var warnings = [];
-            Uglify.AST_Node.warn_function = function(txt) {
-                warnings.push(txt);
-            };
-            try {
-                var result = Uglify.minify(read("./test/input/issue-1323/sample.js"), {
-                    mangle: false,
-                    sourceMap: {
-                        content: "inline"
-                    }
-                });
-                assert.strictEqual(result.code, "var bar=function(bar){return bar};");
-                assert.strictEqual(warnings.length, 1);
-                assert.strictEqual(warnings[0], "inline source map not found");
-            } finally {
-                Uglify.AST_Node.warn_function = warn_function;
-            }
-        });
-        it("Should fail with multiple input and inline source map", function() {
-            var result = Uglify.minify([
-                read("./test/input/issue-520/input.js"),
-                read("./test/input/issue-520/output.js")
-            ], {
-                sourceMap: {
-                    content: "inline",
-                    url: "inline"
-                }
-            });
-            var err = result.error;
-            assert.ok(err instanceof Error);
-            assert.strictEqual(err.stack.split(/\n/)[0], "Error: inline source map only works with singular input");
-        });
-    });
-
-    describe("sourceMapInline", function() {
-        it("should append source map to output js when sourceMapInline is enabled", function() {
-            var result = Uglify.minify('var a = function(foo) { return foo; };', {
-                sourceMap: {
-                    url: "inline"
-                }
-            });
-            var code = result.code;
-            assert.strictEqual(code, "var a=function(n){return n};\n" +
-                "//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIjAiXSwibmFtZXMiOlsiYSIsImZvbyJdLCJtYXBwaW5ncyI6IkFBQUEsSUFBSUEsRUFBSSxTQUFTQyxHQUFPLE9BQU9BIn0=");
-        });
-        it("should not append source map to output js when sourceMapInline is not enabled", function() {
-            var result = Uglify.minify('var a = function(foo) { return foo; };');
-            var code = result.code;
-            assert.strictEqual(code, "var a=function(n){return n};");
-        });
-        it("should work with max_line_len", function() {
-            var result = Uglify.minify(read("./test/input/issue-505/input.js"), {
-                output: {
-                    max_line_len: 20
-                },
-                sourceMap: {
-                    url: "inline"
-                }
-            });
-            assert.strictEqual(result.error, undefined);
-            assert.strictEqual(result.code, read("./test/input/issue-505/output.js"));
-        });
-    });
-
     describe("#__PURE__", function() {
         it("should drop #__PURE__ hint after use", function() {
             var result = Uglify.minify('//@__PURE__ comment1 #__PURE__ comment2\n foo(), bar();', {
diff --git a/test/mocha/sourcemaps.js b/test/mocha/sourcemaps.js
new file mode 100644
index 00000000..7f4acb78
--- /dev/null
+++ b/test/mocha/sourcemaps.js
@@ -0,0 +1,141 @@
+var assert = require("assert");
+var readFileSync = require("fs").readFileSync;
+var Uglify = require("../../");
+
+function read(path) {
+    return readFileSync(path, "utf8");
+}
+
+function source_map(code) {
+    return JSON.parse(Uglify.minify(code, {
+        compress: false,
+        mangle: false,
+        sourceMap: true,
+    }).map);
+}
+
+describe("sourcemaps", function() {
+    it("Should give correct version", function() {
+        var map = source_map("var x = 1 + 1;");
+        assert.strictEqual(map.version, 3);
+        assert.deepEqual(map.names, [ "x" ]);
+    });
+
+    it("Should give correct names", function() {
+        var map = source_map([
+            "({",
+            "    get enabled() {",
+            "        return 3;",
+            "    },",
+            "    set enabled(x) {",
+            "        ;",
+            "    }",
+            "});",
+        ].join("\n"));
+        assert.deepEqual(map.names, [ "enabled", "x" ]);
+    });
+
+    it("Should mark array/object literals", function() {
+        var result = Uglify.minify([
+            "var obj = {};",
+            "obj.wat([]);",
+        ].join("\n"), {
+            sourceMap: true,
+        });
+        if (result.error) throw result.error;
+        assert.strictEqual(result.map, '{"version":3,"sources":["0"],"names":["obj","wat"],"mappings":"AAAA,IAAIA,IAAM,GACVA,IAAIC,IAAI"}');
+    });
+
+    describe("inSourceMap", function() {
+        it("Should read the given string filename correctly when sourceMapIncludeSources is enabled (#1236)", function() {
+            var result = Uglify.minify(read("./test/input/issue-1236/simple.js"), {
+                sourceMap: {
+                    content: read("./test/input/issue-1236/simple.js.map"),
+                    filename: "simple.min.js",
+                    includeSources: true
+                }
+            });
+
+            var map = JSON.parse(result.map);
+
+            assert.equal(map.file, 'simple.min.js');
+            assert.equal(map.sourcesContent.length, 1);
+            assert.equal(map.sourcesContent[0],
+                'let foo = x => "foo " + x;\nconsole.log(foo("bar"));');
+        });
+        it("Should process inline source map", function() {
+            var code = Uglify.minify(read("./test/input/issue-520/input.js"), {
+                compress: { toplevel: true },
+                sourceMap: {
+                    content: "inline",
+                    url: "inline"
+                }
+            }).code + "\n";
+            assert.strictEqual(code, readFileSync("test/input/issue-520/output.js", "utf8"));
+        });
+        it("Should warn for missing inline source map", function() {
+            var warn_function = Uglify.AST_Node.warn_function;
+            var warnings = [];
+            Uglify.AST_Node.warn_function = function(txt) {
+                warnings.push(txt);
+            };
+            try {
+                var result = Uglify.minify(read("./test/input/issue-1323/sample.js"), {
+                    mangle: false,
+                    sourceMap: {
+                        content: "inline"
+                    }
+                });
+                assert.strictEqual(result.code, "var bar=function(bar){return bar};");
+                assert.strictEqual(warnings.length, 1);
+                assert.strictEqual(warnings[0], "inline source map not found");
+            } finally {
+                Uglify.AST_Node.warn_function = warn_function;
+            }
+        });
+        it("Should fail with multiple input and inline source map", function() {
+            var result = Uglify.minify([
+                read("./test/input/issue-520/input.js"),
+                read("./test/input/issue-520/output.js")
+            ], {
+                sourceMap: {
+                    content: "inline",
+                    url: "inline"
+                }
+            });
+            var err = result.error;
+            assert.ok(err instanceof Error);
+            assert.strictEqual(err.stack.split(/\n/)[0], "Error: inline source map only works with singular input");
+        });
+    });
+
+    describe("sourceMapInline", function() {
+        it("should append source map to output js when sourceMapInline is enabled", function() {
+            var result = Uglify.minify('var a = function(foo) { return foo; };', {
+                sourceMap: {
+                    url: "inline"
+                }
+            });
+            var code = result.code;
+            assert.strictEqual(code, "var a=function(n){return n};\n" +
+                "//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIjAiXSwibmFtZXMiOlsiYSIsImZvbyJdLCJtYXBwaW5ncyI6IkFBQUEsSUFBSUEsRUFBSSxTQUFTQyxHQUFPLE9BQU9BIn0=");
+        });
+        it("should not append source map to output js when sourceMapInline is not enabled", function() {
+            var result = Uglify.minify('var a = function(foo) { return foo; };');
+            var code = result.code;
+            assert.strictEqual(code, "var a=function(n){return n};");
+        });
+        it("should work with max_line_len", function() {
+            var result = Uglify.minify(read("./test/input/issue-505/input.js"), {
+                output: {
+                    max_line_len: 20
+                },
+                sourceMap: {
+                    url: "inline"
+                }
+            });
+            assert.strictEqual(result.error, undefined);
+            assert.strictEqual(result.code, read("./test/input/issue-505/output.js"));
+        });
+    });
+});
diff --git a/test/run-tests.js b/test/run-tests.js
index 1b146e86..3ed28df5 100755
--- a/test/run-tests.js
+++ b/test/run-tests.js
@@ -22,9 +22,6 @@ if (failures) {
 var mocha_tests = require("./mocha.js");
 mocha_tests();
 
-var run_sourcemaps_tests = require('./sourcemaps');
-run_sourcemaps_tests();
-
 /* -----[ utils ]----- */
 
 function tmpl() {
@@ -49,16 +46,9 @@ function log_test(name) {
 }
 
 function find_test_files(dir) {
-    var files = fs.readdirSync(dir).filter(function(name){
+    return fs.readdirSync(dir).filter(function(name) {
         return /\.js$/i.test(name);
     });
-    if (process.argv.length > 2) {
-        var x = process.argv.slice(2);
-        files = files.filter(function(f){
-            return x.indexOf(f) >= 0;
-        });
-    }
-    return files;
 }
 
 function test_directory(dir) {
diff --git a/test/sourcemaps.js b/test/sourcemaps.js
deleted file mode 100644
index 2717eff4..00000000
--- a/test/sourcemaps.js
+++ /dev/null
@@ -1,40 +0,0 @@
-var UglifyJS = require("..");
-var ok = require("assert");
-
-module.exports = function () {
-    console.log("--- Sourcemaps tests");
-
-    var basic = source_map([
-        'var x = 1 + 1;'
-    ].join('\n'));
-
-    ok.equal(basic.version, 3);
-    ok.deepEqual(basic.names, ['x']);
-
-    var issue836 = source_map([
-        "({",
-        "    get enabled() {",
-        "        return 3;",
-        "    },",
-        "    set enabled(x) {",
-        "        ;",
-        "    }",
-        "});",
-    ].join("\n"));
-
-    ok.deepEqual(issue836.names, ['enabled', 'x']);
-}
-
-function source_map(js) {
-    return JSON.parse(UglifyJS.minify(js, {
-        compress: false,
-        mangle: false,
-        sourceMap: true
-    }).map);
-}
-
-// Run standalone
-if (module.parent === null) {
-    module.exports();
-}
-