Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Conversation

lukyth
Copy link
Contributor

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

Jessidhia reacted with thumbs up emoji
`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
@codecov
Copy link

codecovbot commentedFeb 4, 2019
edited
Loading

Codecov Report

Merging#206 intomaster willincrease coverage by<.01%.
The diff coverage is100%.

@@            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
Impacted FilesCoverage Δ
...t-plugin/lib/rules/adjacent-overload-signatures.js100% <100%> (ø)⬆️

@bradzacher
Copy link
Member

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.
That way you're avoiding the string concatenation cost, your code is more explicit, and it leaves it open for us to support more modifiers in future, if the need arrises.

lastMember = {  name: 'str',  static: true,}
j-f1 and lukyth reacted with thumbs up emoji

@lukyth
Copy link
ContributorAuthor

@bradzacher Thank you so much for your comment 😄 I've refactored the code according to it (2285b6c). Could you please check that out?

bradzacher
bradzacher previously approved these changesFeb 5, 2019
Copy link
Member

@bradzacherbradzacher left a 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;
Copy link
Member

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.

Copy link
ContributorAuthor

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.

@lukyth
Copy link
ContributorAuthor

@bradzacher Done :)

@JamesHenryJamesHenry merged commit07e950e intotypescript-eslint:masterFeb 7, 2019
@lukythlukyth deleted the lukyth/fix-static-adjacent-overload branchFebruary 12, 2019 01:38
kaicataldo pushed a commit to kaicataldo/typescript-eslint that referenced this pull requestAug 27, 2019
…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
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsApr 21, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@JamesHenryJamesHenryJamesHenry approved these changes

@bradzacherbradzacherbradzacher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[adjacent-overload-signatures] False positive when static and instance methods share the same name
3 participants
@lukyth@bradzacher@JamesHenry

[8]ページ先頭

©2009-2025 Movatter.jp