Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(eslint-plugin): fix false positive from adjacent-overload-signatures#206
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(eslint-plugin): fix false positive from adjacent-overload-signatures#206
Uh oh!
There was an error while loading.Please reload this page.
Conversation
`adjacent-overload-signatures` rule won't distinguish between static and instance methods, despite they're different. So when they have the same name, they'll be treated as the same thing, which can cause a false positive result.Fixtypescript-eslint#169
codecovbot commentedFeb 4, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #206 +/- ##==========================================+ Coverage 96.57% 96.58% +<.01%========================================== Files 51 51 Lines 2455 2458 +3 Branches 370 370 ==========================================+ Hits 2371 2374 +3 Misses 42 42 Partials 42 42
|
thanks for the submission! Solution looks good! I think in this case, a better solution could be to switch the interim variable's type entirely to an object.
|
@bradzacher Thank you so much for your comment 😄 I've refactored the code according to it (2285b6c). Could you please check that out? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
one final nit, otherwise LGTM
@@ -84,27 +95,34 @@ module.exports = { | |||
if (members) { | |||
let name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
you can now move this into theforEach
scope, as it's not reused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OnlylastMethod
andseenMethods
are being reused. So I'll move the rest (name
,method
, andindex
) into theforEach
scope.
@bradzacher Done :) |
…ne config (typescript-eslint#206)* [FEAT][BREAKING][member-delimiter-style] Add handling for single line, enforce requireLast* switch to messageIds* [FEAT][BREAKING][2/2][member-delimiter-style] Separate single/multiline config* start updating tests to match new config* refactored tests to match new config* fix lint errors* remove unneeded call param
adjacent-overload-signatures
rule won't distinguish between static and instance methods, despite they're different. So when they have the same name, they'll be treated as the same thing, which can cause a false positive result.Fix#169