diff --git a/lib/sinon/default-behaviors.js b/lib/sinon/default-behaviors.js index 96a4b416d..d7387f7a9 100644 --- a/lib/sinon/default-behaviors.js +++ b/lib/sinon/default-behaviors.js @@ -258,7 +258,7 @@ var defaultBehaviors = { Object.defineProperty(rootStub.rootObj, rootStub.propName, { value: newVal, enumerable: true, - configurable: isPropertyConfigurable(rootStub.rootObj, rootStub.propName) + configurable: rootStub.shadowsPropOnPrototype || isPropertyConfigurable(rootStub.rootObj, rootStub.propName) }); return fake; diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 9e1e580af..dd66f346e 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -191,7 +191,11 @@ function Sandbox() { var descriptor = getPropertyDescriptor(object, property); function restorer() { - Object.defineProperty(object, property, descriptor); + if (descriptor.isOwn) { + Object.defineProperty(object, property, descriptor); + } else { + delete object[property]; + } } restorer.object = object; restorer.property = property; diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index a24d140ea..4624b9847 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -6,7 +6,7 @@ var behaviors = require("./default-behaviors"); var createProxy = require("./proxy"); var functionName = require("@sinonjs/commons").functionName; var hasOwnProperty = require("@sinonjs/commons").prototypes.object.hasOwnProperty; -var isNonExistentOwnProperty = require("./util/core/is-non-existent-own-property"); +var isNonExistentProperty = require("./util/core/is-non-existent-property"); var spy = require("./spy"); var extend = require("./util/core/extend"); var getPropertyDescriptor = require("./util/core/get-property-descriptor"); @@ -69,8 +69,8 @@ function stub(object, property) { throwOnFalsyObject.apply(null, arguments); - if (isNonExistentOwnProperty(object, property)) { - throw new TypeError("Cannot stub non-existent own property " + valueToString(property)); + if (isNonExistentProperty(object, property)) { + throw new TypeError("Cannot stub non-existent property " + valueToString(property)); } var actualDescriptor = getPropertyDescriptor(object, property); @@ -97,8 +97,9 @@ function stub(object, property) { extend.nonEnum(s, { rootObj: object, propName: property, + shadowsPropOnPrototype: !actualDescriptor.isOwn, restore: function restore() { - if (actualDescriptor !== undefined) { + if (actualDescriptor !== undefined && actualDescriptor.isOwn) { Object.defineProperty(object, property, actualDescriptor); return; } diff --git a/lib/sinon/util/core/get-property-descriptor.js b/lib/sinon/util/core/get-property-descriptor.js index 8dd3dc830..8efafc2e2 100644 --- a/lib/sinon/util/core/get-property-descriptor.js +++ b/lib/sinon/util/core/get-property-descriptor.js @@ -3,9 +3,15 @@ module.exports = function getPropertyDescriptor(object, property) { var proto = object; var descriptor; + var isOwn = Boolean(object && Object.getOwnPropertyDescriptor(object, property)); while (proto && !(descriptor = Object.getOwnPropertyDescriptor(proto, property))) { proto = Object.getPrototypeOf(proto); } + + if (descriptor) { + descriptor.isOwn = isOwn; + } + return descriptor; }; diff --git a/lib/sinon/util/core/is-non-existent-own-property.js b/lib/sinon/util/core/is-non-existent-own-property.js deleted file mode 100644 index 2df12609a..000000000 --- a/lib/sinon/util/core/is-non-existent-own-property.js +++ /dev/null @@ -1,7 +0,0 @@ -"use strict"; - -function isNonExistentOwnProperty(object, property) { - return object && typeof property !== "undefined" && !(property in object); -} - -module.exports = isNonExistentOwnProperty; diff --git a/lib/sinon/util/core/is-non-existent-property.js b/lib/sinon/util/core/is-non-existent-property.js new file mode 100644 index 000000000..f09dc3650 --- /dev/null +++ b/lib/sinon/util/core/is-non-existent-property.js @@ -0,0 +1,12 @@ +"use strict"; + +/** + * @param {*} object + * @param {String} property + * @returns whether a prop exists in the prototype chain + */ +function isNonExistentProperty(object, property) { + return object && typeof property !== "undefined" && !(property in object); +} + +module.exports = isNonExistentProperty; diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index c5343224f..56c62984b 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -580,9 +580,11 @@ describe("issues", function() { function ClassWithoutProps() { return; } + function AnotherClassWithoutProps() { return; } + ClassWithoutProps.prototype.constructor = ClassWithoutProps; AnotherClassWithoutProps.prototype.constructor = AnotherClassWithoutProps; var arg1 = new ClassWithoutProps(); //arg1.constructor.name === ClassWithoutProps @@ -640,6 +642,7 @@ describe("issues", function() { function Foo() { return; } + // eslint-disable-next-line mocha/no-setup-in-describe Foo.prototype.testMethod = function() { return; @@ -679,6 +682,7 @@ describe("issues", function() { function Foo() { return; } + // eslint-disable-next-line mocha/no-setup-in-describe Foo.prototype.testMethod = function() { return 1; @@ -709,4 +713,46 @@ describe("issues", function() { refute.equals(fake.lastArg, 3); }); }); + + describe("#2226 - props on prototype are not restored correctly", function() { + function createObjectWithPropFromPrototype() { + var proto = {}; + var obj = {}; + + Object.setPrototypeOf(obj, proto); + Object.defineProperty(proto, "test", { writable: true, value: 1 }); + return obj; + } + + it("should restore fakes shadowing prototype props correctly", function() { + var obj = createObjectWithPropFromPrototype(); + + var originalPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.replace(obj, "test", 2); + var replacedPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.restore(); + var restoredPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + assert.isUndefined(originalPropertyDescriptor); + refute.isUndefined(replacedPropertyDescriptor); + assert.isUndefined(restoredPropertyDescriptor); + }); + + it("should restore stubs shadowing prototype props correctly", function() { + var obj = createObjectWithPropFromPrototype(); + var originalPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.stub(obj, "test").value(2); + var replacedPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + sinon.restore(); + var restoredPropertyDescriptor = Object.getOwnPropertyDescriptor(obj, "test"); + + assert.isUndefined(originalPropertyDescriptor); + refute.isUndefined(replacedPropertyDescriptor); + assert.isUndefined(restoredPropertyDescriptor); + }); + }); }); diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 37c0c2eb6..c4e406ace 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -562,7 +562,7 @@ describe("Sandbox", function() { function() { sandbox.stub(object, Symbol()); }, - { message: "Cannot stub non-existent own property Symbol()" }, + { message: "Cannot stub non-existent property Symbol()" }, TypeError );