MatchersUtil#contains uses deep equality rather than === for set members

[#169001712]

lib/jasmine-core/jasmine.js

@@ -322,6 +322,10 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) {
     );
   };
 
+  j$.isIterable_ = function(value) {
+    return value && !!value[Symbol.iterator];
+  };
+
   j$.isDataView = function(obj) {
     return (
       obj !== null &&
@@ -4685,23 +4689,45 @@ getJasmineRequireObj().MatchersUtil = function(j$) {
    * @returns {boolean} True if `needle` was found in `haystack`
    */
   MatchersUtil.prototype.contains = function(haystack, needle) {
-    if (j$.isSet(haystack)) {
+    if (!haystack) {
-      return haystack.has(needle);
+      return false;
     }
 
-    if (
+    if (j$.isSet(haystack)) {
-      Object.prototype.toString.apply(haystack) === '[object Array]' ||
+      // Try .has() first. It should be faster in cases where
-      (!!haystack && !haystack.indexOf)
+      // needle === something in haystack. Fall back to .equals() comparison
-    ) {
+      // if that fails.
+      if (haystack.has(needle)) {
+        return true;
+      }
+    }
+
+    if (j$.isIterable_(haystack) && !j$.isString_(haystack)) {
+      // Arrays, Sets, etc.
+      for (const candidate of haystack) {
+        if (this.equals(candidate, needle)) {
+          return true;
+        }
+      }
+
+      return false;
+    }
+
+    if (haystack.indexOf) {
+      // Mainly strings
+      return haystack.indexOf(needle) >= 0;
+    }
+
+    if (j$.isNumber_(haystack.length)) {
+      // Objects that are shaped like arrays but aren't iterable
       for (var i = 0; i < haystack.length; i++) {
         if (this.equals(haystack[i], needle)) {
           return true;
         }
       }
-      return false;
     }
 
-    return !!haystack && haystack.indexOf(needle) >= 0;
+    return false;
   };
 
   MatchersUtil.prototype.buildFailureMessage = function() {

spec/core/baseSpec.js

@@ -109,6 +109,28 @@ describe('base helpers', function() {
     });
   });
 
+  describe('isIterable_', function() {
+    it('returns true when the object is an Array', function() {
+      expect(jasmineUnderTest.isIterable_([])).toBe(true);
+    });
+
+    it('returns true when the object is a Set', function() {
+      expect(jasmineUnderTest.isIterable_(new Set())).toBe(true);
+    });
+    it('returns true when the object is a Map', function() {
+      expect(jasmineUnderTest.isIterable_(new Map())).toBe(true);
+    });
+
+    it('returns true when the object implements @@iterator', function() {
+      const myIterable = { [Symbol.iterator]: function() {} };
+      expect(jasmineUnderTest.isIterable_(myIterable)).toBe(true);
+    });
+
+    it('returns false when the object does not implement @@iterator', function() {
+      expect(jasmineUnderTest.isIterable_({})).toBe(false);
+    });
+  });
+
   describe('isPending_', function() {
     it('returns a promise that resolves to true when the promise is pending', function() {
       var promise = new Promise(function() {});

spec/core/matchers/matchersUtilSpec.js

@@ -1030,7 +1030,7 @@ describe('matchersUtil', function() {
       expect(matchersUtil.contains(null, 'A')).toBe(false);
     });
 
-    it('passes with array-like objects', function() {
+    it('works with array-like objects that implement iterable', function() {
       var capturedArgs = null,
         matchersUtil = new jasmineUnderTest.MatchersUtil();
 
@@ -1040,6 +1040,19 @@ describe('matchersUtil', function() {
 
       testFunction('foo', 'bar');
       expect(matchersUtil.contains(capturedArgs, 'bar')).toBe(true);
+      expect(matchersUtil.contains(capturedArgs, 'baz')).toBe(false);
+    });
+
+    it("passes with array-like objects that don't implement iterable", function() {
+      const arrayLike = {
+        0: 'a',
+        1: 'b',
+        length: 2
+      };
+      const matchersUtil = new jasmineUnderTest.MatchersUtil();
+
+      expect(matchersUtil.contains(arrayLike, 'b')).toBe(true);
+      expect(matchersUtil.contains(arrayLike, 'c')).toBe(false);
     });
 
     it('passes for set members', function() {
@@ -1051,13 +1064,12 @@ describe('matchersUtil', function() {
       expect(matchersUtil.contains(set, setItem)).toBe(true);
     });
 
-    // documenting current behavior
+    it('passes for objects that equal to a set member', function() {
-    it('fails (!) for objects that equal to a set member', function() {
       var matchersUtil = new jasmineUnderTest.MatchersUtil();
       var set = new Set();
       set.add({ foo: 'bar' });
 
-      expect(matchersUtil.contains(set, { foo: 'bar' })).toBe(false);
+      expect(matchersUtil.contains(set, { foo: 'bar' })).toBe(true);
     });
   });
 

src/core/base.js

@@ -161,6 +161,10 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) {
     );
   };
 
+  j$.isIterable_ = function(value) {
+    return value && !!value[Symbol.iterator];
+  };
+
   j$.isDataView = function(obj) {
     return (
       obj !== null &&

src/core/matchers/matchersUtil.js

@@ -32,23 +32,45 @@ getJasmineRequireObj().MatchersUtil = function(j$) {
    * @returns {boolean} True if `needle` was found in `haystack`
    */
   MatchersUtil.prototype.contains = function(haystack, needle) {
-    if (j$.isSet(haystack)) {
+    if (!haystack) {
-      return haystack.has(needle);
+      return false;
     }
 
-    if (
+    if (j$.isSet(haystack)) {
-      Object.prototype.toString.apply(haystack) === '[object Array]' ||
+      // Try .has() first. It should be faster in cases where
-      (!!haystack && !haystack.indexOf)
+      // needle === something in haystack. Fall back to .equals() comparison
-    ) {
+      // if that fails.
+      if (haystack.has(needle)) {
+        return true;
+      }
+    }
+
+    if (j$.isIterable_(haystack) && !j$.isString_(haystack)) {
+      // Arrays, Sets, etc.
+      for (const candidate of haystack) {
+        if (this.equals(candidate, needle)) {
+          return true;
+        }
+      }
+
+      return false;
+    }
+
+    if (haystack.indexOf) {
+      // Mainly strings
+      return haystack.indexOf(needle) >= 0;
+    }
+
+    if (j$.isNumber_(haystack.length)) {
+      // Objects that are shaped like arrays but aren't iterable
       for (var i = 0; i < haystack.length; i++) {
         if (this.equals(haystack[i], needle)) {
           return true;
         }
       }
-      return false;
     }
 
-    return !!haystack && haystack.indexOf(needle) >= 0;
+    return false;
   };
 
   MatchersUtil.prototype.buildFailureMessage = function() {