gajus / eslint-plugin-jsdoc

JSDoc specific linting rules for ESLint.
Other
1.09k stars 157 forks source link

Custom tag (`@readonly-properties`?) which enforced that a function would not modify passed-in and/or closure-obtained objects unless whitelisted #309

Open brettz9 opened 5 years ago

brettz9 commented 5 years ago

Similar in some ways to #128, this idea would be to add a custom tag which prevents a function from modifying any properties on an object/array/function that is dropped in via closure.

--- Want to back this issue? **[Post a bounty on it!](https://app.bountysource.com/issues/76498206-custom-tag-readonly-properties-which-enforced-that-a-function-would-not-modify-passed-in-and-or-closure-obtained-objects-unless-whitelisted?utm_campaign=plugin&utm_content=tracker%2F23037809&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://app.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F23037809&utm_medium=issues&utm_source=github).
golopot commented 5 years ago

I am uncomfortable with inventing a new tag in this project. Also the functionality sounds like a job for type-checker. You can do this now if you use typescript's "checkjs".

/**
 * @param {Readonly<{foo: number}>} a - either this
 * @param {{readonly foo: number}} b - or this
 */
function f (a, b) {
  a.foo = 1 // Cannot assign to 'foo' because it is a read-only property.
  b.foo = 1 // Cannot assign to 'foo' because it is a read-only property.
}
brettz9 commented 5 years ago

I haven't thought through the precise API I'd like, but please note that gajus has already approved in concept a custom @use tag per #128.

These ideas are for enforcing disciplined coding practices for which no enforcement mechanisms exist (and jsdoc of course makes provision for custom tags).

Note that while I am aware this overlaps with some TypeScript functionality, this issue is not limited to parameters; it relates to closure variables as well, adjusting your example (noting though that a and b are no longer function parameters as in your example):

/* eslint jsdoc/property-access: {default: "forbid"} */

var a = {};
var b = {};

/**
 * @property-write-access a
 */
function f () {
  a.foo = 1 // Ok.
  b.foo = 1 // Cannot assign to 'foo' because parent object `b` is not whitelisted for writable-properties in this function.
}

Since JavaScript does not allow easily dropping in (or passing in or accepting) objects by value rather than reference, this proposal aims to make clear to those looking at the (nested) function--which properties it is acting on, whether it has side effects, and whether it can be easily extracted out of a nested context for use elsewhere (and to prevent unintended modifications/dependencies).

gajus commented 5 years ago

I don't see a particular issue with adding ESLint-plugin specific tag.

The bigger question is adoption VS cost of maintenance, i.e. how many people are going to use this VS how much time will need to be spent maintaining, documenting, testing the functionality. As far as I can tell at this stage, it is going to be a fairly isolated component, therefore as long as @brettz9 is keen to develop it and promote it (we can add "experiment" flag in documentation or something), then I am happy with this being added. Worst comes to worse, if there is no adoption and @brettz9 no longer supports development of this functionality, then it can be pulled out into a dedicated plugin.