[total-functions/no-unsafe-readonly-mutable-assignment] False positive?

[ryb73]

Hi,

I'm getting the following error:

  25:3  error  Unsafe "Mutable" to "Mutable" assignment. Source: "{ c: number; } | {}", destination: "{ c: number; } | { c?: undefined; }"  total-functions/no-unsafe-readonly-mutable-assignment

On the following code:

import merge from "lodash/merge";

export function mergeImmutable<T1, T2>(v1: T1, v2: T2): T1 & T2;
export function mergeImmutable<T1, T2, T3>(
  v1: T1,
  v2: T2,
  v3: T3,
): T1 & T2 & T3;
export function mergeImmutable<T1, T2, T3, T4>(
  v1: T1,
  v2: T2,
  v3: T3,
  v4: T4,
): T1 & T2 & T3 & T4;
export function mergeImmutable<T extends Readonly<any>[]>(
  ...args: Readonly<T[]>
) {
  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
  return merge({}, ...args);
}

const x = mergeImmutable(
  { a: 1 },
  { b: 2 },
  Math.random() < 0.5 ? { c: 3 } : {},
);

I'm new to eslint-plugin-total-functions, but it seems to me that the error message doesn't make sense: it's complaining that a mutable value is being assigned to a mutable argument? Is this error being thrown incorrectly, or am I misunderstanding the message?

[danielnixon]

It's working as intended imo, though the error message could certainly do with some work.

Consider each part of the source union separately:

1. { c: number; } - mutable
2. {} - immutable

Therefore { c: number; } | {} is, overall, mutable. That explains why the error message says (confusingly) you're assigning from mutable to mutable.

But consider this scenario:

type Source = { c: number; } | {};
type Destination = { c: number; } | { c?: undefined; };

const sourceVal1 = {};

const sourceVal: Source = Date.now() > 0 ? sourceVal1 : { c: 42 };

console.log(sourceVal); // Outputs '{}'

// doesn't compile
// sourceVal.c = 24;
// doesn't compile
// sourceVal.c = undefined;
// doesn't compile
// sourceVal1.c = 24;
// doesn't compile
// sourceVal1.c = undefined;

// This is the problematic assignment (as we'll see next). It's flagged by this rule.
const destValue: Destination = sourceVal;

// this compiles, but...
destValue.c = 1;

// Whoops, we mutated a seemingly immutable value, exactly what this rule seeks to flag
console.log(sourceVal1); // Outputs '{ "c": 1 }'
console.log(sourceVal); // Outputs '{ "c": 1 }'

The error message could be improved to refer to the {} part of the union specifically, which is ultimately what's causing the issue.

[ryb73]

I think I'm starting to wrap my head around this, but it's surprisingly complicated, so bear with me.

In your example, the line const destValue: Destination = sourceVal; raises the lint error Unsafe "Mutable" to "Mutable" assignment. Source: "Source", destination: "Destination".

That line is in fact erroneous because the value stored in sourceVal could be the immutable type {}, which is incompatible with the mutable type Destination. Am I wrong in suggesting that Unsafe "*Immutable*" to "Mutable" assignment would be a more appropriate message here?

Relating this back to my example, I'm starting to grasp the problem but am still having trouble. I've come up with a simplified reproduction:

function f(x: { n?: number }) {
  x.n = 2;
}

f({ n: 1 }); // This works
f({}); // This works
f(Math.random() < 0.5 ? { n: 1 } : ({} as { n?: number })); // This works
f(Math.random() < 0.5 ? { n: 1 } : {}); // This raises a lint error (Unsafe "Mutable" to "Mutable" assignment. Source: "{ n: number; } | {}", destination: "{ n?: number | undefined; })

// For fun, a couple more examples:
const v1: {} = {};
f(v1); // This raises a lint error, as I would expect it to
const v2: { n?: number } = {};
f(v2); // This works, as expected

Given your explanation, I can kind of understand the error now: the value {} is inferred to be of type {}, which is considered to be immutable.

From a pragmatic point of view this is clearly wrong: my intent as a programmer is that the {} be interpreted as { n?: number } where the n is omitted. In fact this works fine for the f({}) call.

I suppose this is a quirk on TypeScript's end that you have no control over? I'm curious to hear your thoughts. Thanks for your consideration.