eslint-plugin-total-functions icon indicating copy to clipboard operation
eslint-plugin-total-functions copied to clipboard

[no-unsafe-mutable-readonly-assignment] impractical to use with many readonly array methods

Open danielnixon opened this issue 3 years ago • 4 comments

As discussed in https://github.com/danielnixon/readonly-types/issues/7, lots of readonly array methods return mutable arrays. An example of this is concat. So if you have code like the following very common example, you will run afoul of the no-unsafe-mutable-readonly-assignment rule:

const foo: ReadonlyArray<"string"> = ["foo"] as const;
// concat returns a mutable array,
// which we try to assign to an readonly array,
// which triggers no-unsafe-mutable-readonly-assignment
const bar: ReadonlyArray<"string"> = foo.concat("bar");

This situation renders the no-unsafe-mutable-readonly-assignment rule impractical for all but the most hardcore readonly aficionados. If https://github.com/danielnixon/readonly-types/issues/7 were to deliver a VeryReadonlyArray we could recommend the use of that, but that's probably too much of an imposition.

We don't want to allow mutable -> readonly assignment in the general case, because in principle that mutable value could be retained and used to cause surprising mutation in the readonly value at some point in the future. In the case of array methods like concat we know that no references to the returned array is retained, so we should relax no-unsafe-mutable-readonly-assignment to not flag such cases.

danielnixon avatar Sep 25 '20 05:09 danielnixon

In fact it would probably make sense to implement this in a way that users can specify as an option a list of types and methods that should be ignored (as long as their return values are immediately assigned to a readonly value).

That way users can ignore methods in libraries that are equivalent to the concat situation.

danielnixon avatar Oct 09 '20 22:10 danielnixon

@willheslam I'd welcome contributions here if you're interested in taking a crack ;-)

danielnixon avatar Oct 09 '20 22:10 danielnixon

@danielnixon It's a little daunting but I will take a look at it this weekend! :)

I have a few examples of seemingly false positive "readonly assigned to mutative array" scenarios that it would be useful to catalogue regardless!

willheslam avatar Oct 09 '20 23:10 willheslam

I have a few examples of seemingly false positive "readonly assigned to mutative array" scenarios

I've fixed one of those just now: https://github.com/danielnixon/eslint-plugin-total-functions/commit/95acfaea315ada82abf298e10fe2f7f4a0c4c2f0

danielnixon avatar Oct 10 '20 05:10 danielnixon

This issue is less critical now that we have PrincipledArray:

https://github.com/agiledigital/readonly-types/issues/7#issuecomment-1416679052 https://github.com/agiledigital/readonly-types/blob/master/src/index.test.ts#L170-L278

danielnixon avatar Feb 04 '23 06:02 danielnixon

Nice work @haolinj 🚀

danielnixon avatar Mar 03 '23 05:03 danielnixon