Enable use of assignment in the presence of accessors (#2538)

Author: fatso83

.eslintignore

@@ -8,6 +8,7 @@ docs/releases/
 docs/_releases/
 docs/assets/js/
 docs/vendor
+vendor/
 
 # has linting of its own
 docs/release-source/release/examples

docs/release-source/release.md

@@ -36,5 +36,5 @@ If you need to support old runtimes you can try [Sinon 9][compat-doc-v9].
 
 {% include docs/contribute.md %}
 
-[compat-doc]: https://github.com/sinonjs/sinon/COMPATIBILITY.md
+[compat-doc]: https://github.com/sinonjs/sinon/blob/main/COMPATIBILITY.md
 [compat-doc-v9]: https://github.com/sinonjs/sinon/blob/v9.2.4/COMPATIBILITY.md

docs/release-source/release/sandbox.md

@@ -236,9 +236,17 @@ console.log(myObject.myMethod());
 // strawberry
 ```
 
-#### `sandbox.replaceGetter();`
+#### `sandbox.replace.usingAccessor(object, property, value);`
 
-Replaces getter for `property` on `object` with `replacement` argument. Attempts to replace an already replaced getter cause an exception.
+Usually one intends to _replace_ the value or getter of a field, but there are use cases where one actually wants to _assign_ a value to a property using an existing setter. `#replace.usingAccessor(object, property, value)` will do just that; pass the value into setter function and vice-versa use the getter to get the value used for restoring later on.
+
+##### Use case: no-frills dependency injection in ESM with cleanup
+
+One use case can be to conveniently allow ESM module stubbing using pure dependency injection, having Sinon help you with the cleanup, without resorting to external machinery such as module loaders or require hooks (see [#2403](https://github.com/sinonjs/sinon/issues/2403)). This would then work regardless of bundler, browser or server environment.
+
+#### `sandbox.replaceGetter(object, property, replacementFunction);`
+
+Replaces an existing getter for `property` on `object` with the `replacementFunction` argument. Attempts to replace an already replaced getter cause an exception.
 
 `replacement` must be a `Function`, and can be instances of `spies`, `stubs` and `fakes`.
 
@@ -257,9 +265,9 @@ console.log(myObject.myProperty);
 // strawberry
 ```
 
-#### `sandbox.replaceSetter();`
+#### `sandbox.replaceSetter(object, property, replacementFunction);`
 
-Replaces setter for `property` on `object` with `replacement` argument. Attempts to replace an already replaced setter cause an exception.
+Replaces an existing setter for `property` on `object` with the `replacementFunction` argument. Attempts to replace an already replaced setter cause an exception.
 
 `replacement` must be a `Function`, and can be instances of `spies`, `stubs` and `fakes`.
 

lib/sinon/sandbox.js

@@ -35,6 +35,40 @@ function applyOnEach(fakes, method) {
     });
 }
 
+function throwOnAccessors(descriptor) {
+    if (typeof descriptor.get === "function") {
+        throw new Error("Use sandbox.replaceGetter for replacing getters");
+    }
+
+    if (typeof descriptor.set === "function") {
+        throw new Error("Use sandbox.replaceSetter for replacing setters");
+    }
+}
+
+function verifySameType(object, property, replacement) {
+    if (typeof object[property] !== typeof replacement) {
+        throw new TypeError(
+            `Cannot replace ${typeof object[
+                property
+            ]} with ${typeof replacement}`
+        );
+    }
+}
+
+function checkForValidArguments(descriptor, property, replacement) {
+    if (typeof descriptor === "undefined") {
+        throw new TypeError(
+            `Cannot replace non-existent property ${valueToString(
+                property
+            )}. Perhaps you meant sandbox.define()?`
+        );
+    }
+
+    if (typeof replacement === "undefined") {
+        throw new TypeError("Expected replacement argument to be defined");
+    }
+}
+
 function Sandbox() {
     const sandbox = this;
     let fakeRestorers = [];
@@ -66,11 +100,6 @@ function Sandbox() {
         return collection;
     };
 
-    // this is for testing only
-    sandbox.getRestorers = function () {
-        return fakeRestorers;
-    };
-
     sandbox.createStubInstance = function createStubInstance() {
         const stubbed = sinonCreateStubInstance.apply(null, arguments);
 
@@ -196,11 +225,22 @@ function Sandbox() {
         sandbox.injectedKeys.length = 0;
     };
 
-    function getFakeRestorer(object, property) {
+    /**
+     * Creates a restorer function for the property
+     *
+     * @param {object|Function} object
+     * @param {string} property
+     * @param {boolean} forceAssignment
+     * @returns {Function} restorer function
+     */
+    function getFakeRestorer(object, property, forceAssignment = false) {
         const descriptor = getPropertyDescriptor(object, property);
+        const value = object[property];
 
         function restorer() {
-            if (descriptor?.isOwn) {
+            if (forceAssignment) {
+                object[property] = value;
+            } else if (descriptor?.isOwn) {
                 Object.defineProperty(object, property, descriptor);
             } else {
                 delete object[property];
@@ -225,41 +265,43 @@ function Sandbox() {
         });
     }
 
+    /**
+     * Replace an existing property
+     *
+     * @param {object|Function} object
+     * @param {string} property
+     * @param {*} replacement a fake, stub, spy or any other value
+     * @returns {*}
+     */
     sandbox.replace = function replace(object, property, replacement) {
         const descriptor = getPropertyDescriptor(object, property);
+        checkForValidArguments(descriptor, property, replacement);
+        throwOnAccessors(descriptor);
+        verifySameType(object, property, replacement);
 
-        if (typeof descriptor === "undefined") {
-            throw new TypeError(
-                `Cannot replace non-existent property ${valueToString(
-                    property
-                )}. Perhaps you meant sandbox.define()?`
-            );
-        }
+        verifyNotReplaced(object, property);
 
-        if (typeof replacement === "undefined") {
-            throw new TypeError("Expected replacement argument to be defined");
-        }
+        // store a function for restoring the replaced property
+        push(fakeRestorers, getFakeRestorer(object, property));
 
-        if (typeof descriptor.get === "function") {
-            throw new Error("Use sandbox.replaceGetter for replacing getters");
-        }
+        object[property] = replacement;
 
-        if (typeof descriptor.set === "function") {
-            throw new Error("Use sandbox.replaceSetter for replacing setters");
-        }
+        return replacement;
+    };
 
-        if (typeof object[property] !== typeof replacement) {
-            throw new TypeError(
-                `Cannot replace ${typeof object[
-                    property
-                ]} with ${typeof replacement}`
-            );
-        }
+    sandbox.replace.usingAccessor = function replaceUsingAccessor(
+        object,
+        property,
+        replacement
+    ) {
+        const descriptor = getPropertyDescriptor(object, property);
+        checkForValidArguments(descriptor, property, replacement);
+        verifySameType(object, property, replacement);
 
         verifyNotReplaced(object, property);
 
         // store a function for restoring the replaced property
-        push(fakeRestorers, getFakeRestorer(object, property));
+        push(fakeRestorers, getFakeRestorer(object, property, true));
 
         object[property] = replacement;
 

package.json

@@ -47,7 +47,7 @@
     "dev-docs": "cd docs; cp -rl release-source releases/dev; npm run serve-docs",
     "build-docs": "cd docs; bundle exec jekyll build",
     "serve-docs": "cd docs; bundle exec jekyll serve --incremental --verbose --livereload",
-    "lint": "eslint --max-warnings 99 '**/*.{js,cjs,mjs}'",
+    "lint": "eslint --max-warnings 101 '**/*.{js,cjs,mjs}'",
     "unimported": "unimported .",
     "pretest-webworker": "npm run build",
     "prebuild": "rimraf pkg && npm run check-dependencies",

test/sandbox-test.js

@@ -398,6 +398,7 @@ describe("Sandbox", function () {
                 constructor() {
                     this.privateGetter = () => 42;
                 }
+
                 getValue() {
                     return this.privateGetter();
                 }
@@ -1072,8 +1073,8 @@ describe("Sandbox", function () {
             assert.equals(actual, replacement);
         });
 
-        describe("when asked to replace a getter", function () {
-            it("should throw an Error", function () {
+        describe("when asked to replace a property with a getter", function () {
+            it("should throw an Error by default that informs of replaceGetter", function () {
                 const sandbox = this.sandbox;
                 const object = {
                     get foo() {
@@ -1093,7 +1094,7 @@ describe("Sandbox", function () {
             });
         });
 
-        describe("when asked to replace a setter", function () {
+        describe("when asked to replace a property with a setter", function () {
             it("should throw an Error", function () {
                 const sandbox = this.sandbox;
                 const object = {
@@ -1116,6 +1117,28 @@ describe("Sandbox", function () {
         });
     });
 
+    describe(".replace.usingAccessor", function () {
+        it("should allow using assignment when replacing a value", function () {
+            const sandbox = createSandbox();
+            let quaziPrivateStateOfObject = "original";
+            const object = {
+                // eslint-disable-next-line accessor-pairs
+                get foo() {
+                    return quaziPrivateStateOfObject;
+                },
+                set foo(value) {
+                    quaziPrivateStateOfObject = value;
+                },
+            };
+
+            assert.equals(object.foo, "original");
+            sandbox.replace.usingAccessor(object, "foo", "fake");
+            assert.equals(object.foo, "fake");
+            sandbox.restore();
+            assert.equals(object.foo, "original");
+        });
+    });
+
     describe(".replaceGetter", function () {
         beforeEach(function () {
             this.sandbox = createSandbox();
@@ -1857,8 +1880,12 @@ describe("Sandbox", function () {
             /* eslint-disable no-empty-function, accessor-pairs */
             this.sandbox.inject(this.obj);
 
-            const myObj = { a: function () {} };
+            const myObj = {
+                a: function () {},
+            };
+
             function MyClass() {}
+
             MyClass.prototype.method1 = noop;
             Object.defineProperty(myObj, "b", {
                 get: function () {