Fix overwriting existing sourcesContent in sourcemaps

[Jimbly]

When running Uglify on already processed code, when given a sourcemap that includes a sourcesContent with the original code, it's generating a sourcemap which instead references the intermediate source content.

As an example, running uglify on this file from Babel generates this bad output, but when fixed generates this good output

Uglify command line used to test: uglifyjs --source-map "includeSources,content=inline,url=inline" in.js

It looks like Uglify only does the wrong thing if the intermediate file is named the same as the original file - if the original file has a different name, Uglify correctly grabs the original provided sourcemap for that file, but if they're the same name, it blows away the provided sourcemap.

[alexlamsl]

Thanks for the detailed report and code contribution. Please include the following mocha test to avoid future regression:

--- a/test/mocha/sourcemaps.js
+++ b/test/mocha/sourcemaps.js
@@ -244,6 +244,39 @@ describe("sourcemaps", function() {
             assert.strictEqual(result.code, '(function(){console.log("hello")}).call(this);');
             assert.strictEqual(result.map, '{"version":3,"sources":["main.coffee"],"names":["console","log"],"mappings":"CAAA,WAAAA,QAAQC,IAAI"}');
         });
+        it("Should not overwrite existing sourcesContent", function() {
+            var result = UglifyJS.minify({
+                "in.js": [
+                    '"use strict";',
+                    "",
+                    "var _window$foo = window.foo,",
+                    "    a = _window$foo[0],",
+                    "    b = _window$foo[1];",
+                ].join("\n"),
+            }, {
+                compress: false,
+                mangle: false,
+                sourceMap: {
+                    content: {
+                        version: 3,
+                        sources: [ "in.js" ],
+                        names: [
+                            "window",
+                            "foo",
+                            "a",
+                            "b",
+                        ],
+                        mappings: ";;kBAAaA,MAAM,CAACC,G;IAAfC,C;IAAGC,C",
+                        file: "in.js",
+                        sourcesContent: [ "let [a, b] = window.foo;\n" ],
+                    },
+                    includeSources: true,
+                },
+            });
+            if (result.error) throw result.error;
+            assert.strictEqual(result.code, '"use strict";var _window$foo=window.foo,a=_window$foo[0],b=_window$foo[1];');
+            assert.strictEqual(result.map, '{"version":3,"sources":["in.js"],"sourcesContent":["let [a, b] = window.foo;\\n"],"names":["window","foo","a","b"],"mappings":"6BAAaA,OAAOC,IAAfC,E,eAAGC,E"}');
+        });
     });

     describe("sourceMapInline", function() {

[alexlamsl]

Admittedly an odd case, but this will still overwrite an existing blank source file − please consider the following instead:

if (!(source in sources_content)) sources_content[source] = content;

[Jimbly]

Sounds good! For running on very old Node/browsers, would if (!HOP(sources_content, source)) be better? Or doesn't matter anymore and prefer readability at this point?

[alexlamsl]

sources_content is initiated to Object.create(null), so in theory you won't get any inherited properties, i.e. in this particular case source in sources_content is equivalent to HOP(sources_content, source) on all platforms

[Jimbly]

Ah, I missed the Object.create(null), thought this was a caller-provided map, you're right! I've changed the check to source in sources_content.

[alexlamsl]

Thanks for the changes − LGTM 👍