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

Rules improvements with the help of @typescript-eslint/parser #347

Open
astoilkov opened this issue Aug 25, 2019 · 10 comments · Fixed by #608
Open

Rules improvements with the help of @typescript-eslint/parser #347

astoilkov opened this issue Aug 25, 2019 · 10 comments · Fixed by #608
Labels

Comments

@astoilkov
Copy link

astoilkov commented Aug 25, 2019

I don't have experience with writing ESLint rules but I am wondering if @typescript-eslint/parser can help improve some of the rules. For example, prefer-event-key can extend its reach by detecting that the object type is KeyboardEvent and make the check.

function onKeyDown(e: KeyboardEvent) {
  if (e.keyCode) {
    // ...
  }
}
@sindresorhus
Copy link
Owner

I have been thinking a lot about this too. We have encountered many limitations with rules here that would not have happened with better type information. I'm open to exploring making rules better by using type information when available. I can even see us adding TypeScript-only rules where it's not feasible to do it without type information.

See this relevant discussion: typescript-eslint/typescript-eslint#542 (comment)

If we decide to do this, I think it would be best to start with one of the simpler rules, just to experiment.

@MrHen @futpib Thoughts?

@MrHen
Copy link
Contributor

MrHen commented Sep 5, 2019

I'm for typing and TypeScript but I am against supporting gobs of different pseudo-JavaScript languages based on the whims of whatever maintainers happen to be around. Babel seems ubiquitous at this point and I have a bias toward TypeScript because I've used it in the past which is why I'm comfortable with those two. But, personally, I'm not remotely interested in Flow or CoffeeScript or whatever flavor of the month pops out next.

Perhaps we should explicitly create eslint-plugin-unicorn-xyz repos to store language specific rules? I'm more than willing to sign up for eslint-plugin-unicorn-typescript rules. Unless we are fine explicitly blessing TypeScript and that's it?

Not sure what long term plans @sindresorhus has for supporting TypeScript versus Flow, versus the next best thing. Are you all-in on TypeScript at this point?

@sindresorhus
Copy link
Owner

sindresorhus commented Sep 5, 2019

@MrHen This is only about TypeScript. I have no intention of supporting Flow.

Perhaps we should explicitly create eslint-plugin-unicorn-xyz repos to store language specific rules?

The problem with that is that the point here is to have one rule that works in both JS and TS, but works better in TS by taking advantage of the type information.

Unless we are fine explicitly blessing TypeScript and that's it?

Yes

@MrHen
Copy link
Contributor

MrHen commented Sep 5, 2019

Them I'm all for it. Viva la typing.

@sindresorhus
Copy link
Owner

sindresorhus commented Oct 8, 2019

I'm gonna keep a list of things that could be improved with type information:

@Torhal
Copy link

Torhal commented Nov 15, 2019

This would be especially helpful for the "prevent-abbreviations" rule. At the moment, I can't use it with TypeScript because the fix removes the typings for function/method parameters which is disastrous for parameter objects.

@sindresorhus
Copy link
Owner

@Torhal Agreed. Moved it into a new issue: #438

@fisker
Copy link
Collaborator

fisker commented Mar 17, 2020

#608 Didn't fix this, I missed some info.

@felixfbecker

This comment has been minimized.

@fisker
Copy link
Collaborator

fisker commented Apr 7, 2020

@felixfbecker Can you open a separate issue about this? Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants