Bug 1352429 - Improve error message for in operator. r=arai

js/src/jit/BaselineIC.cpp

@@ -1270,7 +1270,7 @@ DoInFallback(JSContext* cx, BaselineFrame* frame, ICIn_Fallback* stub_,
     FallbackICSpew(cx, stub, "In");
 
     if (!objValue.isObject()) {
-        ReportValueError(cx, JSMSG_IN_NOT_OBJECT, -1, objValue, nullptr);
+        ReportInNotObjectError(cx, key, -2, objValue, -1);
         return false;
     }
 

js/src/js.msg

@@ -116,7 +116,7 @@ MSG_DEF(JSMSG_JSON_CYCLIC_VALUE,       0, JSEXN_TYPEERR, "cyclic object value")
 MSG_DEF(JSMSG_BAD_INSTANCEOF_RHS,      1, JSEXN_TYPEERR, "invalid 'instanceof' operand {0}")
 MSG_DEF(JSMSG_BAD_LEFTSIDE_OF_ASS,     0, JSEXN_REFERENCEERR, "invalid assignment left-hand side")
 MSG_DEF(JSMSG_BAD_PROTOTYPE,           1, JSEXN_TYPEERR, "'prototype' property of {0} is not an object")
-MSG_DEF(JSMSG_IN_NOT_OBJECT,           1, JSEXN_TYPEERR, "invalid 'in' operand {0}")
+MSG_DEF(JSMSG_IN_NOT_OBJECT,           2, JSEXN_TYPEERR, "cannot use 'in' operator to search for '{0}' in '{1}'")
 MSG_DEF(JSMSG_TOO_MANY_CON_SPREADARGS, 0, JSEXN_RANGEERR, "too many constructor arguments")
 MSG_DEF(JSMSG_TOO_MANY_FUN_SPREADARGS, 0, JSEXN_RANGEERR, "too many function arguments")
 MSG_DEF(JSMSG_UNINITIALIZED_LEXICAL,   1, JSEXN_REFERENCEERR, "can't access lexical declaration `{0}' before initialization")

js/src/tests/ecma_6/Expressions/inNotObjectError.js

@@ -0,0 +1,46 @@
+var BUGNUMBER = 1352429;
+var summary = 'Error message should provide enough infomation for use of in operator';
+
+print(BUGNUMBER + ": " + summary);
+
+function checkErr(substr, str, messageSubstr, messageStr) {
+    var caught = false;
+    try {
+        substr in str;
+    } catch (e) {
+        caught = true;
+        assertEq(e.message.includes(messageSubstr), true);
+        assertEq(e.message.includes(messageStr), true);
+        assertEq(e.message.length < 100, true);
+    }
+    assertEq(caught, true);
+}
+
+// These test cases check if long string is omitted properly.
+checkErr('subString', 'base', 'subString', 'base');
+checkErr('this is subString', 'base', 'this is subStrin...', 'base');
+checkErr('subString', 'this is baseString', 'subString', 'this is baseStri...');
+checkErr('this is subString', 'this is base', 'this is subStrin...', 'this is base');
+checkErr('HEAD' + 'subString'.repeat(30000), 'HEAD' + 'base'.repeat(30000), 'HEADsubStringsub...', 'HEADbasebasebase...');
+
+// These test cases check if it does not crash and throws appropriate error.
+assertThrowsInstanceOf(() => { 1 in 'hello' }, TypeError);
+assertThrowsInstanceOf(() => { 'hello' in 1 }, TypeError);
+assertThrowsInstanceOf(() => { 'hello' in null }, TypeError);
+assertThrowsInstanceOf(() => { null in 'hello' }, TypeError);
+assertThrowsInstanceOf(() => { null in null }, TypeError);
+assertThrowsInstanceOf(() => { 'hello' in true }, TypeError);
+assertThrowsInstanceOf(() => { false in 1.1 }, TypeError);
+assertThrowsInstanceOf(() => { Symbol.iterator in undefined }, TypeError);
+assertThrowsInstanceOf(() => { [] in undefined }, TypeError);
+assertThrowsInstanceOf(() => { /a/ in 'hello' }, TypeError);
+var str = 'hello';
+assertThrowsInstanceOf(() => { str in 'hello' }, TypeError);
+class A {};
+assertThrowsInstanceOf(() => { new A() in undefined }, TypeError);
+var a = new A();
+a.b = 1.1;
+assertThrowsInstanceOf(() => { a.b in 1.1 }, TypeError);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0, 0);

js/src/vm/Interpreter.cpp

@@ -46,6 +46,7 @@
 #include "vm/Scope.h"
 #include "vm/Shape.h"
 #include "vm/Stopwatch.h"
+#include "vm/StringBuffer.h"
 #include "vm/TraceLogging.h"
 
 #include "jsatominlines.h"
@@ -1683,6 +1684,40 @@ class ReservedRooted : public RootedBase<T, ReservedRooted<T>>
     DECLARE_POINTER_ASSIGN_OPS(ReservedRooted, T)
 };
 
+void
+js::ReportInNotObjectError(JSContext* cx, HandleValue lref, int lindex,
+                           HandleValue rref, int rindex)
+{
+    auto uniqueCharsFromString = [](JSContext* cx, HandleValue ref) -> UniqueChars {
+        static const size_t MAX_STRING_LENGTH = 16;
+        RootedString str(cx, ref.toString());
+        if (str->length() > MAX_STRING_LENGTH) {
+            StringBuffer buf(cx);
+            if (!buf.appendSubstring(str, 0, MAX_STRING_LENGTH))
+                return nullptr;
+            if (!buf.append("..."))
+                return nullptr;
+            str = buf.finishString();
+            if (!str)
+                return nullptr;
+        }
+        return UniqueChars(JS_EncodeString(cx, str));
+    };
+
+    UniqueChars lbytes = lref.isString()
+                       ? uniqueCharsFromString(cx, lref)
+                       : DecompileValueGenerator(cx, lindex, lref, nullptr);
+    if (!lbytes)
+        return;
+    UniqueChars rbytes = rref.isString()
+                       ? uniqueCharsFromString(cx, rref)
+                       : DecompileValueGenerator(cx, rindex, rref, nullptr);
+    if (!rbytes)
+        return;
+    JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr, JSMSG_IN_NOT_OBJECT,
+                               lbytes.get(), rbytes.get());
+}
+
 static MOZ_NEVER_INLINE bool
 Interpret(JSContext* cx, RunState& state)
 {
@@ -2206,7 +2241,8 @@ CASE(JSOP_IN)
 {
     HandleValue rref = REGS.stackHandleAt(-1);
     if (!rref.isObject()) {
-        ReportValueError(cx, JSMSG_IN_NOT_OBJECT, -1, rref, nullptr);
+        HandleValue lref = REGS.stackHandleAt(-2);
+        ReportInNotObjectError(cx, lref, -2, rref, -1);
         goto error;
     }
     bool found;

js/src/vm/Interpreter.h

@@ -552,6 +552,9 @@ ReportRuntimeLexicalError(JSContext* cx, unsigned errorNumber, HandlePropertyNam
 void
 ReportRuntimeLexicalError(JSContext* cx, unsigned errorNumber, HandleScript script, jsbytecode* pc);
 
+void
+ReportInNotObjectError(JSContext* cx, HandleValue lref, int lindex, HandleValue rref, int rindex);
+
 // The parser only reports redeclarations that occurs within a single
 // script. Due to the extensibility of the global lexical scope, we also check
 // for redeclarations during runtime in JSOP_DEF{VAR,LET,CONST}.