Enforce Mutable Return Types

[bitjson]

This is a follow up to @RebeccaStevens' #153 which described the issue very well:

Enforce Mutable Return Types

If the return type has to immutable then something very non-functional is going on. What happens to the data after it is returned by a function should be of no concern to the function itself. Mutable return types are more compatible with 3rd-party non-function code as readonly types can not be given as mutable types in TypeScript (while mutable types can be given as either a mutable or readonly type).

An option to enforce this behaviour should be added to the rule (whether this is the default behavior or not is currently undecided).

That issue was mostly solved (and marked as solved) by #480, but as far as I can tell, there isn't yet a way to enforce mutable return types with the new prefer-immutable-types rule.

Maybe a solution could add a "Mutable" option to the available settings for returnTypes.enforcement?

type Options = {
  // ...
  returnTypes?: {
    enforcement: "Mutable" | "None" | "ReadonlyShallow" | "ReadonlyDeep" | "Immutable";
    ignoreInferredTypes: boolean;
    ignoreClasses: boolean | "fieldsOnly";
    ignoreNamePattern?: string[] | string;
    ignoreTypePattern?: string[] | string;
  };
  // ...
};

Am I missing another way in which this feature was added? Thanks!

[RebeccaStevens]

This was researched and tested out, but the reason why this never made it in is because in practice, enforcing everything to be deeply mutable results in extremely inefficient code and has little to no benefit, and can even be a detriment.

For example, take this snippet of code:

type Foo = { hello: number };
type Bar = { world: number };

function addBar(foo: Readonly<Foo>) {
  return {
    foo,
    bar: { world: 2 },
  };
}

const foobar = addBar({ hello: 1 });

Here the return type of addBar is shallowly mutable, but it's not deeply mutable as its foo property is immutable. To make it mutable, we'd need to deeply clone the contents of foo. If the rest of the codebase is also written in a functional way, this cloning then becomes wasted effort as foo was never going to be mutated anyway.

Requiring shallow mutability instead of deep mutability may have some benefits but what is "shallow mutability" in this context? Should every top-level property be required to be mutable? Or do only some need to be?

In practice, I've found that with regard to return type immutability, it best not to enforce any level of mutability. Some function may need to use a certain level, but in general it seems to be something that shouldn't be enforced.

I'm open to feedback on this.

[bitjson]

Thanks for the response! This makes a lot of sense.

I've been requiring immutable function parameters and simply re-casting particular items as mutable prior to returning them (so no deep cloning, just some additional type syntax). After going back over most of my current usage, I don't think I'm really getting much value out of this extra syntax beyond what's already being accomplished by functional/immutable-data.

So I think I agree, in my case (a library written in a functional style) it makes sense to just cut out enforcement around mutability in parameter or return types.

I'll go ahead and close this issue, thanks!