sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.18k stars 362 forks source link

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

Open astoilkov opened 5 years ago

astoilkov commented 5 years ago

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 commented 5 years ago

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: https://github.com/typescript-eslint/typescript-eslint/issues/542#issuecomment-494465051

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

sindresorhus commented 4 years ago

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

Torhal commented 4 years ago

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 commented 4 years ago

@Torhal Agreed. Moved it into a new issue: https://github.com/sindresorhus/eslint-plugin-unicorn/issues/438

fisker commented 4 years ago

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

felixfbecker commented 4 years ago

I have another case where React.Children.map().flat() gets flagged by prefer-flatmap even though React.Children has no flatMap() 😕

fisker commented 4 years ago

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

fregante commented 2 weeks ago

I created a label to group all the issues that would benefit from type information: https://github.com/sindresorhus/eslint-plugin-unicorn/labels/types