-
-
Notifications
You must be signed in to change notification settings - Fork 394
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
Issue 3265: Add WPS476 SneakyTypeVarWithDefaultViolation #3314
base: master
Are you sure you want to change the base?
Changes from 15 commits
e4c5c98
632171d
8b2c65a
4f3bd91
c48d615
57530d2
48ce42e
f016624
5a03e99
990c9de
e12ae40
3061b70
3f6e072
41c7e05
b31d45a
06b55b4
cc68e6b
f5b0eea
ef34b32
082b819
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
""" | ||
This file contains all possible violations for python 3.13+. | ||
|
||
It is used for e2e tests. | ||
""" | ||
|
||
class NewStyleGenerics[ | ||
TypeVarDefault=int, | ||
*FollowingTuple=*tuple[int, ...] # noqa: WPS476 | ||
]: | ||
"""TypeVarTuple follows a defaulted TypeVar.""" |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -33,6 +33,11 @@ | |||||||||||||||||||||
'WPS402', # we obviously use a lot of `noqa` comments | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
#: List of ignored violations on python 3.13+. | ||||||||||||||||||||||
IGNORED_VIOLATIONS3_13 = ( | ||||||||||||||||||||||
|
||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
#: Number and count of violations that would be raised. | ||||||||||||||||||||||
SHOULD_BE_RAISED = types.MappingProxyType( | ||||||||||||||||||||||
{ | ||||||||||||||||||||||
|
@@ -234,6 +239,7 @@ | |||||||||||||||||||||
'WPS473': 0, | ||||||||||||||||||||||
'WPS474': 1, | ||||||||||||||||||||||
'WPS475': 1, | ||||||||||||||||||||||
'WPS476': 0, # enabled only in python 3.13+ | ||||||||||||||||||||||
'WPS500': 1, | ||||||||||||||||||||||
'WPS501': 1, | ||||||||||||||||||||||
'WPS502': 0, # disabled since 1.0.0 | ||||||||||||||||||||||
|
@@ -292,6 +298,13 @@ | |||||||||||||||||||||
}, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
#: Number and count of violations that would be raised in 3.13+ section. | ||||||||||||||||||||||
SHOULD_BE_RAISED3_13 = types.MappingProxyType( | ||||||||||||||||||||||
dict.fromkeys(SHOULD_BE_RAISED, 0) | { | ||||||||||||||||||||||
'WPS476': 1 | ||||||||||||||||||||||
} | ||||||||||||||||||||||
) | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Let's simplify this case. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def _assert_errors_count_in_output( | ||||||||||||||||||||||
output, | ||||||||||||||||||||||
|
@@ -324,6 +337,7 @@ def test_codes(all_violations): | |||||||||||||||||||||
('filename', 'violations'), | ||||||||||||||||||||||
[ | ||||||||||||||||||||||
('noqa.py', SHOULD_BE_RAISED), | ||||||||||||||||||||||
('noqa313.py', SHOULD_BE_RAISED3_13), | ||||||||||||||||||||||
], | ||||||||||||||||||||||
) | ||||||||||||||||||||||
def test_noqa_fixture_disabled( | ||||||||||||||||||||||
|
@@ -360,7 +374,14 @@ def test_noqa_fixture_disabled( | |||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def test_noqa_fixture(absolute_path): | ||||||||||||||||||||||
@pytest.mark.parametrize( | ||||||||||||||||||||||
'filename', | ||||||||||||||||||||||
[ | ||||||||||||||||||||||
'noqa.py', | ||||||||||||||||||||||
'noqa313.py', | ||||||||||||||||||||||
] | ||||||||||||||||||||||
) | ||||||||||||||||||||||
def test_noqa_fixture(absolute_path, filename): | ||||||||||||||||||||||
"""End-to-End test to check that `noqa` works.""" | ||||||||||||||||||||||
process = subprocess.Popen( | ||||||||||||||||||||||
[ | ||||||||||||||||||||||
|
@@ -370,7 +391,7 @@ def test_noqa_fixture(absolute_path): | |||||||||||||||||||||
'--isolated', | ||||||||||||||||||||||
'--select', | ||||||||||||||||||||||
'WPS, E999', | ||||||||||||||||||||||
absolute_path('fixtures', 'noqa', 'noqa.py'), | ||||||||||||||||||||||
absolute_path('fixtures', 'noqa', filename), | ||||||||||||||||||||||
], | ||||||||||||||||||||||
stdout=subprocess.PIPE, | ||||||||||||||||||||||
stderr=subprocess.PIPE, | ||||||||||||||||||||||
|
@@ -383,15 +404,24 @@ def test_noqa_fixture(absolute_path): | |||||||||||||||||||||
assert not stderr.count('WPS') | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def test_noqa_fixture_without_ignore(absolute_path): | ||||||||||||||||||||||
@pytest.mark.parametrize( | ||||||||||||||||||||||
('filename', 'ignored_violations'), | ||||||||||||||||||||||
[ | ||||||||||||||||||||||
('noqa.py', IGNORED_VIOLATIONS), | ||||||||||||||||||||||
('noqa313.py', IGNORED_VIOLATIONS3_13), | ||||||||||||||||||||||
] | ||||||||||||||||||||||
) | ||||||||||||||||||||||
def test_noqa_fixture_without_ignore( | ||||||||||||||||||||||
absolute_path, filename, ignored_violations | ||||||||||||||||||||||
): | ||||||||||||||||||||||
"""End-to-End test to check that `noqa` works without ignores.""" | ||||||||||||||||||||||
process = subprocess.Popen( | ||||||||||||||||||||||
[ | ||||||||||||||||||||||
'flake8', | ||||||||||||||||||||||
'--isolated', | ||||||||||||||||||||||
'--select', | ||||||||||||||||||||||
'WPS, E999', | ||||||||||||||||||||||
absolute_path('fixtures', 'noqa', 'noqa.py'), | ||||||||||||||||||||||
absolute_path('fixtures', 'noqa', filename), | ||||||||||||||||||||||
], | ||||||||||||||||||||||
stdout=subprocess.PIPE, | ||||||||||||||||||||||
stderr=subprocess.PIPE, | ||||||||||||||||||||||
|
@@ -400,5 +430,5 @@ def test_noqa_fixture_without_ignore(absolute_path): | |||||||||||||||||||||
) | ||||||||||||||||||||||
stdout, _ = process.communicate() | ||||||||||||||||||||||
|
||||||||||||||||||||||
for violation in IGNORED_VIOLATIONS: | ||||||||||||||||||||||
for violation in ignored_violations: | ||||||||||||||||||||||
assert stdout.count(violation) > 0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
from typing import Final | ||
|
||
import pytest | ||
|
||
from wemake_python_styleguide.violations.best_practices import ( | ||
SneakyTypeVarWithDefaultViolation, | ||
) | ||
from wemake_python_styleguide.visitors.ast.classes.classdef import ( | ||
ConsecutiveDefaultTypeVarsVisitor, | ||
) | ||
|
||
Tapeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
class_header_formats: Final = ['Class[{0}]', 'Class(Generic[{0}])'] | ||
various_code: Final = ( | ||
'pi = 3.14\n' | ||
'a = obj.method_call()\n' | ||
'w, h = get_size()\n' | ||
'obj.field = function_call()\n' | ||
'AlmostTypeVar = NotReallyATypeVar()\n' | ||
"NonDefault = TypeVar('NonDefault')\n" | ||
) | ||
classes_with_various_bases: Final = ( | ||
'class SimpleBase(object): ...\n' | ||
'class NotANameSubscript(Some.Class[object]): ...\n' | ||
'class NotAGenericBase(NotAGeneric[T]): ...\n' | ||
'class OldGenericDefinition(Generic[T]): ...\n' | ||
) | ||
|
||
|
||
def test_sneaky_type_var_with_default( | ||
assert_errors, | ||
parse_ast_tree, | ||
default_options, | ||
): | ||
"""Test that WPS476 works correctly.""" | ||
src = ( | ||
f'{classes_with_various_bases}\n' | ||
'\n' | ||
f'class Class[T=int, *Ts=*tuple[int, ...]]:\n' | ||
' ...' | ||
) | ||
|
||
tree = parse_ast_tree(src) | ||
|
||
visitor = ConsecutiveDefaultTypeVarsVisitor(default_options, tree=tree) | ||
visitor.run() | ||
|
||
assert_errors(visitor, [SneakyTypeVarWithDefaultViolation]) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'class_header', | ||
[ | ||
( | ||
"T = TypeVar('T')\n" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, move multiline example to variables, it is easier to read. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The whole list, or each individual example to its variable? |
||
"Ts = TypeVarTuple('Ts')\n" | ||
'\n' | ||
'class Class(Generic[T, *Ts]):' | ||
), | ||
( | ||
"T = TypeVar('T', default=int)\n" | ||
"Ts = TypeVarTuple('Ts')\n" | ||
'\n' | ||
'class Class(Generic[T, *Ts]):' | ||
), | ||
( | ||
'class Class[T, *Ts]:' | ||
) | ||
], | ||
) | ||
def test_type_var_ignored( | ||
assert_errors, | ||
parse_ast_tree, | ||
default_options, | ||
class_header, | ||
): | ||
"""Test that WPS476 ignores non-defaulted and old TypeVars.""" | ||
src = ( | ||
f'{various_code}\n' | ||
f'{classes_with_various_bases}\n' | ||
f'{class_header}\n' | ||
' ...' | ||
) | ||
|
||
tree = parse_ast_tree(src) | ||
|
||
visitor = ConsecutiveDefaultTypeVarsVisitor(default_options, tree=tree) | ||
visitor.run() | ||
|
||
assert_errors(visitor, []) |
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.