Skip to content

ignore one arg when checking class method arg length#3139

Merged
sobolevn merged 2 commits intowemake-services:masterfrom
Sxderp:pr-test_complex
Dec 13, 2024
Merged

ignore one arg when checking class method arg length#3139
sobolevn merged 2 commits intowemake-services:masterfrom
Sxderp:pr-test_complex

Conversation

@Sxderp
Copy link
Copy Markdown
Contributor

@Sxderp Sxderp commented Dec 13, 2024

fixes #2394

I have made things!

I'll be happy to update the documentation to say the WPS211 uses an arg - 1 for validating class methods, but I'm not sure exactly where.

I also don't know what to put in the changelog.

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

Closes #2394

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

@Sxderp Sxderp marked this pull request as draft December 13, 2024 00:38
@Sxderp Sxderp marked this pull request as ready for review December 13, 2024 00:53
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (9eb9188) to head (b1699d7).
Report is 199 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #3139    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          117       118     +1     
  Lines         6083      5919   -164     
  Branches       901       856    -45     
==========================================
- Hits          6083      5919   -164     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

"""Checks the number of the arguments in a function."""
self.check_arguments_count_any(node)
parent = get_parent(node)
if isinstance(parent, ast.ClassDef) and not self._check_staticmethod(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe instead we should just ignore first arg named self / cls / mc? We even have _get_args_without_special_argument for that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting switching

wemake_python_styleguide.logic.tree.functions.get_all_arguments

to

wemake_python_styleguide.logic.arguments.function_args._get_args_without_special_argument

around line 51?

Of course I would make the function public and update callers, etc etc. I just want to confirm I understand this right.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this seems like the simpliest solution.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I took a look and that doesn't seem to work. The function doesn't grab keyword or variable length arguments. I did refactor a little bit to try and reuse some of that code though.

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Looks great now! Please, add a changelog entry.

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Nevermind, I will do that :)
Thanks!

@sobolevn sobolevn merged commit 2f4c3ae into wemake-services:master Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WPS211: Ignore self and cls

2 participants