[prefer-readonly-type] doesn't catch mutable maps and sets

[danielnixon]

Examples:

// prefer-readonly-type flags this
// eslint-disable-next-line functional/prefer-readonly-type
const foo = new Array<string>();

// but not this
const bar = new Set<string>();

// nor this
const baz = new Map<string, string>();

And with explicit readonly type annotation:

// this readonly type annotation appeases prefer-readonly-type
const foo: readonly string[] = new Array<string>();

// so ideally these would too (if/when Set and Map are flagged)
const bar: ReadonlySet<string> = new Set<string>();
const baz: ReadonlyMap<string, string> = new Map<string, string>();

ReadonlySet and ReadonlyMap are built-in TypeScript types, but they don't enjoy special syntax support like arrays do (you can't say readonly Set<string>, you must say ReadonlySet<string>). For this reason I suspect this issue is a cousin to #150 and #51.

[danielnixon]

This seems like it could be a duplicate of #25, but even with checkImplicit true those constructor calls aren't flagged. 🤔

[RebeccaStevens]

Readonly sets and maps are already supposed to supported.
We currently only have 2 tests that test them though so it might be the case that they aren't being supported in all cases.

Set test: https://github.com/jonaskello/eslint-plugin-functional/blob/master/tests/rules/prefer-readonly-type.test.ts#L395-L409
Map test: https://github.com/jonaskello/eslint-plugin-functional/blob/master/tests/rules/prefer-readonly-type.test.ts#L411-L427

I'll need to look more into this at a later date.

[RebeccaStevens]

No one else has reported this issue and its been open for quite a while now. I'm going to close this an assume everything is ok.
If you are still having this issue; please open a new issue.

[danielnixon]

The situation is unchanged with the latest version:

[danielnixon]

I can guess why nobody else has reported the issue: arrays are omnipresent and so the (negative) consequences of them being mutable are most commonly encountered and highest priority to guard against. Sets and maps are comparatively less common and (speculating) when they are used their scope is much less than arrays (which get sprayed everywhere).

Nevertheless, TypeScript does provide ReadonlySet and ReadonlyMap types out of the box, so it would be nice for an ESLint rule to treat them the same as Array and ReadonlyrArray.

I won't create a new issue because it would just be identical to this one. I'll see if I can find some time to raise a PR.

[RebeccaStevens]

The prefer-readonly-type rule has been deprecated with the release of 5.0
Hopefully you shouldn't have this issue with the new prefer-immutable-types rule. Please report back if you are.

[danielnixon]

Oh, I just noticed 5.0.0. It's two hours old! I'll report back but the docs look great (https://github.com/eslint-functional/eslint-plugin-functional/blob/main/docs/rules/type-declaration-immutability.md 🚀).

Thanks @RebeccaStevens