Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[no-this-alias] does not detect all cases of aliasing this to local variable #4690

Closed
3 tasks done
fushi opened this issue Mar 16, 2022 · 3 comments · Fixed by #4718
Closed
3 tasks done

[no-this-alias] does not detect all cases of aliasing this to local variable #4690

fushi opened this issue Mar 16, 2022 · 3 comments · Fixed by #4718
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@fushi
Copy link

fushi commented Mar 16, 2022

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
   "@typescript-eslint/no-this-alias": "error",
  }
}
let that;

that = this;

Expected Result

3:5 error Unexpected aliasing of 'this' to local variable @typescript-eslint/no-this-alias

Actual Result

No error or warning.

Additional Info

https://github.com/sindresorhus/eslint-plugin-unicorn catches this with unicorn/no-this-assignment

Versions

package version
@typescript-eslint/eslint-plugin 15.5.0
@typescript-eslint/parser 15.5.0
TypeScript 4.6.2
ESLint 8.9.0
node 16.13.2
@fushi fushi added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Mar 16, 2022
@bradzacher bradzacher added enhancement New feature or request accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Mar 16, 2022
@juank1809
Copy link
Contributor

juank1809 commented Mar 20, 2022

This is because the rule only handles the case when 'this' is being assigned in a variable declaration and ignoring when you are assigning it to a variable already declared

I'd be glad to submit a PR to this issue.

@JoshuaKGoldberg
Copy link
Member

Hmm, I'm a little worried that this would cause new rule reports in cases that are actually fine in the spirit of the rule. Vaguely:

class SomeClass {
  doThing(target, useSelf) {
    target.work(useSelf ? target : this);
  }
}

I wonder if there should be an option around this behavior?

I'll defer to @bradzacher - it seems fine to me, but just wanted to raise.

@bradzacher
Copy link
Member

Based on the current PR #4718 - this wouldn't match on that junk.
Specifically it'll just be handling thing = this - the AssignmentExpression case.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
4 participants