Bug 947044 - Provide a suggestion when throwing ReferenceError: <name> is not defined. r=luke

js/src/jit-test/tests/basic/testErrorInFinalizerCalledWhileUnwinding.js

@@ -25,7 +25,7 @@ try {
     test();
 } catch(e) {
     caught = true;
-    assertEq(''+e, "ReferenceError: not_defined is not defined");
+    assertEq(e.toString().contains("ReferenceError: not_defined is not defined"), true);
 }
 
 assertEq(finalizerRun, true);

js/src/jit-test/tests/basic/testReferenceErrorNameSuggestion-01.js

@@ -0,0 +1,15 @@
+// Test that we get a suggestion in a ReferenceError's message.
+
+function levenshteinDistance() {}
+
+let e;
+try {
+  // Note "ie" instead of "ei" in "levenshtein".
+  levenshtienDistance()
+} catch (ee) {
+  e = ee;
+}
+
+assertEq(e !== undefined, true);
+assertEq(e.name, "ReferenceError");
+assertEq(e.message.contains("did you mean 'levenshteinDistance'?"), true);

js/src/jit-test/tests/basic/testReferenceErrorNameSuggestion-02.js

@@ -0,0 +1,16 @@
+// Test that we don't waste cycles on edit distance when names are too long.
+
+const name = "a".repeat(10000);
+this[name] = () => {};
+
+let e;
+try {
+  eval(name + "a()");
+} catch (ee) {
+  e = ee;
+}
+
+assertEq(e !== undefined, true);
+assertEq(e.name, "ReferenceError");
+// Name was too long, shouldn't have provided a suggestion.
+assertEq(e.message.contains("did you mean"), false);

js/src/jit-test/tests/basic/testReferenceErrorNameSuggestion-03.js

@@ -0,0 +1,23 @@
+// Test that we actually suggest the most similar name.
+
+function rza() {}
+function gza() {}
+function odb() {}
+function methodMan() {}
+function inspectahDeck() {}
+function raekwonTheChef() {}
+function ghostfaceKillah() {}
+function uGod() {}
+function mastaKillah() {}
+
+let e;
+try {
+  // Missing "d".
+  methoMan();
+} catch (ee) {
+  e = ee;
+}
+
+assertEq(e !== undefined, true);
+assertEq(e.name, "ReferenceError");
+assertEq(e.message.contains("did you mean 'methodMan'?"), true);

js/src/jit/BaselineIC.cpp

@@ -5833,7 +5833,7 @@ DoGetNameFallback(JSContext *cx, BaselineFrame *frame, ICGetName_Fallback *stub_
         if (!GetScopeNameForTypeOf(cx, scopeChain, name, res))
             return false;
     } else {
-        if (!GetScopeName(cx, scopeChain, name, res))
+        if (!GetScopeName(cx, script, pc, scopeChain, name, res))
             return false;
     }
 

js/src/jit/IonCaches.cpp

@@ -4374,10 +4374,10 @@ NameIC::update(JSContext *cx, size_t cacheIndex, HandleObject scopeChain,
     }
 
     if (cache.isTypeOf()) {
-        if (!FetchName<true>(cx, obj, holder, name, shape, vp))
+        if (!FetchName<true>(cx, script, pc, obj, holder, name, shape, vp))
             return false;
     } else {
-        if (!FetchName<false>(cx, obj, holder, name, shape, vp))
+        if (!FetchName<false>(cx, script, pc, obj, holder, name, shape, vp))
             return false;
     }
 

js/src/js.msg

@@ -43,6 +43,7 @@
 
 MSG_DEF(JSMSG_NOT_AN_ERROR,            0, JSEXN_NONE, "<Error #0 is reserved>")
 MSG_DEF(JSMSG_NOT_DEFINED,             1, JSEXN_REFERENCEERR, "{0} is not defined")
+MSG_DEF(JSMSG_NOT_DEFINED_DID_YOU_MEAN, 2, JSEXN_REFERENCEERR, "{0} is not defined, did you mean '{1}'?")
 MSG_DEF(JSMSG_MORE_ARGS_NEEDED,        3, JSEXN_TYPEERR, "{0} requires more than {1} argument{2}")
 MSG_DEF(JSMSG_INCOMPATIBLE_PROTO,      3, JSEXN_TYPEERR, "{0}.prototype.{1} called on incompatible {2}")
 MSG_DEF(JSMSG_NO_CONSTRUCTOR,          1, JSEXN_TYPEERR, "{0} has no constructor")

js/src/jscntxt.cpp

@@ -14,6 +14,7 @@
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MemoryReporting.h"
 
+#include <algorithm>
 #include <ctype.h>
 #include <stdarg.h>
 #include <string.h>
@@ -49,6 +50,7 @@
 #include "jsobjinlines.h"
 #include "jsscriptinlines.h"
 
+#include "vm/ScopeObject-inl.h"
 #include "vm/Stack-inl.h"
 
 using namespace js;
@@ -879,10 +881,178 @@ js::CallErrorReporter(JSContext *cx, const char *message, JSErrorReport *reportp
         onError(cx, message, reportp);
 }
 
-void
-js_ReportIsNotDefined(JSContext *cx, const char *name)
+static const size_t MAX_NAME_LENGTH_FOR_EDIT_DISTANCE = 1000;
+static const size_t MAX_REFERENCE_ERROR_NAMES_TO_CHECK = 1000;
+
+/*
+ * The edit distance between two strings is defined as the Levenshtein distance,
+ * which is described here: (http://en.wikipedia.org/wiki/Levenshtein_distance).
+ * Intuitively, this is the number of insert, delete, and/or substitution
+ * operations required to get from one string to the other.
+ *
+ * Given two atoms, this function computes their edit distance using dynamic
+ * programming. The resulting algorithm has O(m * n) complexity, but since it is
+ * only used for ReferenceError reporting, and the given atoms are expected to
+ * be small, its performance should be good enough. Despite that, we will only
+ * compute the edit distance for names whose length are shorter than
+ * MAX_NAME_LENGTH_FOR_EDIT_DISTANCE. We shouldn't ever find a pair with an edit
+ * distance of 0 (or else there wouldn't have been a ReferenceError), so we set
+ * the value presult points to to 0 and return true when a name is too long.
+ */
+static bool ComputeEditDistance(JSContext *cx, HandleAtom atom1,
+                                HandleAtom atom2, size_t *presult)
 {
-    JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_DEFINED, name);
+    *presult = 0;
+
+    const size_t m = atom1->length();
+    if (m >= MAX_NAME_LENGTH_FOR_EDIT_DISTANCE)
+        return true;
+    const size_t n = atom2->length();
+    if (m >= MAX_NAME_LENGTH_FOR_EDIT_DISTANCE)
+        return true;
+
+    Vector<size_t> d(cx);
+    if (!d.growBy((m + 1) * (n + 1)))
+        return false;
+
+    AutoStableStringChars aChars(cx);
+    AutoStableStringChars bChars(cx);
+    if (!aChars.initTwoByte(cx, atom1) || !bChars.initTwoByte(cx, atom2))
+        return false;
+
+    const char16_t *a = aChars.twoByteRange().start().get();
+    const char16_t *b = bChars.twoByteRange().start().get();
+
+    /*
+     * D(i, j) is defined as the edit distance between the i-length prefix
+     * of a and the m-length prefix of b.
+     */
+#define D(i, j) (d[(i) * ((n) + 1) + (j)])
+
+    /*
+     * Given the i-length prefix of a, the 0-length prefix of b can be
+     * obtained by deleting i characters.
+     */
+    for (size_t i = 0; i <= m; ++i)
+        D(i, 0) = i;
+
+    /*
+     * Given the j-length prefix of b, the 0-length prefix of a can be
+     * obtained by inserting i characters.
+     */
+    for (size_t j = 0; j <= n; ++j)
+        D(0, j) = j;
+
+    for (size_t i = 1; i <= m; ++i) {
+        for (size_t j = 1; j <= n; ++j) {
+            /*
+             * If the i-length prefix of a and the j-length prefix of b are
+             * equal in their last character, their edit distance equals
+             * that of the i-1-length and j-1-length prefix of a and b,
+             * respectively.
+             */
+            if (a[i - 1] == b[j - 1])
+                D(i, j) = D(i - 1, j - 1); // No operation required
+            else {
+                D(i, j) = std::min(
+                                   D(i - 1, j) + 1, // Deletion
+                                   std::min(
+                                            D(i, j - 1) + 1, // Insertion
+                                            D(i - 1, j - 1) + 1 // Substitution
+                                            )
+                                   );
+            }
+        }
+    }
+
+    *presult = D(m, n);
+
+#undef D
+
+    return true;
+}
+
+void
+js_ReportIsNotDefined(JSContext *cx, HandleScript script, jsbytecode *pc, HandleAtom atom)
+{
+    /*
+     * Walk the static scope chain and the global object to find the name that
+     * most closely matches the one we are looking for, so we can provide it as
+     * a hint to the user.
+     *
+     * To quantify how closely one name matches another, we define a metric on
+     * strings known as the edit distance (see ComputeEditDistance for details).
+     * We then pick the name with the shortest edit distance from the name we
+     * were trying to find.
+     */
+    AutoIdVector ids(cx);
+    for (StaticScopeIter<CanGC> ssi(cx, InnermostStaticScope(script, pc)); !ssi.done(); ssi++) {
+        switch (ssi.type()) {
+          case StaticScopeIter<NoGC>::BLOCK:
+            if (!GetPropertyNames(cx, &ssi.block(), JSITER_OWNONLY, &ids)) {
+                /*
+                 * If GetPropertyNames fails (due to overrecursion), we still
+                 * want to act as if we had never called it, and report the
+                 * reference error instead. Otherwise, we would break
+                 * tests/gc/bug-886560.js.
+                 */
+                js_ReportIsNotDefined(cx, atom);
+                return;
+            }
+            break;
+
+          case StaticScopeIter<NoGC>::FUNCTION:
+          {
+            RootedScript script(cx, ssi.funScript());
+            for (BindingIter bi(script); !bi.done(); bi++)
+                ids.append(NameToId(bi->name()));
+            break;
+          }
+
+          case StaticScopeIter<CanGC>::NAMED_LAMBDA:
+            ids.append(NameToId(ssi.lambdaName()));
+            break;
+        }
+    }
+    if (!GetPropertyNames(cx, cx->global(), JSITER_OWNONLY, &ids)) {
+        // See comment above
+        js_ReportIsNotDefined(cx, atom);
+        return;
+    }
+
+    RootedAtom bestMatch(cx);
+    size_t minDistance = (size_t) -1;
+    size_t max = std::min(ids.length(), MAX_REFERENCE_ERROR_NAMES_TO_CHECK);
+    for (size_t i = 0; i < max; ++i) {
+        RootedAtom otherAtom(cx, JSID_TO_ATOM(ids[i]));
+        size_t distance;
+        if (!ComputeEditDistance(cx, atom, otherAtom, &distance))
+            return;
+        if (distance != 0 && distance < minDistance) {
+            bestMatch = JSID_TO_ATOM(ids[i]);
+            minDistance = distance;
+        }
+    }
+
+    if (!bestMatch) {
+        // We didn't find any suitable suggestions.
+        js_ReportIsNotDefined(cx, atom);
+        return;
+    }
+
+    JSAutoByteString bytes1(cx, atom);
+    JSAutoByteString bytes2(cx, bestMatch);
+    JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr,
+                         JSMSG_NOT_DEFINED_DID_YOU_MEAN, bytes1.ptr(),
+                         bytes2.ptr());
+}
+
+void
+js_ReportIsNotDefined(JSContext *cx, HandleAtom atom)
+{
+    JSAutoByteString bytes(cx, atom);
+    JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_DEFINED,
+                         bytes.ptr());
 }
 
 bool

js/src/jscntxt.h

@@ -750,7 +750,10 @@ CallErrorReporter(JSContext *cx, const char *message, JSErrorReport *report);
 } /* namespace js */
 
 extern void
-js_ReportIsNotDefined(JSContext *cx, const char *name);
+js_ReportIsNotDefined(JSContext *cx, js::HandleScript script, jsbytecode *pc, js::HandleAtom atom);
+
+extern void
+js_ReportIsNotDefined(JSContext *cx, js::HandleAtom atom);
 
 /*
  * Report an attempt to access the property of a null or undefined value (v).

js/src/jsobj.cpp

@@ -5076,9 +5076,8 @@ GetPropertyHelperInline(JSContext *cx,
 
             if (op == JSOP_GETXPROP) {
                 /* Undefined property during a name lookup, report an error. */
-                JSAutoByteString printable;
-                if (js_ValueToPrintable(cx, IdToValue(id), &printable))
-                    js_ReportIsNotDefined(cx, printable.ptr());
+                RootedAtom atom(cx, JSID_TO_ATOM(id));
+                js_ReportIsNotDefined(cx, atom);
                 return false;
             }
 

js/src/vm/Interpreter-inl.h

@@ -224,17 +224,17 @@ GetLengthProperty(const Value &lval, MutableHandleValue vp)
 }
 
 template <bool TypeOf> inline bool
-FetchName(JSContext *cx, HandleObject obj, HandleObject obj2, HandlePropertyName name,
-          HandleShape shape, MutableHandleValue vp)
+FetchName(JSContext *cx, HandleScript script, jsbytecode *pc, HandleObject obj,
+          HandleObject obj2, HandlePropertyName name, HandleShape shape,
+          MutableHandleValue vp)
 {
+    if (!shape && TypeOf) {
+        vp.setUndefined();
+        return true;
+    }
+
     if (!shape) {
-        if (TypeOf) {
-            vp.setUndefined();
-            return true;
-        }
-        JSAutoByteString printable;
-        if (AtomToPrintableString(cx, name, &printable))
-            js_ReportIsNotDefined(cx, printable.ptr());
+        js_ReportIsNotDefined(cx, script, pc, name);
         return false;
     }
 

js/src/vm/Interpreter.cpp

@@ -267,8 +267,8 @@ GetPropertyOperation(JSContext *cx, InterpreterFrame *fp, HandleScript script, j
 static inline bool
 NameOperation(JSContext *cx, InterpreterFrame *fp, jsbytecode *pc, MutableHandleValue vp)
 {
-    JSObject *obj = fp->scopeChain();
-    PropertyName *name = fp->script()->getName(pc);
+    RootedScript script(cx, fp->script());
+    PropertyName *name = script->getName(pc);
 
     /*
      * Skip along the scope chain to the enclosing global object. This is
@@ -279,6 +279,7 @@ NameOperation(JSContext *cx, InterpreterFrame *fp, jsbytecode *pc, MutableHandle
      * the actual behavior even if the id could be found on the scope chain
      * before the global object.
      */
+    JSObject *obj = fp->scopeChain();
     if (IsGlobalOp(JSOp(*pc)))
         obj = &obj->global();
 
@@ -299,10 +300,10 @@ NameOperation(JSContext *cx, InterpreterFrame *fp, jsbytecode *pc, MutableHandle
     /* Kludge to allow (typeof foo == "undefined") tests. */
     JSOp op2 = JSOp(pc[JSOP_NAME_LENGTH]);
     if (op2 == JSOP_TYPEOF) {
-        if (!FetchName<true>(cx, scopeRoot, pobjRoot, nameRoot, shapeRoot, vp))
+        if (!FetchName<true>(cx, script, pc, scopeRoot, pobjRoot, nameRoot, shapeRoot, vp))
             return false;
     } else {
-        if (!FetchName<false>(cx, scopeRoot, pobjRoot, nameRoot, shapeRoot, vp))
+        if (!FetchName<false>(cx, script, pc, scopeRoot, pobjRoot, nameRoot, shapeRoot, vp))
             return false;
     }
 
@@ -3540,7 +3541,9 @@ js::CallProperty(JSContext *cx, HandleValue v, HandlePropertyName name, MutableH
 }
 
 bool
-js::GetScopeName(JSContext *cx, HandleObject scopeChain, HandlePropertyName name, MutableHandleValue vp)
+js::GetScopeName(JSContext *cx, HandleScript script, jsbytecode *pc,
+                 HandleObject scopeChain, HandlePropertyName name,
+                 MutableHandleValue vp)
 {
     RootedShape shape(cx);
     RootedObject obj(cx), pobj(cx);
@@ -3548,9 +3551,7 @@ js::GetScopeName(JSContext *cx, HandleObject scopeChain, HandlePropertyName name
         return false;
 
     if (!shape) {
-        JSAutoByteString printable;
-        if (AtomToPrintableString(cx, name, &printable))
-            js_ReportIsNotDefined(cx, printable.ptr());
+        js_ReportIsNotDefined(cx, script, pc, name);
         return false;
     }
 

js/src/vm/Interpreter.h

@@ -372,7 +372,8 @@ bool
 CallProperty(JSContext *cx, HandleValue value, HandlePropertyName name, MutableHandleValue vp);
 
 bool
-GetScopeName(JSContext *cx, HandleObject obj, HandlePropertyName name, MutableHandleValue vp);
+GetScopeName(JSContext *cx, HandleScript script, jsbytecode *pc, HandleObject obj,
+             HandlePropertyName name, MutableHandleValue vp);
 
 bool
 GetScopeNameForTypeOf(JSContext *cx, HandleObject obj, HandlePropertyName name,

js/src/vm/ScopeObject-inl.h

@@ -149,6 +149,14 @@ StaticScopeIter<allowGC>::fun() const
     return obj->template as<JSFunction>();
 }
 
+template <AllowGC allowGC>
+inline PropertyName *
+StaticScopeIter<allowGC>::lambdaName() const
+{
+    JS_ASSERT(type() == NAMED_LAMBDA);
+    return obj->template as<JSFunction>().name();
+}
+
 }  /* namespace js */
 
 #endif /* vm_ScopeObject_inl_h */

js/src/vm/ScopeObject.cpp

@@ -34,11 +34,10 @@ typedef MutableHandle<ArgumentsObject *> MutableHandleArgumentsObject;
 
 /*****************************************************************************/
 
-static JSObject *
-InnermostStaticScope(JSScript *script, jsbytecode *pc)
+JSObject *
+js::InnermostStaticScope(JSScript *script, jsbytecode *pc)
 {
     JS_ASSERT(script->containsPC(pc));
-    JS_ASSERT(JOF_OPTYPE(*pc) == JOF_SCOPECOORD);
 
     NestedScopeObject *scope = script->getStaticScope(pc);
     if (scope)

js/src/vm/ScopeObject.h

@@ -56,6 +56,8 @@ class StaticWithObject;
  *
  * (See also AssertDynamicScopeMatchesStaticScope.)
  */
+JSObject *InnermostStaticScope(JSScript *script, jsbytecode *pc);
+
 template <AllowGC allowGC>
 class StaticScopeIter
 {
@@ -93,6 +95,7 @@ class StaticScopeIter
     StaticWithObject &staticWith() const;
     JSScript *funScript() const;
     JSFunction &fun() const;
+    PropertyName *lambdaName() const;
 };
 
 /*****************************************************************************/