feat(lint): add useReadonlyClassProperties rule

[seeintea]

Summary

Resolves #4356

add useReadonlyClassProperties rule

add options checkAllProperties

Test Plan

Added tests and snapshots

[codspeed-hq]

CodSpeed Performance Report

Merging #4507 will not alter performance

_{Comparing seeintea:feat/use_readonly_class_properties (9f24fe8) with main (d319d40)}

Summary

✅ 95 untouched benchmarks

[arendjr]

Nice work!

Main question: How do TypeScript's semantics affect interior mutability again? For instance, this member:

private readonly numbers: Array<number>;

Does this protect only against reassignment of numbers? Or is this.numbers.push(1); off-limits too now? I know there's the ReadonlyArray type in TS, but I always forget if it gets implied by the readonly keyword or not 😅

[arendjr]

Did you mean something like this:

Suggested change

	/// This rule reports on private members are marked as readonly
	/// if they're never modified outside of the constructor.
	/// This rule reports on private members and marks them as `readonly`
	/// if they're never modified outside of the constructor.

[arendjr]

Suggested change

	/// Check on all properties (`public` and `protected` properties). Default: false.
	/// Check on all properties (`public` and `protected` properties). Default: `false`.

[arendjr]

We generally provide two sections of examples: ### Invalid and ### Valid (in that order). This helps people to understand the scope of the rule.

[arendjr]

Nit: I think this is a little more idiomatic in Rust:

Suggested change

	if eligible {
	return Some(property_parameter.into());
	}
	None
	eligible.then(|| property_parameter.into())

[arendjr]

I guess you may want to cover other mutating expressions as well, such as the += operator?

[seeintea]

+= operator is also of JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION

I'll try to test more types tomorrow (It's nighttime here for me 🌙)

[arendjr]

Ah, great!

[seeintea]

Nice work!

Main question: How do TypeScript's semantics affect interior mutability again? For instance, this member:

private readonly numbers: Array<number>;

Does this protect only against reassignment of numbers? Or is this.numbers.push(1); off-limits too now? I know there's the ReadonlyArray type in TS, but I always forget if it gets implied by the readonly keyword or not 😅

I just tested typescript-eslint

It only seems to care if the property is reassignment or not

private numbers: Array<number>;

// It will still prompt  [Member 'number' is never reassigned; mark it as `readonly`.]
this.number.push(1);
// but it won't be prompted.
this.number = [1];

So I think he only detects if a memory address has been modified.

If want to disable this.numbers.push(1); need use ReadonlyArray

private numbers: ReadonlyArray<number>;
// illegal operation
this.numbers.push(1); 

[Conaclos]

Why not just use a Vec?

Suggested change

	) -> FxHashSet<AnyClassProperties> {
	) -> Vec<AnyClassProperties> {

[Conaclos]

Suggested change

	.name_token()
	.ok()?
	.text_range(),
	.range(),

[Conaclos]

Because you declared AnyClassProperties as an AST node union, into_syntax() or (syntax().clone()) should be available it. To get access to these methods, you need to import AstNode:

use biome_rowan::AstNode;

[Conaclos]

Suggested change

	) -> FxHashSet<AnyClassProperties> {
	) -> Vec<AnyClassProperties> {

[seeintea]

I'll fix them, after I've finished cover other mutating expressions JsSyntaxKind

This was an error

[chansuke]

Suggested change

[chansuke]

Suggested change

	```ts
	class ValidContainer {
	// Public members might be modified externally
	public publicMember: boolean;
	// Protected members might be modified by child classes
	protected protectedMember: number;
	// This is modified later on by the class
	private modifiedLater = 'unchanged';
	public mutate() {
	this.modifiedLater = 'mutated';
	}
	// This is modified later on by the class
	#modifiedLaterPrivateField = 'unchanged';
	public mutatePrivateField() {
	this.#modifiedLaterPrivateField = 'mutated';
	}
	```ts
	class ValidContainer {
	// Public members might be modified externally
	public publicMember: boolean;
	// Protected members might be modified by child classes
	protected protectedMember: number;
	// // Holds the initial state and updated by the `mutate` method
	private modifiedLater = 'unchanged';
	public mutate() {
	this.modifiedLater = 'mutated';
	}
	// Holds the initial state and updated by the `mutatePrivateField` method
	#modifiedLaterPrivateField = 'unchanged';
	public mutatePrivateField() {
	this.#modifiedLaterPrivateField = 'mutated';
	}

imho: the comment This is modified later on by the class could be improved with a more precise and clear description of the field's purpose and behavior. how do you think?