Rename refactoring that renames constructor parameter doesn't apply to its use in the constructor

[dbaeumer]

From @kberg on March 11, 2016 3:2

Starting with

interface Person {
  firstname: string;
  lastname: string;
}

class Student {
  fullname: string;
  constructor(public firstname, public middleinitial, public lastname) {
    this.fullname = firstname + " " + middleinitial + " " + lastname;
  }
}


class Startup {
  public static greeter(person: Person) {
    return "Hello, " + person.firstname + " " + person.lastname;
  }

  public static main(): number {
    var user: Person = new Student("Jane", "M.", "User");
    console.log(Startup.greeter(user));
    return 0;
  }
}

Startup.main();

In this case, Student can pass for a Person. But if I rename Person.firstname to fname, there's another compiler error.

interface Person {
  fname: string;
  lastname: string;
}

class Student {
  fullname: string;
  constructor(public firstname, public middleinitial, public lastname) {
    this.fullname = firstname + " " + middleinitial + " " + lastname;
  }
}


class Startup {
  public static greeter(person: Person) {
    return "Hello, " + person.fname + " " + person.lastname;
  }

  public static main(): number {
    var user: Person = new Student("Jane", "M.", "User");
    console.log(Startup.greeter(user));
    return 0;
  }
}

Startup.main();

Because "Type 'Student' is not assignable to type 'Person'. Property 'fname' is missing in type 'Student'."

If, instead we concretely indicate that Student is a person,

class Student implements Person {
  fullname: string;
  constructor(public firstname, public middleinitial, public lastname) {
    this.fullname = firstname + " " + middleinitial + " " + lastname;
  }
}

Such a rename would propagate:

interface Person {
  fname: string;
  lastname: string;
}

class Student implements Person {
  fullname: string;
  constructor(public fname, public middleinitial, public lastname) {
    this.fullname = firstname + " " + middleinitial + " " + lastname;
  }
}


class Startup {
  public static greeter(person: Person) {
    return "Hello, " + person.fname + " " + person.lastname;
  }

  public static main(): number {
    var user: Person = new Student("Jane", "M.", "User");
    console.log(Startup.greeter(user));
    return 0;
  }
}

Startup.main();

Well, mostly. The constructor broke.

Copied from original issue: microsoft/vscode#3987

[RyanCavanaugh]

It would be insane to propagate renames like this. We have no idea which types are actually semantically related to Person and which aren't and could end up renaming dozens of unrelated properties in other types.

[mprobst]

@RyanCavanaugh you could consider finding all types that are ever assigned into the type that's being renamed, right?

interface I { a: number }
let x = {a: 1}; // anonymous type X that structurally matches I
let ii: I = x; // X flows into I

Here, TS could infer that it should rename a in both I and the literal of the anonymous type X, given that X gets assigned into I. Not sure how much effort that is though?

[mhegazy]

not sure that this is correct either. i would expect in some cases that you want to see these as errors after your rename session.

A more appropriate solution is to do a structural comparison, and if it matches, give the user the option to change it. this is tracked by #6388

[RyanCavanaugh]

There's really no winning here.

Consider code like this:

interface HasName1 {
  name: string;
}
interface HasName2 {
  name: string;
}

function doSomething(x: { name: string }) {
  console.log(x.name);
}

function fn() { }
var a: HasName1 = { name: 'hello' };
var b: HasName2 = { name: 'world' };

doSomething(fn);
doSomething(a);
doSomething(b);

In any given rename position for name here, it's super unclear what we should do, Should renaming HasName1#name cause HasName2#name to get renamed as well? Should renaming the anonymous type in doSomething cause both interfaces to get renamed? Should renaming the field in the object literals cause Function#name in lib.d.ts to get changed?

[mhegazy]

As i noted earlier. I think there is a middle ground here. rename stays the way it is today, just because it simply makes sense in a typed language. we could add a new "structural rename", i would assume users would preview the changes here one by one, as they are mostly "suggestions", a little bit more informed than find and replace.