Aha I've definitely made this mistake myself. +1!

I can take a stab at it.

Hi @JoshuaKGoldberg I've started implementing this rule and have a question about #7089 (comment) in the previous PR.

Does this comment mean that the expectation is to only check the assignment directly below the function body?
If so, it would allow the following cases.

class Foo {
  constructor(public foo: string, bar: boolean) {
      if (bar) {
          this.foo = foo;
       }
  }
}

@yeonjuan that specific case would be invalid as you're assigning the initial value to itself and it's not within a function scope.

The edge-case that comes to mind would be this code:

class Foo {
  resetFoo: () => void;

  constructor(public foo: string) {
    this.resetFoo = () => {
      this.foo = foo;
    };
  }
}

const foo = new Foo("a");
console.log(foo.foo); // -> "a"
foo.foo = "b";
console.log(foo.foo); // -> "b"
foo.resetFoo();
console.log(foo.foo); // -> "a"

But there's also this edge case:

class Foo {
  constructor(public foo: string, bar: string) {
    if (bar) {
      this.foo = bar;
    } else {
      this.foo = foo;
    }
  }
}

Should we error here and enforce that the code drops the else branch?

This further refinement of that case probably needs some thought too:

class Foo {
  constructor(public foo: number, bar?: number) {
    if (bar) {
      this.foo = foo - bar;
    }

    if (this.foo < 0) {
      this.foo = foo;
    }
  }
}

Should this code error? We could handle this by tracking assignments?

It's worth mentioning that it's also completely valid for us to declare all of this code as a known edge case that we don't want to handle for now. I.e. declare a failing test with a comment saying as much.
The likely scenario is that these usecases are purely academic and so no user would ever hit them or complain. So yeah purposely and clearly not handling them until a user complains is a valid decision to make here - on that I'm personally in favour of.

IMO, the more errors the better. If you're reassigning a parameter property... you should just make it a normal property and assign it with a normal parameter anyway, especially if you have something so weird that you're deliberately reassigning it to itself. So, I personally would break on the side of flagging rather than not flagging in the case of any ambiguities, such as the ones above.

Another proposal, though, that includes and extends the spirit of this rule, could be "no using the parameter at all; only permit access via this.foo".

class Foo {
  resetFoo: () => void;

  constructor(public foo: string) {
    console.log(foo)
                ^^^ 
    /* error, use `this.foo` to access `foo`, or explicitly create a variable with its initial value */
  }
}

That forces you to be explicit about capturing the initial value of property into an appropriately named variable if that's what you actually want. IMO, the below code is unambiguously better than using the foo parameter to capture an initial value anyway.

class Foo {
  resetFoo: () => void;

  constructor(public foo: string) {
    const initialFoo = this.foo;
    this.resetFoo = () => {
      this.foo = initialFoo;
    };
  }
}

I'd wager most uses of foo without this.foo in this context are written without taking into consideration that the two values could differ, either because

1. easy accidental oversight
2. Muscle memory coming from c++/java/C# where this.foo is equivalent to foo, and including this is often considered bad style (an opinion I disagree with). In most cases, you can rely on the TS compiler to rescue you, by giving an error and saying "did you mean this.foo", but not here, due to there being an identifier of the same name but potentially different value in scope, from a single declaration 🤮

Along the lines of point 2 above, this proposal has the added benefit of simultaneously preventing errors resulting from writing foo = bar when this.foo = bar was meant, a situation, that again, the compiler won't help you with, and should probably get flagged in its own lint rule if it doesn't make it into this one.

Unfortunately, there's one footgun I can think of with the above approach, which is that depending on your tsconfig target setting and usedDefineForClassFields setting, this.foo may not equal foo, even in the first line of the constructor.

class RunsCodeBeforeConstructor {
  constructor(public foo: number = 0) {
    console.log(`parameter and property are equal: ${this.foo === foo}`);
  }

  // @ts-ignore: Whether this is considered an error depends on useDefineForClassFields and target
  mayRunBeforeConstructorSemiChecked = this.foo += 10;

  // never an error
  mayRunBeforeConstructor1NeverChecked = (() => this.foo += 10)();
}

(playground link)

So we might need to use caution if implying to users that const initialFoo = this.foo is always guaranteed to work correctly. Or this could be a documentation candidate for a :::warning block, or a "When not to use this rule" section.

This impacts users who have target >= ECMA2022 and have explicitly set useDefineForClassFields: false, or users who have target < ECMA 2022 (unless they have opted in to useDefineForClassFields: true)

FWIW - I only know about this because I ran into a bug trying to upgrade a project to target: 2022. So it's a real thing that code can depend on the initialization order. BUT, even that bug (which had to do with required property initializer side effects) was far less esoteric IMO than reassigning a constructor property in an initializer.

Further reading for those interested:

See MDN docs prescribing ECMA class initialization order. The thing to note is that in ECMA-compliant classes the constructor will be evaluated before any class field initializers.

See useDefineForClassFields docs. The only useful information here is the default values.

See original useDefineForClassFields announcement for TS 3.7. This is rather outdated since with target >= ES2022, it does not use Object.defineProperty... It just emits an ECMA class. The functional difference today is that if you opt in to the old behavior, TS will put class field initializers in the emitted constructor before any code that you've written in the constructor, meaning those initializers will run first. And, if you're truly masochistic, you can make it that this.foo !== foo.

Oh boy, so circling back to the issue report, there's also this !@#$^&* code:
(ECMA >= 2022 && useDefineForClassFields: false)

class Foo {
  constructor(public foo: number) {
    // not a noop
    this.foo = foo;
  }

  gotcha = this.foo += 10;
}

emits

"use strict";
class Foo {
    constructor(foo) {
        this.foo = foo;
        this.gotcha = this.foo += 10;
        // not a noop
        this.foo = foo;
    }
}

Something to warn of in the docs/report message (mostly to say "for your own good, please use ECMA-compliant classes")?