From 612a9bd14ed7c71b6869c081097732328f01e978 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Tue, 12 Sep 2017 19:08:03 +0200 Subject: [PATCH] Remove support for stubbing undefined props This was a longer discussion that can be found in the issues #1508, #1552 and #1537 - and internally amongst the core team. Support for stubbing props using get()/set() was added in #1237 and also worked for undefined properties. This type of behavior (stubbing undefined props) has been explicitly disabled for sandboxes, so in order for Sinon to behave in a consistent way, we should do the same here. This change will bring normal stubs back to how it has used to be historically, and will make sandbox stubs and normal sinon stubs behave the same. It might break things for people relying on the behavior that has been present since Sinon 2.0, but it should make things more reliable going forward. --- lib/sinon/stub.js | 5 ++++ test/stub-test.js | 68 +++++++---------------------------------------- 2 files changed, 15 insertions(+), 58 deletions(-) diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index a7c32ad39..a21544d83 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -9,6 +9,7 @@ var getPropertyDescriptor = require("./util/core/get-property-descriptor"); var wrapMethod = require("./util/core/wrap-method"); var stubEntireObject = require("./stub-entire-object"); var throwOnFalsyObject = require("./throw-on-falsy-object"); +var valueToString = require("./util/core/value-to-string"); var slice = Array.prototype.slice; @@ -19,6 +20,10 @@ function stub(object, property) { throwOnFalsyObject.apply(null, arguments); + if (object && typeof property !== "undefined" && !(property in object)) { + throw new TypeError("Cannot stub non-existent own property " + valueToString(property)); + } + var actualDescriptor = getPropertyDescriptor(object, property); var isStubbingEntireObject = typeof property === "undefined" && typeof object === "object"; var isCreatingNewStub = !object && typeof property === "undefined"; diff --git a/test/stub-test.js b/test/stub-test.js index 9b19268fe..5b225e9ee 100644 --- a/test/stub-test.js +++ b/test/stub-test.js @@ -5,7 +5,6 @@ var createStub = require("../lib/sinon/stub"); var createStubInstance = require("../lib/sinon/stub").createStubInstance; var createSpy = require("../lib/sinon/spy"); var sinonMatch = require("../lib/sinon/match"); -var getPropertyDescriptor = require("../lib/sinon/util/core/get-property-descriptor"); var deprecated = require("../lib/sinon/util/core/deprecated"); var assert = referee.assert; var refute = referee.refute; @@ -1164,6 +1163,16 @@ describe("stub", function () { stub.restore(); }); + + it("throws if stubbing non-existent property", function () { + var myObj = {}; + + assert.exception(function () { + createStub(myObj, "ouch"); + }); + + refute.defined(myObj.ouch); + }); }); describe("stubbed function", function () { @@ -2579,16 +2588,6 @@ describe("stub", function () { assert.equals(myObj.prop, "bar"); }); - it("can set getters for non-existing properties", function () { - var myObj = {}; - - createStub(myObj, "prop").get(function getterFn() { - return "bar"; - }); - - assert.equals(myObj.prop, "bar"); - }); - it("can restore stubbed setters for functions", function () { var propFn = function propFn() { return "bar"; @@ -2627,19 +2626,6 @@ describe("stub", function () { assert.equals(myObj.prop, "bar"); }); - it("can restore stubbed getters for previously undefined properties", function () { - var myObj = {}; - - var stub = createStub(myObj, "nonExisting"); - - stub.get(function getterFn() { - return "baz"; - }); - - stub.restore(); - - assert.equals(getPropertyDescriptor(myObj, "nonExisting"), undefined); - }); }); describe(".set", function () { @@ -2689,18 +2675,6 @@ describe("stub", function () { assert.equals(myObj.example, "bar"); }); - it("can set setters for non-existing properties", function () { - var myObj = {}; - - createStub(myObj, "prop").set(function setterFn() { - myObj.example = "bar"; - }); - - myObj.prop = "foo"; - - assert.equals(myObj.example, "bar"); - }); - it("can restore stubbed setters for functions", function () { var propFn = function propFn() { return "bar"; @@ -2741,19 +2715,6 @@ describe("stub", function () { assert.equals(myObj.otherProp, "bar"); }); - it("can restore stubbed setters for previously undefined properties", function () { - var myObj = {}; - - var stub = createStub(myObj, "nonExisting"); - - stub.set(function setterFn() { - myObj.otherProp = "baz"; - }); - - stub.restore(); - - assert.equals(getPropertyDescriptor(myObj, "nonExisting"), undefined); - }); }); describe(".value", function () { @@ -2777,15 +2738,6 @@ describe("stub", function () { assert.equals(myObj.prop, "rawString"); }); - it("allows restoring previously undefined properties", function () { - var obj = {}; - var stub = createStub(obj, "nonExisting").value(2); - - stub.restore(); - - assert.equals(getPropertyDescriptor(obj, "nonExisting"), undefined); - }); - it("allows stubbing function static properties", function () { var myFunc = function () {}; myFunc.prop = "rawString";