fix: Map unique files once, regardless of path separator (#287)

* Map unique files once, regardless of path separator
* Use reduce instead of forEach
* Reduce the amount of duplicated test data
* Convert var to const
* Replace one-shot factory functions with const objects
* Make the tests behave the same way on windows and *nix

packages/istanbul-lib-source-maps/lib/transformer.js

@@ -234,12 +234,17 @@ SourceMapTransformer.prototype.processFile = function(
 SourceMapTransformer.prototype.transform = function(coverageMap) {
     var that = this,
         finder = this.finder,
-        output = {},
+        uniqueFiles = {},
+        key,
         getMappedCoverage = function(file) {
-            if (!output[file]) {
-                output[file] = new MappedCoverage(file);
+            key = getUniqueKey(file);
+            if (!uniqueFiles[key]) {
+                uniqueFiles[key] = {
+                    file: file,
+                    mappedCoverage: new MappedCoverage(file)
+                };
             }
-            return output[file];
+            return uniqueFiles[key].mappedCoverage;
         };
 
     coverageMap.files().forEach(function(file) {
@@ -248,7 +253,10 @@ SourceMapTransformer.prototype.transform = function(coverageMap) {
             changed;
 
         if (!sourceMap) {
-            output[file] = fc;
+            uniqueFiles[getUniqueKey(file)] = {
+                file: file,
+                mappedCoverage: fc
+            };
             return;
         }
 
@@ -257,9 +265,22 @@ SourceMapTransformer.prototype.transform = function(coverageMap) {
             debug('File [' + file + '] ignored, nothing could be mapped');
         }
     });
-    return libCoverage.createCoverageMap(output);
+    return libCoverage.createCoverageMap(getOutput(uniqueFiles));
 };
 
+function getUniqueKey(path) {
+    return path.replace(/[\\/]/g, '_');
+}
+
+function getOutput(cache) {
+    return Object.keys(cache).reduce((output, key) => {
+        const item = cache[key];
+        return Object.assign(output, {
+            [item.file]: item.mappedCoverage
+        });
+    }, {});
+}
+
 module.exports = {
     create: function(finder, opts) {
         return new SourceMapTransformer(finder, opts);

packages/istanbul-lib-source-maps/test/transformer.test.js

@@ -1,83 +1,89 @@
 /* globals describe, it */
-var assert = require('chai').assert,
-    isWindows = require('is-windows'),
-    createMap = require('istanbul-lib-coverage').createCoverageMap,
-    SMC = require('source-map').SourceMapConsumer,
-    createTransformer = require('../lib/transformer').create;
+const path = require('path');
+const assert = require('chai').assert;
+const createMap = require('istanbul-lib-coverage').createCoverageMap;
+const SMC = require('source-map').SourceMapConsumer;
+const createTransformer = require('../lib/transformer').create;
 
-function createData() {
-    var sourceMap = {
+const coverageData = {
+    statementMap: {
+        '0': {
+            start: { line: 2, column: 0 },
+            end: { line: 2, column: 29 }
+        },
+        '1': {
+            start: { line: 3, column: 0 },
+            end: { line: 3, column: 47 }
+        }
+    },
+    fnMap: {},
+    branchMap: {},
+    s: {
+        '0': 0,
+        '1': 0,
+        '2': 0
+    },
+    f: {},
+    b: {}
+};
+
+const sourceFileSlash = path.posix.normalize('/path/to/file.js');
+const sourceFileBackslash = path.win32.normalize('/path/to/file.js');
+
+const testDataSlash = {
+    sourceMap: {
         version: 3,
-        sources: ['file.js'],
+        sources: [sourceFileSlash],
         mappings: ';AAAa,mBAAW,GAAG,MAAM,CAAC;AACrB,kBAAU,GAAG,yBAAyB,CAAC'
-    };
+    },
+    coverageData: Object.assign({}, coverageData, {
+        path: sourceFileSlash
+    })
+};
 
-    var coverageData = {
-        path: '/path/to/file.js',
-        statementMap: {
-            '0': {
-                start: {
-                    line: 2,
-                    column: 0
-                },
-                end: {
-                    line: 2,
-                    column: 29
-                }
-            },
-            '1': {
-                start: {
-                    line: 3,
-                    column: 0
+const testDataBackslash = {
+    coverageData: Object.assign({}, coverageData, {
+        path: sourceFileBackslash
+    })
+};
+
+describe('transformer', function() {
+    it('maps statement locations', function() {
+        const coverageMap = createMap({});
+        coverageMap.addFileCoverage(testDataSlash.coverageData);
+
+        const mapped = createTransformer(function() {
+            return new SMC(testDataSlash.sourceMap);
+        }).transform(coverageMap);
+
+        assert.deepEqual(
+            mapped.data[testDataSlash.coverageData.path].statementMap,
+            {
+                '0': {
+                    start: { line: 1, column: 13 },
+                    end: { line: 1, column: 34 }
                 },
-                end: {
-                    line: 3,
-                    column: 47
+                '1': {
+                    start: { line: 2, column: 13 },
+                    end: { line: 2, column: 52 }
                 }
             }
-        },
-        fnMap: {},
-        branchMap: {},
-        s: {
-            '0': 0,
-            '1': 0,
-            '2': 0
-        },
-        f: {},
-        b: {}
-    };
-
-    return {
-        sourceMap: sourceMap,
-        coverageData: coverageData
-    };
-}
+        );
+    });
 
-describe('transformer', function() {
-    it('maps statement locations', function() {
-        if (isWindows()) {
-            return this.skip();
-        }
+    it('maps each file only once, /path/to/file.js and \\path\\to\\file.js are the same file', function() {
+        const coverageMap = createMap({});
 
-        var coverageMap = createMap({}),
-            testData = createData(),
-            coverageData = testData.coverageData,
-            sourceMap = testData.sourceMap;
+        coverageMap.addFileCoverage(testDataSlash.coverageData);
+        coverageMap.addFileCoverage(testDataBackslash.coverageData);
 
-        coverageMap.addFileCoverage(coverageData);
-        var mapped = createTransformer(function() {
-            return new SMC(sourceMap);
+        const mapped = createTransformer(function(file) {
+            return file === testDataSlash.coverageData.path
+                ? new SMC(testDataSlash.sourceMap)
+                : undefined;
         }).transform(coverageMap);
 
-        assert.deepEqual(mapped.data[coverageData.path].statementMap, {
-            '0': {
-                start: { line: 1, column: 13 },
-                end: { line: 1, column: 34 }
-            },
-            '1': {
-                start: { line: 2, column: 13 },
-                end: { line: 2, column: 52 }
-            }
-        });
+        assert.equal(Object.keys(mapped.data).length, 1);
+        assert.isDefined(mapped.data[testDataBackslash.coverageData.path]);
     });
 });