Skip to content

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 >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge 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 >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge enhancement: new plugin rule New rule request for eslint-plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants