Skip to content

Issue 3265: Add WPS476 SneakyTypeVarWithDefaultViolation#3314

Merged
sobolevn merged 39 commits intowemake-services:masterfrom
Tapeline:issue-3265
Mar 15, 2025
Merged

Issue 3265: Add WPS476 SneakyTypeVarWithDefaultViolation#3314
sobolevn merged 39 commits intowemake-services:masterfrom
Tapeline:issue-3265

Conversation

@Tapeline
Copy link
Copy Markdown
Contributor

@Tapeline Tapeline commented Feb 20, 2025

I have made things!

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

Note

Making of this feature resulted into a refactoring of visitors.ast.classes following the complexity waterfall. Thorough checking is required

🙏 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.

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.

Thanks!

Comment thread tests/fixtures/noqa/noqa.py Outdated
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default.py Outdated
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default.py Outdated
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default.py Outdated
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default.py Outdated
Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
Comment thread wemake_python_styleguide/violations/best_practices.py Outdated
Comment thread wemake_python_styleguide/violations/best_practices.py
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7552d14) to head (ccb55c0).
Report is 95 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #3314   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          358       361    +3     
  Lines        11761     11814   +53     
  Branches       803       805    +2     
=========================================
+ Hits         11761     11814   +53     

☔ 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.

Comment thread tests/test_checker/test_noqa.py Outdated
Comment on lines +37 to +39
IGNORED_VIOLATIONS3_13 = (

)
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.

Suggested change
IGNORED_VIOLATIONS3_13 = (
)
IGNORED_VIOLATIONS3_13 = ()

Comment thread tests/test_checker/test_noqa.py Outdated
Comment on lines +302 to +306
SHOULD_BE_RAISED3_13 = types.MappingProxyType(
dict.fromkeys(SHOULD_BE_RAISED, 0) | {
'WPS476': 1
}
)
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.

Suggested change
SHOULD_BE_RAISED3_13 = types.MappingProxyType(
dict.fromkeys(SHOULD_BE_RAISED, 0) | {
'WPS476': 1
}
)
SHOULD_BE_RAISED3_13 = types.MappingProxyType(
{
'WPS476': 1
}
)

Let's simplify this case.

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.

Testcases check that every violation code is defined in such dictionary, so I was forced to add that line. Formatter ate my comment on this, btw

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.

'class_header',
[
(
"T = TypeVar('T')\n"
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.

Please, move multiline example to variables, it is easier to read.

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.

The whole list, or each individual example to its variable?

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.

we use examples like this:

wrong_type_var_defaults = """
T = TypeVar('T')

# ...
"""

@Tapeline Tapeline requested a review from sobolevn March 9, 2025 08:51
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.

Almost ready! Just tests to fix :)

Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
):
"""Test that WPS476 works correctly."""
src = (
f'{classes_with_various_bases}\n'
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.

Why do we need this line here? It should be a separate test.

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.

Removed this line, because it's included in test_type_var_ignored testcase

Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
@Tapeline Tapeline requested a review from sobolevn March 10, 2025 18:31
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
# Conflicts:
#	CHANGELOG.md
#	poetry.lock
#	tests/test_checker/test_noqa.py
#	wemake_python_styleguide/violations/best_practices.py
# Conflicts:
#	tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py
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.

We are almost there :)

Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
Comment thread wemake_python_styleguide/visitors/ast/classes/classdef.py
Comment thread wemake_python_styleguide/visitors/ast/classes/classdef.py Outdated
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.

Thank you! Great work 🎉

Comment thread tests/test_visitors/test_ast/test_classes/test_type_var_with_default313.py Outdated
Comment thread wemake_python_styleguide/visitors/ast/classes/classdef.py Outdated
@sobolevn sobolevn merged commit 90cb10a into wemake-services:master Mar 15, 2025
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.

Do not allow TypeVarTuple after TypeVar with a default

2 participants