this-function parameter can be renamed

[sandersn]

function(this/**/: string, x: number) {
  return this.length;
}

Now try to rename or find all references at /**/.

Expected: Message "You cannot rename this element".

Actual: Rename just the parameter, nothing in the body. Find all references just finds one reference, the definition.

Probably this needs to be classified as ThisKeyword by the parser as long as that doesn't break anything else.

[sandersn]

Thanks to @weswigham for coming up with this one.

[DanielRosenwasser]

I dunno, I like the flexibility here - for instance, imagine I want to stop using this and want to start passing in an object explicitly? As long as this stays contained to the function and doesn't leak into other scopes that have their own this, I feel like this is fine.

[sandersn]

Unfortunately it would be a lot of work because this isn't treated like a normal identifier anywhere else. Notice that the current behaviour doesn't rename body thises. That's because those occurrences aren't linked to the parameter via a symbol, but ad-hoc code in checkThisExpression.

[weswigham]

So, for consistency, we should probably just change the parameter to be a this-keyword, then if we actually want renaming capabilities, add a branch in find all references/rename where inputting a value-position this yields all similarly-scoped value-position thises.

The other option being to leave it as a parameter and add special cases for parameters named 'this' in a bunch of places, which seems hackier to me, IMO.

[basarat]

imagine I want to stop using this and want to start passing in an object explicitly

I end up adding this: {} and then fix the errors for this access and then after fixing I delete the this: {} annotation. Just a workflow that's worked for me personally ;) 🌹

[ghost]

This is related to #9198, so do you mind if I take this?

[sandersn]

Sure, although #9198 is a lot bigger than this issue. The fix in #9171 just prevents rename for this identifiers, and the alternate approach of parsing a ThisKeyword as a parameter makes the possibility of renaming Just Working even less likely.

[RyanCavanaugh]

Is this fixed?