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

feat(eslint-plugin): add rule strict-boolean-expressions #579

Merged
merged 7 commits into from Jul 1, 2019
Merged

feat(eslint-plugin): add rule strict-boolean-expressions #579

merged 7 commits into from Jul 1, 2019

Conversation

jonathanrdelgado
Copy link
Contributor

I ported over the strict-boolean-expressions rule from TSLint. I made the conscious decision to not move the previous options, as I felt they directly contradicted the rule itself.

For quick reference: https://palantir.github.io/tslint/rules/strict-boolean-expressions

This was my first implementation of an ESLint rule, so I am very open to any and all feedback. Thank you for your time!

JamesHenry
JamesHenry previously approved these changes Jun 20, 2019
Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done, @jonathanrdelgado, LGTM!

@JamesHenry JamesHenry added the 1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready label Jun 20, 2019
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few suggestions. Code looks good otherwise. Thanks for contributing!

@bradzacher bradzacher added the enhancement: new plugin rule New rule request for eslint-plugin label Jun 28, 2019
@codecov
Copy link

codecov bot commented Jun 28, 2019

Codecov Report

Merging #579 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #579      +/-   ##
==========================================
+ Coverage   94.33%   94.36%   +0.02%     
==========================================
  Files         109      110       +1     
  Lines        4540     4560      +20     
  Branches     1254     1258       +4     
==========================================
+ Hits         4283     4303      +20     
  Misses        149      149              
  Partials      108      108
Impacted Files Coverage Δ
...int-plugin/src/rules/strict-boolean-expressions.ts 100% <100%> (ø)
packages/eslint-plugin/src/rules/index.ts 100% <100%> (ø) ⬆️

- Add support for generics
- Abstract Logical/Unary expressions
- Abstract reports for multiple calls
- Filter UnaryExpressions at the walker
@jonathanrdelgado
Copy link
Contributor Author

Thank you for the specific feedback and suggestions, it was very helpful!

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work. I'll merge it as soon as the CI finishes.

@bradzacher bradzacher merged commit 34e7d1e into typescript-eslint:master Jul 1, 2019
@phaux
Copy link
Contributor

phaux commented Jul 20, 2019

It's kinda useless without the ignore-rhs option.

Edit: At least until we get nullish coalescing operator

@bradzacher
Copy link
Member

Someone else has PR'd it #691

Also, please don't leave comments on closed PRs. It's unnecessary notification spam that serves zero purpose.
Open a new issue if you have a feature request or a bug report.

@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Jul 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready enhancement: new plugin rule New rule request for eslint-plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants