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

fix(eslint-plugin): [unbound-method] handling of logical expr #1440

Merged
merged 2 commits into from Jan 16, 2020

Conversation

bradzacher
Copy link
Member

Fixes #1425

  • Refactored tests. Having all of the tests is super hard to understand when one fails.
  • The rule previously allowed true && instance.unbound, which is obviously not right.

Looking into it, there's 3 logical expressions: ||, && and ??.

  • || and ?? both will return either the left or right expression.
  • && will always return the right. Or rather, it can only return the left if it's falsy.
    • This means that const x = instance.unbound && something is safe, but const x = something && instance.unbound is not safe.

@bradzacher bradzacher added the bug Something isn't working label Jan 13, 2020
@typescript-eslint
Copy link
Contributor

Thanks for the PR, @bradzacher!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@codecov
Copy link

codecov bot commented Jan 13, 2020

Codecov Report

Merging #1440 into master will decrease coverage by 0.01%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1440      +/-   ##
==========================================
- Coverage   94.45%   94.44%   -0.02%     
==========================================
  Files         142      142              
  Lines        6083     6085       +2     
  Branches     1728     1730       +2     
==========================================
+ Hits         5746     5747       +1     
  Misses        183      183              
- Partials      154      155       +1
Impacted Files Coverage Δ
packages/eslint-plugin/src/rules/unbound-method.ts 94.44% <80%> (-2.62%) ⬇️

@bradzacher bradzacher changed the title fix(eslint-plugin): [unbound-method] fix handling of logical expressions fix(eslint-plugin): [unbound-method] handling of logical expr Jan 16, 2020
@bradzacher bradzacher merged commit 9c5b857 into master Jan 16, 2020
@bradzacher bradzacher deleted the 1425-unbound-method branch January 16, 2020 16:59
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[unbound-method] False positives when not invoking or assigning method
1 participant