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

2268 only disallow certain magic attributes #2281

Merged
merged 4 commits into from
Dec 2, 2021

Conversation

ariebovenberg
Copy link
Contributor

I have made things!

See #2268

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 #2268

@ariebovenberg ariebovenberg marked this pull request as ready for review December 2, 2021 15:43
'__subclasses__',
'__mro__',
'__version__',
_disallowed_magic_attributes: ClassVar[FrozenSet[str]] = frozenset((
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this to constants.py, it is rather long!

It would also be a good idea to link to it from the violation's docs.

Thanks a lot for your time and effort 👍

@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #2281 (5388faf) into master (0e34852) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 5388faf differs from pull request most recent head bda482a. Consider uploading reports for the commit bda482a to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2281   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          119       119           
  Lines         6316      6316           
  Branches      1409      1409           
=========================================
  Hits          6316      6316           
Impacted Files Coverage Δ
wemake_python_styleguide/constants.py 100.00% <ø> (ø)
wemake_python_styleguide/violations/oop.py 100.00% <ø> (ø)
...emake_python_styleguide/visitors/ast/attributes.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e34852...bda482a. Read the comment docs.

@ariebovenberg
Copy link
Contributor Author

@sobolevn note I added some missing magic methods (like __next__) to the already existing ALL_MAGIC_METHODS constant. This may affect other violations (in a good way -- it's more complete now). Do we want to mention this as a fix in the changelog as well?

@sobolevn
Copy link
Member

sobolevn commented Dec 2, 2021

Do we want to mention this as a fix in the changelog as well?

Yes, please! 👍

I've also added attrs magic methods, because they are special enough to cover 🙂

@ariebovenberg
Copy link
Contributor Author

Added the changelog entry. I set this PR as a ref since this is where we discussed the fix.

@sobolevn sobolevn merged commit e37b8ae into wemake-services:master Dec 2, 2021
@sobolevn
Copy link
Member

sobolevn commented Dec 2, 2021

Thanks again!

@ariebovenberg ariebovenberg deleted the 2268-blocklist branch December 8, 2021 10:38
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.

Additional allowed magic attributes; consider 'blocklist' over 'allowlist' approach (WPS609)
2 participants