Bug 1352429 - Improve error message for in operator. r=arai
authorsnowman-mh <mcdonalds.only@gmail.com>
Wed, 08 Nov 2017 15:03:47 +0900
changeset 444478 389bd78cf79dd989348a244d8b7d89a77f637e3f
parent 444477 56d0ff94578a9a8a2ebd7132846e2222116d0d9e
child 444582 91a4c7a72f31c89d174a173ef8323f6b06895929
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1352429
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1352429 - Improve error message for in operator. r=arai
js/src/jit/BaselineIC.cpp
js/src/js.msg
js/src/tests/ecma_6/Expressions/inNotObjectError.js
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -1265,17 +1265,17 @@ DoInFallback(JSContext* cx, BaselineFram
              HandleValue key, HandleValue objValue, MutableHandleValue res)
 {
     // This fallback stub may trigger debug mode toggling.
     DebugModeOSRVolatileStub<ICIn_Fallback*> stub(frame, 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;
     }
 
     if (stub->state().maybeTransition())
         stub->discardStubs(cx);
 
     if (stub->state().canAttachStub()) {
         RootedScript script(cx, frame->script());
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -111,17 +111,17 @@ MSG_DEF(JSMSG_NOT_OBJORNULL,           1
 // JSON
 MSG_DEF(JSMSG_JSON_BAD_PARSE,          3, JSEXN_SYNTAXERR, "JSON.parse: {0} at line {1} column {2} of the JSON data")
 MSG_DEF(JSMSG_JSON_CYCLIC_VALUE,       0, JSEXN_TYPEERR, "cyclic object value")
 
 // Runtime errors
 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")
 MSG_DEF(JSMSG_BAD_CONST_ASSIGN,        1, JSEXN_TYPEERR, "invalid assignment to const `{0}'")
 MSG_DEF(JSMSG_CANT_DECLARE_GLOBAL_BINDING, 2, JSEXN_TYPEERR, "cannot declare global binding `{0}': {1}")
 
 // Date
 MSG_DEF(JSMSG_INVALID_DATE,            0, JSEXN_RANGEERR, "invalid date")
new file mode 100644
--- /dev/null
+++ b/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);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -41,16 +41,17 @@
 #include "vm/AsyncFunction.h"
 #include "vm/AsyncIteration.h"
 #include "vm/Debugger.h"
 #include "vm/GeneratorObject.h"
 #include "vm/Opcodes.h"
 #include "vm/Scope.h"
 #include "vm/Shape.h"
 #include "vm/Stopwatch.h"
+#include "vm/StringBuffer.h"
 #include "vm/TraceLogging.h"
 
 #include "jsatominlines.h"
 #include "jsboolinlines.h"
 #include "jsfuninlines.h"
 #include "jsscriptinlines.h"
 
 #include "jit/JitFrames-inl.h"
@@ -1678,16 +1679,50 @@ class ReservedRooted : public RootedBase
     MutableHandle<T> operator&() { return &*savedRoot; }
 
     DECLARE_NONPOINTER_ACCESSOR_METHODS(savedRoot->get())
     DECLARE_NONPOINTER_MUTABLE_ACCESSOR_METHODS(savedRoot->get())
     DECLARE_POINTER_CONSTREF_OPS(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)
 {
 /*
  * Define macros for an interpreter loop. Opcode dispatch may be either by a
  * switch statement or by indirect goto (aka a threaded interpreter), depending
  * on compiler support.
  *
@@ -2201,17 +2236,18 @@ END_CASE(JSOP_AND)
             ADVANCE_AND_DISPATCH(1 + JSOP_IFEQ_LENGTH);                       \
         }                                                                     \
     JS_END_MACRO
 
 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;
     {
         ReservedRooted<JSObject*> obj(&rootObject0, &rref.toObject());
         ReservedRooted<jsid> id(&rootId0);
         FETCH_ELEMENT_ID(-2, id);
         if (!HasProperty(cx, obj, id, &found))
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -547,16 +547,19 @@ void
 ReportRuntimeLexicalError(JSContext* cx, unsigned errorNumber, HandleId id);
 
 void
 ReportRuntimeLexicalError(JSContext* cx, unsigned errorNumber, HandlePropertyName name);
 
 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}.
 void
 ReportRuntimeRedeclaration(JSContext* cx, HandlePropertyName name, const char* redeclKind);
 
 enum class CheckIsObjectKind : uint8_t {
     IteratorNext,