Fixed toHaveProperty returning false positives when looking for undefined property

CHANGELOG.md

@@ -20,6 +20,7 @@
 
 ### Fixes
 
+- `[expect]` Fix false positives when looking for undefined prop ([#8786](https://github.com/facebook/jest/issues/8786))
 - `[expect]` Display expectedDiff more carefully in toBeCloseTo ([#8389](https://github.com/facebook/jest/pull/8389))
 - `[jest-diff]` Do not inverse format if line consists of one change ([#8903](https://github.com/facebook/jest/pull/8903))
 - `[jest-fake-timers]` `getTimerCount` will not include cancelled immediates ([#8764](https://github.com/facebook/jest/pull/8764))

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

@@ -3083,6 +3083,16 @@ Expected value: <green>undefined</>
 Received value: <red>3</>"
 `;
 
+exports[`.toHaveProperty() {pass: false} expect({"a": {}}).toHaveProperty('a.b', undefined) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toHaveProperty<dim>(</><green>path</><dim>, </><green>value</><dim>)</>
+
+Expected path: <green>\\"a.b\\"</>
+Received path: <red>\\"a\\"</>
+
+Expected value: <green>undefined</>
+Received value: <red>{}</>"
+`;
+
 exports[`.toHaveProperty() {pass: false} expect({"a": 1}).toHaveProperty('a.b.c.d') 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toHaveProperty<dim>(</><green>path</><dim>)</>
 
@@ -3339,18 +3349,6 @@ Expected path: <green>\\"a.b\\"</>
 Expected value: not <green>undefined</>"
 `;
 
-exports[`.toHaveProperty() {pass: true} expect({"a": {}}).toHaveProperty('a.b', undefined) 1`] = `
-"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveProperty<dim>(</><green>path</><dim>, </><green>value</><dim>)</>
-
-Expected path: <green>\\"a.b\\"</>
-Received path: <red>\\"a\\"</>
-
-Expected value: not <green>undefined</>
-Received value:     <red>{}</>
-
-<dim>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`] = `
 "<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveProperty<dim>(</><green>path</><dim>)</>
 

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

@@ -1659,7 +1659,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}],
     [Object.assign(Object.create(null), {property: 1}), 'property', 1],
     [new Foo(), 'a', undefined],
@@ -1699,7 +1698,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/__tests__/utils.test.js

@@ -21,13 +21,15 @@ const {
 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'],
@@ -37,6 +39,7 @@ describe('getPath()', () => {
 
   test('property doesnt exist', () => {
     expect(getPath({a: {b: {}}}, 'a.b.c')).toEqual({
+      endPropIsDefined: false,
       hasEndProp: false,
       lastTraversedObject: {},
       traversedPath: ['a', 'b'],
@@ -46,6 +49,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'],
@@ -64,12 +68,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'],
@@ -82,6 +88,7 @@ describe('getPath()', () => {
     A.prototype.a = 'a';
 
     expect(getPath(new A(), 'a')).toEqual({
+      endPropIsDefined: true,
       hasEndProp: true,
       lastTraversedObject: new A(),
       traversedPath: ['a'],
@@ -100,6 +107,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/src/matchers.ts

@@ -730,37 +730,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
       ? () =>

packages/expect/src/utils.ts

@@ -16,6 +16,7 @@ import {
 
 type GetPath = {
   hasEndProp?: boolean;
+  endPropIsDefined?: boolean;
   lastTraversedObject: unknown;
   traversedPath: Array<string>;
   value?: unknown;
@@ -84,8 +85,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();