Fixed toHaveProperty returning false positives when looking for undefined property (#8923)

Author: SixTfour

CHANGELOG.md

@@ -35,6 +35,7 @@
 - `[jest-config]` Pass `moduleTypes` to `ts-node` to enforce CJS when transpiling ([#12397](https://github.com/facebook/jest/pull/12397))
 - `[jest-config, jest-haste-map]` Allow searching for tests in `node_modules` by exposing `retainAllFiles` ([#11084](https://github.com/facebook/jest/pull/11084))
 - `[jest-environment-jsdom]` Make `jsdom` accessible to extending environments again ([#12232](https://github.com/facebook/jest/pull/12232))
+- `[@jest/expect-utils]` [**BREAKING**] Fix false positives when looking for `undefined` prop ([#8923](https://github.com/facebook/jest/pull/8923))
 - `[jest-haste-map]` Don't use partial results if file crawl errors ([#12420](https://github.com/facebook/jest/pull/12420))
 - `[jest-jasmine2, jest-types]` [**BREAKING**] Move all `jasmine` specific types from `@jest/types` to its own package ([#12125](https://github.com/facebook/jest/pull/12125))
 - `[jest-matcher-utils]` Pass maxWidth to `pretty-format` to avoid printing every element in arrays by default ([#12402](https://github.com/facebook/jest/pull/12402))

packages/expect-utils/src/__tests__/utils.test.ts

@@ -19,13 +19,15 @@ import {
 describe('getPath()', () => {
   test('property exists', () => {
     expect(getPath({a: {b: {c: 5}}}, 'a.b.c')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: {c: 5},
       traversedPath: ['a', 'b', 'c'],
       value: 5,
     });
 
     expect(getPath({a: {b: {c: {d: 1}}}}, 'a.b.c.d')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: {d: 1},
       traversedPath: ['a', 'b', 'c', 'd'],
@@ -35,6 +37,7 @@ describe('getPath()', () => {
 
   test('property doesnt exist', () => {
     expect(getPath({a: {b: {}}}, 'a.b.c')).toEqual({
+      endPropIsDefined: false,
       hasEndProp: false,
       lastTraversedObject: {},
       traversedPath: ['a', 'b'],
@@ -44,6 +47,7 @@ describe('getPath()', () => {
 
   test('property exist but undefined', () => {
     expect(getPath({a: {b: {c: undefined}}}, 'a.b.c')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: {c: undefined},
       traversedPath: ['a', 'b', 'c'],
@@ -62,12 +66,14 @@ describe('getPath()', () => {
     }
 
     expect(getPath(new A(), 'a')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: new A(),
       traversedPath: ['a'],
       value: 'a',
     });
     expect(getPath(new A(), 'b.c')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: {c: 'c'},
       traversedPath: ['b', 'c'],
@@ -81,6 +87,7 @@ describe('getPath()', () => {
     A.prototype.a = 'a';
 
     expect(getPath(new A(), 'a')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: new A(),
       traversedPath: ['a'],
@@ -99,6 +106,7 @@ describe('getPath()', () => {
 
   test('empty object at the end', () => {
     expect(getPath({a: {b: {c: {}}}}, 'a.b.c.d')).toEqual({
+      endPropIsDefined: false,
       hasEndProp: false,
       lastTraversedObject: {},
       traversedPath: ['a', 'b', 'c'],

packages/expect-utils/src/utils.ts

@@ -16,6 +16,7 @@ import {
 
 type GetPath = {
   hasEndProp?: boolean;
+  endPropIsDefined?: boolean;
   lastTraversedObject: unknown;
   traversedPath: Array<string>;
   value?: unknown;
@@ -74,8 +75,8 @@ export const getPath = (
       // Does object have the property with an undefined value?
       // Although primitive values support bracket notation (above)
       // they would throw TypeError for in operator (below).
-      result.hasEndProp =
-        newObject !== undefined || (!isPrimitive(object) && prop in object);
+      result.endPropIsDefined = !isPrimitive(object) && prop in object;
+      result.hasEndProp = newObject !== undefined || result.endPropIsDefined;
 
       if (!result.hasEndProp) {
         result.traversedPath.shift();

packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap

@@ -3392,6 +3392,16 @@ Expected value: <g>undefined</>
 Received value: <r>3</>
 `;
 
+exports[`.toHaveProperty() {pass: false} expect({"a": {}}).toHaveProperty('a.b', undefined) 1`] = `
+<d>expect(</><r>received</><d>).</>toHaveProperty<d>(</><g>path</><d>, </><g>value</><d>)</>
+
+Expected path: <g>"a.b"</>
+Received path: <r>"a"</>
+
+Expected value: <g>undefined</>
+Received value: <r>{}</>
+`;
+
 exports[`.toHaveProperty() {pass: false} expect({"a": 1}).toHaveProperty('a.b.c.d') 1`] = `
 <d>expect(</><r>received</><d>).</>toHaveProperty<d>(</><g>path</><d>)</>
 
@@ -3680,18 +3690,6 @@ Expected path: <g>"a.b"</>
 Expected value: not <g>undefined</>
 `;
 
-exports[`.toHaveProperty() {pass: true} expect({"a": {}}).toHaveProperty('a.b', undefined) 1`] = `
-<d>expect(</><r>received</><d>).</>not<d>.</>toHaveProperty<d>(</><g>path</><d>, </><g>value</><d>)</>
-
-Expected path: <g>"a.b"</>
-Received path: <r>"a"</>
-
-Expected value: not <g>undefined</>
-Received value:     <r>{}</>
-
-<d>Because a positive assertion passes for expected value undefined if the property does not exist, this negative assertion fails unless the property does exist and has a defined value</>
-`;
-
 exports[`.toHaveProperty() {pass: true} expect({"a": 0}).toHaveProperty('a') 1`] = `
 <d>expect(</><r>received</><d>).</>not<d>.</>toHaveProperty<d>(</><g>path</><d>)</>
 

packages/expect/src/__tests__/matchers.test.js

@@ -1883,7 +1883,6 @@ describe('.toHaveProperty()', () => {
     [{a: {b: [1, 2, 3]}}, ['a', 'b', 1], expect.any(Number)],
     [{a: 0}, 'a', 0],
     [{a: {b: undefined}}, 'a.b', undefined],
-    [{a: {}}, 'a.b', undefined], // delete for breaking change in future major
     [{a: {b: {c: 5}}}, 'a.b', {c: 5}],
     [{a: {b: [{c: [{d: 1}]}]}}, 'a.b[0].c[0].d', 1],
     [{a: {b: [{c: {d: [{e: 1}, {f: 2}]}}]}}, 'a.b[0].c.d[1].f', 2],
@@ -1927,7 +1926,7 @@ describe('.toHaveProperty()', () => {
     [{a: {b: {c: 5}}}, 'a.b', {c: 4}],
     [new Foo(), 'a', 'a'],
     [new Foo(), 'b', undefined],
-    // [{a: {}}, 'a.b', undefined], // add for breaking change in future major
+    [{a: {}}, 'a.b', undefined],
   ].forEach(([obj, keyPath, value]) => {
     test(`{pass: false} expect(${stringify(
       obj,

packages/expect/src/matchers.ts

@@ -719,37 +719,15 @@ const matchers: MatchersObject = {
     }
 
     const result = getPath(received, expectedPath);
-    const {lastTraversedObject, hasEndProp} = result;
+    const {lastTraversedObject, endPropIsDefined, hasEndProp, value} = result;
     const receivedPath = result.traversedPath;
     const hasCompletePath = receivedPath.length === expectedPathLength;
     const receivedValue = hasCompletePath ? result.value : lastTraversedObject;
 
-    const pass = hasValue
-      ? equals(result.value, expectedValue, [iterableEquality])
-      : Boolean(hasEndProp); // theoretically undefined if empty path
-    // Remove type cast if we rewrite getPath as iterative algorithm.
-
-    // Delete this unique report if future breaking change
-    // removes the edge case that expected value undefined
-    // also matches absence of a property with the key path.
-    if (pass && !hasCompletePath) {
-      const message = () =>
-        matcherHint(matcherName, undefined, expectedArgument, options) +
-        '\n\n' +
-        `Expected path: ${printExpected(expectedPath)}\n` +
-        `Received path: ${printReceived(
-          expectedPathType === 'array' || receivedPath.length === 0
-            ? receivedPath
-            : receivedPath.join('.'),
-        )}\n\n` +
-        `Expected value: not ${printExpected(expectedValue)}\n` +
-        `Received value:     ${printReceived(receivedValue)}\n\n` +
-        DIM_COLOR(
-          'Because a positive assertion passes for expected value undefined if the property does not exist, this negative assertion fails unless the property does exist and has a defined value',
-        );
-
-      return {message, pass};
-    }
+    const pass =
+      hasValue && endPropIsDefined
+        ? equals(value, expectedValue, [iterableEquality])
+        : Boolean(hasEndProp);
 
     const message = pass
       ? () =>