fixes and tests for #3755

webpack · Jan 9, 2017 · 08ea58f · 08ea58f
1 parent 9380bd4
commit 08ea58f
Show file tree

Hide file tree

Showing 9 changed files with 153 additions and 40 deletions.
diff --git a/lib/ModuleDependencyError.js b/lib/ModuleDependencyError.js
@@ -2,19 +2,22 @@
 	MIT License http://www.opensource.org/licenses/mit-license.php
 	Author Tobias Koppers @sokra
 */
-var formatLocation = require("./formatLocation");
+"use strict";
 
-function ModuleDependencyError(module, err, loc) {
-	Error.call(this);
-	Error.captureStackTrace(this, ModuleDependencyError);
-	this.name = "ModuleDependencyError";
-	this.message = formatLocation(loc) + " ";
-	this.details = err.stack.split("\n").slice(1).join("\n");
-	this.message += err.message;
-	this.origin = this.module = module;
-	this.error = err;
-}
-module.exports = ModuleDependencyError;
+const formatLocation = require("./formatLocation");
+
+module.exports = class ModuleDependencyError extends Error {
+	constructor(module, err, loc) {
+		super();
 
-ModuleDependencyError.prototype = Object.create(Error.prototype);
-ModuleDependencyError.prototype.constructor = ModuleDependencyError;
+		if(Error.hasOwnProperty("captureStackTrace")) {
+			Error.captureStackTrace(this, this.constructor);
+		}
+		this.name = "ModuleDependencyError";
+
+		this.message = `${formatLocation(loc)} ${err.message}`;
+		this.details = err.stack.split("\n").slice(1).join("\n");
+		this.origin = this.module = module;
+		this.error = err;
+	}
+}
diff --git a/lib/ModuleDependencyWarning.js b/lib/ModuleDependencyWarning.js
@@ -2,19 +2,22 @@
 	MIT License http://www.opensource.org/licenses/mit-license.php
 	Author Tobias Koppers @sokra
 */
-var formatLocation = require("./formatLocation");
+"use strict";
 
-function ModuleDependencyWarning(module, err, loc) {
-	Error.call(this);
-	Error.captureStackTrace(this, ModuleDependencyWarning);
-	this.name = "ModuleDependencyWarning";
-	this.message = formatLocation(loc) + " ";
-	this.details = err.stack.split("\n").slice(1).join("\n");
-	this.message += err.message;
-	this.origin = this.module = module;
-	this.error = err;
-}
-module.exports = ModuleDependencyWarning;
+const formatLocation = require("./formatLocation");
+
+module.exports = class ModuleDependencyWarning extends Error {
+	constructor(module, err, loc) {
+		super();
 
-ModuleDependencyWarning.prototype = Object.create(Error.prototype);
-ModuleDependencyWarning.prototype.constructor = ModuleDependencyWarning;
+		if(Error.hasOwnProperty("captureStackTrace")) {
+			Error.captureStackTrace(this, this.constructor);
+		}
+		this.name = "ModuleDependencyWarning";
+
+		this.message = `${formatLocation(loc)} ${err.message}`;
+		this.details = err.stack.split("\n").slice(1).join("\n");
+		this.origin = this.module = module;
+		this.error = err;
+	}
+}
diff --git a/lib/dependencies/CommonJsInHarmonyWarning.js b/lib/dependencies/CommonJsInHarmonyWarning.js
@@ -2,13 +2,17 @@
 	MIT License http://www.opensource.org/licenses/mit-license.php
 	Author Tobias Koppers @sokra
 */
-function CommonJsInHarmonyWarning(name) {
-	Error.call(this);
-	Error.captureStackTrace(this, CommonJsInHarmonyWarning);
-	this.name = "CommonJsInHarmonyWarning";
-	this.message = name + " is not allowed in EcmaScript module: This module was detected as EcmaScript module (import or export syntax was used). In such a module using '" + name + "' is not allowed.";
-}
-module.exports = CommonJsInHarmonyWarning;
+"use strict";
+
+module.exports = class CommonJsInHarmonyWarning extends Error {
+	constructor(name) {
+		super();
 
-CommonJsInHarmonyWarning.prototype = Object.create(Error.prototype);
-CommonJsInHarmonyWarning.prototype.constructor = CommonJsInHarmonyWarning;
+		if(Error.hasOwnProperty("captureStackTrace")) {
+			Error.captureStackTrace(this, this.constructor);
+		}
+		this.name = "CommonJsInHarmonyWarning";
+
+		this.message = `${name} is not allowed in EcmaScript module: This module was detected as EcmaScript module (import or export syntax was used). In such a module using '${name}' is not allowed.`;
+	}
+}
diff --git a/lib/formatLocation.js b/lib/formatLocation.js
@@ -6,7 +6,7 @@ module.exports = function formatLocation(loc) {
 	if(typeof loc === "string")
 		return loc;
 	if(typeof loc === "number")
-		return loc;
+		return loc + "";
 	if(loc && typeof loc === "object") {
 		if(loc.start && loc.end) {
 			if(typeof loc.start.line === "number" && typeof loc.end.line === "number" && typeof loc.end.column === "number" && loc.start.line === loc.end.line)
@@ -23,15 +23,14 @@ module.exports = function formatLocation(loc) {
 		if(typeof pos === "string")
 			return pos;
 		if(typeof pos === "number")
-			return pos;
+			return pos + "";
 		if(pos && typeof pos === "object") {
 			if(typeof pos.line === "number" && typeof pos.column === "number")
 				return pos.line + ":" + pos.column;
 			if(typeof pos.line === "number")
 				return pos.line + ":?";
 			if(typeof pos.index === "number")
 				return "+" + pos.index;
-			return pos + "";
 		}
 		return "";
 	}

diff --git a/test/cases/parsing/harmony-commonjs-error/correct-module.js b/test/cases/parsing/harmony-commonjs-error/correct-module.js
@@ -0,0 +1,6 @@
+var module = 123;
+var define = 456;
+export default function(exports) {
+	exports.property = module;
+	define({});
+}
diff --git a/test/cases/parsing/harmony-commonjs-error/errors.js b/test/cases/parsing/harmony-commonjs-error/errors.js
@@ -2,5 +2,6 @@ module.exports = [
 	[/module/, /not allowed/],
 	[/module\.exports/, /not allowed/],
 	[/define/, /not allowed/],
+	[/define/, /not allowed/],
 	[/exports/, /not allowed/]
 ];
diff --git a/test/cases/parsing/harmony-commonjs-error/index.js b/test/cases/parsing/harmony-commonjs-error/index.js
@@ -1,4 +1,6 @@
 it("should result in a warning when using module.exports in harmony module", function() {
 	var x = require("./wrong-module");
 	x.should.be.eql(1234);
+	var y = require("./correct-module");
+	y.should.have.property("default").be.type("function");
 });
diff --git a/test/cases/parsing/harmony-commonjs-error/wrong-module.js b/test/cases/parsing/harmony-commonjs-error/wrong-module.js
@@ -5,4 +5,5 @@ module.exports = "abc";
 define([], function() {
 	return 1234
 });
+f(define);
 exports.property = true;
diff --git a/test/formatLocation.test.js b/test/formatLocation.test.js
@@ -0,0 +1,94 @@
+var should = require("should");
+var formatLocation = require("../lib/formatLocation");
+
+describe("formatLocation", () => {
+	var testCases = [{
+		name: "undefined",
+		loc: undefined,
+		result: ""
+	}, {
+		name: "null",
+		loc: null,
+		result: ""
+	}, {
+		name: "string",
+		loc: "str",
+		result: "str"
+	}, {
+		name: "number",
+		loc: 12,
+		result: "12"
+	}, {
+		name: "line-column",
+		loc: {
+			start: {
+				line: 1,
+				column: 2
+			},
+			end: {
+				line: 3,
+				column: 4
+			}
+		},
+		result: "1:2-3:4"
+	}, {
+		name: "line-column (same line)",
+		loc: {
+			start: {
+				line: 1,
+				column: 2
+			},
+			end: {
+				line: 1,
+				column: 4
+			}
+		},
+		result: "1:2-4"
+	}, {
+		name: "line-column (start only)",
+		loc: {
+			start: {
+				line: 5,
+				column: 6
+			}
+		},
+		result: "5:6"
+	}, {
+		name: "start-end string",
+		loc: {
+			start: "start",
+			end: "end"
+		},
+		result: "start-end"
+	}, {
+		name: "start-end number",
+		loc: {
+			start: 9,
+			end: 7
+		},
+		result: "9-7"
+	}, {
+		name: "line",
+		loc: {
+			start: {
+				line: 10
+			},
+			end: {
+				index: 20
+			}
+		},
+		result: "10:?-+20"
+	}, {
+		name: "line",
+		loc: {
+			start: null,
+			end: /f/
+		},
+		""
+	}];
+	testCases.forEach(testCase => {
+		it(`should format location correctly for ${testCase.name}`, () => {
+			formatLocation(testCase.loc).should.be.eql(testCase.result);
+		});
+	});
+});