From a4932e0e0f32bcc2d69f33da422854674e7a5313 Mon Sep 17 00:00:00 2001
From: Sebastian McKenzie <sebmck@gmail.com>
Date: Wed, 11 Feb 2015 15:57:30 +1100
Subject: [PATCH] add messages to make it easier to make error messages

---
 lib/6to5/messages.js                          | 48 +++++++++++++++++++
 .../helpers/build-react-transformer.js        |  3 +-
 .../transformation/helpers/replace-supers.js  |  7 +--
 lib/6to5/transformation/modules/_default.js   | 12 ++---
 .../transformers/es6/classes.js               |  3 +-
 .../transformers/es6/constants.js             |  5 +-
 .../transformers/es6/destructuring.js         |  5 +-
 .../transformation/transformers/es6/for-of.js |  9 ++--
 .../playground/mallet-operator.js             |  7 +--
 .../validation/no-for-in-of-assignment.js     |  5 +-
 .../transformers/validation/setters.js        |  4 +-
 .../validation/undeclared-variable-check.js   |  6 ++-
 lib/6to5/traversal/scope.js                   |  3 +-
 .../errors/constants/options.json             |  2 +-
 .../options.json                              |  2 +-
 .../destructuring-assignment/options.json     |  2 +-
 .../es6-constants/no-assignment/options.json  |  2 +-
 .../es6-constants/no-classes/options.json     |  2 +-
 .../es6-constants/no-declaration/options.json |  2 +-
 .../es6-constants/no-functions/options.json   |  2 +-
 .../update-expression/options.json            |  2 +-
 .../es6-for-of/illegal-left/options.json      |  2 +-
 .../illegal-export-esmodule-2/options.json    |  2 +-
 .../illegal-export-esmodule/options.json      |  2 +-
 24 files changed, 98 insertions(+), 41 deletions(-)
 create mode 100644 lib/6to5/messages.js

diff --git a/lib/6to5/messages.js b/lib/6to5/messages.js
new file mode 100644
index 0000000000..dd55873d0f
--- /dev/null
+++ b/lib/6to5/messages.js
@@ -0,0 +1,48 @@
+var toArray = require("lodash/lang/toArray");
+
+exports.messages = {
+  tailCallReassignmentDeopt: "Function reference has been reassigned so it's probably be dereferenced so we can't optimise this with confidence",
+  JSXNamespacedTags: "Namespace tags are not supported. ReactJSX is not XML.",
+  classesIllegalBareSuper: "Illegal use of bare super",
+  classesIllegalSuperCall: "Direct super call is illegal in non-constructor, use super.$1() instead",
+  classesIllegalConstructorKind: "Illegal kind for constructor method",
+  scopeDuplicateDeclaration: "Duplicate declaration $1",
+  undeclaredVariable: "Reference to undeclared variable $1",
+  undeclaredVariableSuggestion: "Reference to undeclared variable $1 - did you mean $2?",
+  settersInvalidParamLength: "Setters must have only one parameter",
+  noAssignmentsInForHead: "No assignments allowed in for-in/of head",
+  expectedMemberExpressionOrIdentifier: "Expected type MemeberExpression or Identifier",
+  invalidParentForThisNode: "We don't know how to handle this node within the current parent - please open an issue",
+  readOnly: "$1 is read-only",
+  modulesIllegalExportName: "Illegal export $1",
+  unknownForHead: "Unknown node type $1 in ForStatement"
+};
+
+exports.get = function (key) {
+  var msg = exports.messages[key];
+  if (!msg) throw new ReferenceError("Unknown message `" + key + "`");
+
+  var args = [];
+  for (var i = 1; i < arguments.length; i++) {
+    args.push(arguments[i]);
+  }
+  args = exports.parseArgs(args);
+
+  return msg.replace(/\$(\d+)/g, function (str, i) {
+    return args[--i];
+  });
+};
+
+exports.parseArgs = function (args) {
+  return args.map(function (val) {
+    if (val != null && val.inspect) {
+      return val.inspect();
+    } else {
+      try {
+        return JSON.stringify(val) || val + "";
+      } catch (e) {
+        return util.inspect(val);
+      }
+    }
+  });
+};
diff --git a/lib/6to5/transformation/helpers/build-react-transformer.js b/lib/6to5/transformation/helpers/build-react-transformer.js
index 414358ad79..29c78bacac 100644
--- a/lib/6to5/transformation/helpers/build-react-transformer.js
+++ b/lib/6to5/transformation/helpers/build-react-transformer.js
@@ -6,6 +6,7 @@
 // jsx
 
 var isString = require("lodash/lang/isString");
+var messages = require("../../messages");
 var esutils  = require("esutils");
 var react    = require("./react");
 var t        = require("../../types");
@@ -28,7 +29,7 @@ module.exports = function (exports, opts) {
   };
 
   exports.JSXNamespacedName = function (node, parent, scope, file) {
-    throw file.errorWithNode(node, "Namespace tags are not supported. ReactJSX is not XML.");
+    throw file.errorWithNode(node, messages.get("JSXNamespacedTags"));
   };
 
   exports.JSXMemberExpression = {
diff --git a/lib/6to5/transformation/helpers/replace-supers.js b/lib/6to5/transformation/helpers/replace-supers.js
index c3fb9a119d..20479e3c10 100644
--- a/lib/6to5/transformation/helpers/replace-supers.js
+++ b/lib/6to5/transformation/helpers/replace-supers.js
@@ -2,7 +2,8 @@
 
 module.exports = ReplaceSupers;
 
-var t = require("../../types");
+var messages = require("../../messages");
+var t        = require("../../types");
 
 /**
  * Description
@@ -232,7 +233,7 @@ ReplaceSupers.prototype.specHandle = function (getThisReference, node, parent) {
   var thisReference;
 
   if (isIllegalBareSuper(node, parent)) {
-    throw this.file.errorWithNode(node, "Illegal use of bare super");
+    throw this.file.errorWithNode(node, messages.get("classesIllegalBareSuper"));
   }
 
   if (t.isCallExpression(node)) {
@@ -248,7 +249,7 @@ ReplaceSupers.prototype.specHandle = function (getThisReference, node, parent) {
       //  - https://twitter.com/wycats/status/544553184396836864
       if (methodNode.key.name !== "constructor" || !this.inClass) {
         var methodName = methodNode.key.name || "METHOD_NAME";
-        throw this.file.errorWithNode(node, "Direct super call is illegal in non-constructor, use super." + methodName + "() instead");
+        throw this.file.errorWithNode(node, messages.get("classesIllegalSuperCall", methodName));
       }
     } else if (t.isMemberExpression(callee) && isSuper(callee.object, callee)) {
       // super.test(); -> _get(Object.getPrototypeOf(ClassName.prototype), "test", this).call(this);
diff --git a/lib/6to5/transformation/modules/_default.js b/lib/6to5/transformation/modules/_default.js
index a518583bf5..83cd732ca0 100644
--- a/lib/6to5/transformation/modules/_default.js
+++ b/lib/6to5/transformation/modules/_default.js
@@ -2,10 +2,11 @@
 
 module.exports = DefaultFormatter;
 
+var messages = require("../../messages");
+var extend   = require("lodash/object/extend");
 var object   = require("../../helpers/object");
 var util     = require("../../util");
 var t        = require("../../types");
-var extend   = require("lodash/object/extend");
 
 function DefaultFormatter(file) {
   this.file = file;
@@ -124,13 +125,6 @@ DefaultFormatter.prototype.isLocalReference = function (node) {
   return t.isIdentifier(node) && localImports[node.name] && localImports[node.name] !== node;
 };
 
-DefaultFormatter.prototype.checkLocalReference = function (node) {
-  var file = this.file;
-  if (this.isLocalReference(node)) {
-    throw file.errorWithNode(node, "Illegal assignment of module import");
-  }
-};
-
 DefaultFormatter.prototype.remapExportAssignment = function (node) {
   return t.assignmentExpression(
     "=",
@@ -213,7 +207,7 @@ DefaultFormatter.prototype.getExternalReference = function (node, nodes) {
 
 DefaultFormatter.prototype.checkExportIdentifier = function (node) {
   if (t.isIdentifier(node, { name: "__esModule" })) {
-    throw this.file.errorWithNode(node, "Illegal export __esModule - this is used internally for CommonJS interop");
+    throw this.file.errorWithNode(node, messages.get("modulesIllegalExportName", node.name));
   }
 };
 
diff --git a/lib/6to5/transformation/transformers/es6/classes.js b/lib/6to5/transformation/transformers/es6/classes.js
index 220b237231..c2c42a36a8 100644
--- a/lib/6to5/transformation/transformers/es6/classes.js
+++ b/lib/6to5/transformation/transformers/es6/classes.js
@@ -3,6 +3,7 @@
 var ReplaceSupers = require("../../helpers/replace-supers");
 var nameMethod    = require("../../helpers/name-method");
 var defineMap     = require("../../helpers/define-map");
+var messages      = require("../../../messages");
 var util          = require("../../../util");
 var t             = require("../../../types");
 
@@ -277,7 +278,7 @@ ClassTransformer.prototype.pushProperty = function (node) {
 
 ClassTransformer.prototype.pushConstructor = function (method) {
   if (method.kind) {
-    throw this.file.errorWithNode(method, "illegal kind for constructor method");
+    throw this.file.errorWithNode(method, messages.get("classesIllegalConstructorKind"));
   }
 
   var construct = this.constructor;
diff --git a/lib/6to5/transformation/transformers/es6/constants.js b/lib/6to5/transformation/transformers/es6/constants.js
index 286cdb8ff4..a58cd16256 100644
--- a/lib/6to5/transformation/transformers/es6/constants.js
+++ b/lib/6to5/transformation/transformers/es6/constants.js
@@ -1,6 +1,7 @@
 "use strict";
 
-var t = require("../../../types");
+var messages = require("../../../messages");
+var t        = require("../../../types");
 
 exports.check = function (node) {
   return t.isVariableDeclaration(node, { kind: "const" });
@@ -29,7 +30,7 @@ var visitor = {
         // check if there's been a local binding that shadows this constant
         if (!scope.bindingIdentifierEquals(name, constantIdentifier)) continue;
 
-        throw state.file.errorWithNode(id, name + " is read-only");
+        throw state.file.errorWithNode(id, messages.get("readOnly", name));
       }
     } else if (t.isScope(node, parent)) {
       this.skip();
diff --git a/lib/6to5/transformation/transformers/es6/destructuring.js b/lib/6to5/transformation/transformers/es6/destructuring.js
index cbcdda2f88..8f0ffe2428 100644
--- a/lib/6to5/transformation/transformers/es6/destructuring.js
+++ b/lib/6to5/transformation/transformers/es6/destructuring.js
@@ -1,6 +1,7 @@
 "use strict";
 
-var t = require("../../../types");
+var messages = require("../../../messages");
+var t        = require("../../../types");
 
 exports.check = t.isPattern;
 
@@ -387,7 +388,7 @@ exports.VariableDeclaration = function (node, parent, scope, file) {
       declar = declar || t.variableDeclaration(node.kind, []);
 
       if (!t.isVariableDeclaration(node) && declar.kind !== node.kind) {
-        throw file.errorWithNode(node, "Cannot use this node within the current parent");
+        throw file.errorWithNode(node, messages.get("invalidParentForThisNode"));
       }
 
       declar.declarations = declar.declarations.concat(node.declarations);
diff --git a/lib/6to5/transformation/transformers/es6/for-of.js b/lib/6to5/transformation/transformers/es6/for-of.js
index 0312e7f270..42125650b0 100644
--- a/lib/6to5/transformation/transformers/es6/for-of.js
+++ b/lib/6to5/transformation/transformers/es6/for-of.js
@@ -1,7 +1,8 @@
 "use strict";
 
-var util = require("../../../util");
-var t    = require("../../../types");
+var messages = require("../../../messages");
+var util     = require("../../../util");
+var t        = require("../../../types");
 
 exports.check = t.isForOfStatement;
 
@@ -50,7 +51,7 @@ var loose = function (node, parent, scope, file) {
       t.variableDeclarator(left.declarations[0].id, id)
     ]);
   } else {
-    throw file.errorWithNode(left, "Unknown node type " + left.type + " in ForOfStatement");
+    throw file.errorWithNode(left, messages.get("unknownForHead", left.type));
   }
 
   var loop = util.template("for-of-loose", {
@@ -89,7 +90,7 @@ var spec = function (node, parent, scope, file) {
       t.variableDeclarator(left.declarations[0].id, stepValue)
     ]);
   } else {
-    throw file.errorWithNode(left, "Unknown node type " + left.type + " in ForOfStatement");
+    throw file.errorWithNode(left, messages.get("unknownForHead", left.type));
   }
 
   var loop = util.template("for-of", {
diff --git a/lib/6to5/transformation/transformers/playground/mallet-operator.js b/lib/6to5/transformation/transformers/playground/mallet-operator.js
index c8d02aad1a..ff6aff5b6b 100644
--- a/lib/6to5/transformation/transformers/playground/mallet-operator.js
+++ b/lib/6to5/transformation/transformers/playground/mallet-operator.js
@@ -1,7 +1,8 @@
 "use strict";
 
-var build = require("../../helpers/build-conditional-assignment-operator-transformer");
-var t     = require("../../../types");
+var messages = require("../../../messages");
+var build    = require("../../helpers/build-conditional-assignment-operator-transformer");
+var t        = require("../../../types");
 
 exports.playground = true;
 
@@ -11,7 +12,7 @@ build(exports, {
     if (is) {
       var left = node.left;
       if (!t.isMemberExpression(left) && !t.isIdentifier(left)) {
-        throw file.errorWithNode(left, "Expected type MemeberExpression or Identifier");
+        throw file.errorWithNode(left, messages.get("expectedMemberExpressionOrIdentifier"));
       }
       return true;
     }
diff --git a/lib/6to5/transformation/transformers/validation/no-for-in-of-assignment.js b/lib/6to5/transformation/transformers/validation/no-for-in-of-assignment.js
index a8a8044e4f..4f28b28d66 100644
--- a/lib/6to5/transformation/transformers/validation/no-for-in-of-assignment.js
+++ b/lib/6to5/transformation/transformers/validation/no-for-in-of-assignment.js
@@ -1,6 +1,7 @@
 "use strict";
 
-var t = require("../../../types");
+var messages = require("../../../messages");
+var t        = require("../../../types");
 
 exports.check = t.isFor;
 
@@ -9,6 +10,6 @@ exports.ForOfStatement = function (node, parent, scope, file) {
   var left = node.left;
   if (t.isVariableDeclaration(left)) {
     var declar = left.declarations[0];
-    if (declar.init) throw file.errorWithNode(declar, "No assignments allowed in for-in/of head");
+    if (declar.init) throw file.errorWithNode(declar, messages.get("noAssignmentsInForHead"));
   }
 };
diff --git a/lib/6to5/transformation/transformers/validation/setters.js b/lib/6to5/transformation/transformers/validation/setters.js
index 76a1555515..839b73222e 100644
--- a/lib/6to5/transformation/transformers/validation/setters.js
+++ b/lib/6to5/transformation/transformers/validation/setters.js
@@ -1,5 +1,7 @@
 "use strict";
 
+var messages = require("../../../messages");
+
 exports.check = function (node) {
   return node.kind === "set";
 };
@@ -7,6 +9,6 @@ exports.check = function (node) {
 exports.MethodDefinition =
 exports.Property = function (node, parent, scope, file) {
   if (node.kind === "set" && node.value.params.length !== 1) {
-    throw file.errorWithNode(node.value, "Setters must have only one parameter");
+    throw file.errorWithNode(node.value, messages.get("settersInvalidParamLength"));
   }
 };
diff --git a/lib/6to5/transformation/transformers/validation/undeclared-variable-check.js b/lib/6to5/transformation/transformers/validation/undeclared-variable-check.js
index ebf66d15fc..b02a7354ba 100644
--- a/lib/6to5/transformation/transformers/validation/undeclared-variable-check.js
+++ b/lib/6to5/transformation/transformers/validation/undeclared-variable-check.js
@@ -1,6 +1,7 @@
 "use strict";
 
 var levenshtein = require("../../../helpers/levenshtein");
+var messages    = require("../../../messages");
 var t           = require("../../../types");
 
 exports.optional = true;
@@ -28,8 +29,11 @@ exports.Identifier = function (node, parent, scope, file) {
     shortest = distance;
   }
 
+  var msg;
   if (closest) {
-    msg += " - Did you mean " + closest + "?";
+    msg = messages.get("undeclaredVariableSuggestion", node.name, closest);
+  } else {
+    msg = messages.get("undeclaredVariable", node.name);
   }
 
   //
diff --git a/lib/6to5/traversal/scope.js b/lib/6to5/traversal/scope.js
index 683bca76ce..c9710696a0 100644
--- a/lib/6to5/traversal/scope.js
+++ b/lib/6to5/traversal/scope.js
@@ -5,6 +5,7 @@ module.exports = Scope;
 var includes = require("lodash/collection/includes");
 var traverse = require("./index");
 var defaults = require("lodash/object/defaults");
+var messages = require("../messages");
 var globals  = require("globals");
 var flatten  = require("lodash/array/flatten");
 var extend   = require("lodash/object/extend");
@@ -141,7 +142,7 @@ Scope.prototype.checkBlockScopedCollisions = function (kind, name, id) {
   if (kind === "hoisted" && local.kind === "let") return;
 
   if (local.kind === "let" || local.kind === "const") {
-    throw this.file.errorWithNode(id, "Duplicate declaration " + name, TypeError);
+    throw this.file.errorWithNode(id, messages.get("scopeDuplicateDeclaration", name), TypeError);
   }
 };
 
diff --git a/test/fixtures/transformation/errors/constants/options.json b/test/fixtures/transformation/errors/constants/options.json
index 7129c2b990..23340cd45d 100644
--- a/test/fixtures/transformation/errors/constants/options.json
+++ b/test/fixtures/transformation/errors/constants/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Line 2: MULTIPLIER is read-only"
+  "throws": "Line 2: \"MULTIPLIER\" is read-only"
 }
diff --git a/test/fixtures/transformation/es6-classes/defining-constructor-as-a-mutator/options.json b/test/fixtures/transformation/es6-classes/defining-constructor-as-a-mutator/options.json
index 2b8c4aba05..3822922af0 100644
--- a/test/fixtures/transformation/es6-classes/defining-constructor-as-a-mutator/options.json
+++ b/test/fixtures/transformation/es6-classes/defining-constructor-as-a-mutator/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "illegal kind for constructor method"
+  "throws": "Illegal kind for constructor method"
 }
diff --git a/test/fixtures/transformation/es6-constants/destructuring-assignment/options.json b/test/fixtures/transformation/es6-constants/destructuring-assignment/options.json
index feeb18e277..0d6f783696 100644
--- a/test/fixtures/transformation/es6-constants/destructuring-assignment/options.json
+++ b/test/fixtures/transformation/es6-constants/destructuring-assignment/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "a is read-only"
+  "throws": "\"a\" is read-only"
 }
diff --git a/test/fixtures/transformation/es6-constants/no-assignment/options.json b/test/fixtures/transformation/es6-constants/no-assignment/options.json
index 9b96d41ac9..3eed8a7760 100644
--- a/test/fixtures/transformation/es6-constants/no-assignment/options.json
+++ b/test/fixtures/transformation/es6-constants/no-assignment/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "MULTIPLIER is read-only"
+  "throws": "\"MULTIPLIER\" is read-only"
 }
diff --git a/test/fixtures/transformation/es6-constants/no-classes/options.json b/test/fixtures/transformation/es6-constants/no-classes/options.json
index d51ef9b5ce..8901cbf81b 100644
--- a/test/fixtures/transformation/es6-constants/no-classes/options.json
+++ b/test/fixtures/transformation/es6-constants/no-classes/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Duplicate declaration MULTIPLIER"
+  "throws": "Duplicate declaration \"MULTIPLIER\""
 }
diff --git a/test/fixtures/transformation/es6-constants/no-declaration/options.json b/test/fixtures/transformation/es6-constants/no-declaration/options.json
index d51ef9b5ce..8901cbf81b 100644
--- a/test/fixtures/transformation/es6-constants/no-declaration/options.json
+++ b/test/fixtures/transformation/es6-constants/no-declaration/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Duplicate declaration MULTIPLIER"
+  "throws": "Duplicate declaration \"MULTIPLIER\""
 }
diff --git a/test/fixtures/transformation/es6-constants/no-functions/options.json b/test/fixtures/transformation/es6-constants/no-functions/options.json
index d51ef9b5ce..8901cbf81b 100644
--- a/test/fixtures/transformation/es6-constants/no-functions/options.json
+++ b/test/fixtures/transformation/es6-constants/no-functions/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Duplicate declaration MULTIPLIER"
+  "throws": "Duplicate declaration \"MULTIPLIER\""
 }
diff --git a/test/fixtures/transformation/es6-constants/update-expression/options.json b/test/fixtures/transformation/es6-constants/update-expression/options.json
index e3a5a52096..afb7d8b522 100644
--- a/test/fixtures/transformation/es6-constants/update-expression/options.json
+++ b/test/fixtures/transformation/es6-constants/update-expression/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "foo is read-only"
+  "throws": "\"foo\" is read-only"
 }
diff --git a/test/fixtures/transformation/es6-for-of/illegal-left/options.json b/test/fixtures/transformation/es6-for-of/illegal-left/options.json
index 22d8ac64c9..f8b6d8f50e 100644
--- a/test/fixtures/transformation/es6-for-of/illegal-left/options.json
+++ b/test/fixtures/transformation/es6-for-of/illegal-left/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Unknown node type MemberExpression in ForOfStatement"
+  "throws": "Unknown node type \"MemberExpression\" in ForStatement"
 }
diff --git a/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule-2/options.json b/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule-2/options.json
index 5e33c8dbe1..affb837b2c 100644
--- a/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule-2/options.json
+++ b/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule-2/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Illegal export __esModule - this is used internally for CommonJS interop"
+  "throws": "Illegal export \"__esModule\""
 }
diff --git a/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule/options.json b/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule/options.json
index 5e33c8dbe1..affb837b2c 100644
--- a/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule/options.json
+++ b/test/fixtures/transformation/es6-modules-common/illegal-export-esmodule/options.json
@@ -1,3 +1,3 @@
 {
-  "throws": "Illegal export __esModule - this is used internally for CommonJS interop"
+  "throws": "Illegal export \"__esModule\""
 }