Skip to content

Commit

Permalink
Add pattern matching support for naming rules (#2501)
Browse files Browse the repository at this point in the history
* Add pattern matching support for naming rules

* Fix CI

* Fix CI

* Add 3.10 to CI

* Fix coverage

* Fix CI

* Add 3.10 testing
  • Loading branch information
sobolevn authored Sep 27, 2022
1 parent 4ca670e commit 7b34423
Show file tree
Hide file tree
Showing 17 changed files with 200 additions and 38 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ['3.7', '3.8', '3.9']
python-version: ['3.7', '3.8', '3.9', '3.10']

steps:
- uses: actions/checkout@v3
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Semantic versioning in our case means:
### Features

- **Breaking**: drops `python3.6` support
- Adds support for pattern matching naming rules, same as other variables
- Adds `__init_subclass__` in the beginning of accepted methods
order as per WPS338 #2411

Expand Down
10 changes: 8 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ per-file-ignores =
wemake_python_styleguide/logic/safe_eval.py: WPS232
# This module should contain magic numbers:
wemake_python_styleguide/options/defaults.py: WPS432
# Compat/nodes is just pure nuts:
wemake_python_styleguide/compat/nodes.py: WPS113, WPS433, WPS440
# Checker has a lot of imports:
wemake_python_styleguide/checker.py: WPS201
# Allows mypy type hinting, `Ellipsis`` usage, multiple methods:
Expand Down Expand Up @@ -119,13 +121,17 @@ plugins =
coverage_conditional_plugin

[coverage:coverage_conditional_plugin]
# Here we specify our pragma rules:
# Here we specify our pragma rules for conditional coverage:
rules =
# 3.8
"sys_version_info < (3, 8)": py-lt-38
"sys_version_info >= (3, 8)": py-gte-38

# 3.9
"sys_version_info < (3, 9)": py-lt-39
"sys_version_info >= (3, 9)": py-gte-39
# 3.10
"sys_version_info < (3, 10)": py-lt-310
"sys_version_info >= (3, 10)": py-gte-310


[mypy]
Expand Down
8 changes: 8 additions & 0 deletions tests/fixtures/noqa/noqa310.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
user_id = 'uuid-user-id'
match user:
case 'user_id' | 'uid' as _uid: # noqa: WPS122
raise ValueError(_uid)
case {'key': k}: # noqa: WPS111
raise ValueError(k)
case [value]: # noqa: WPS110
raise ValueError(value)
9 changes: 6 additions & 3 deletions tests/fixtures/noqa/noqa38.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ def decorated(): # noqa: WPS216

def wrong_comprehension1():
return [ # noqa: WPS307
node
for node in 'ab'
if node != 'a'
node
for node in 'ab'
if node != 'a'
if node != 'b'
]

Expand All @@ -53,6 +53,9 @@ def positional_only(first, /, second): # noqa: WPS451


for element in range(10):
if (other := element) > 5: # noqa: WPS332
my_print(1)

try: # noqa: WPS452
my_print(1)
except AnyError:
Expand Down
49 changes: 34 additions & 15 deletions tests/test_checker/test_noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
_PY_OLD = sys.version_info < (3, 8)
_PY38 = (3, 8) <= sys.version_info < (3, 9)
_PY39 = (3, 9) <= sys.version_info < (3, 10)
_PY310 = (3, 10) <= sys.version_info < (3, 11)

#: Used to find violations' codes in output.
ERROR_PATTERN = re.compile(r'(WPS\d{3})')
Expand All @@ -42,18 +43,30 @@

#: Number and count of violations that would be raised.
VERSION_SPECIFIC = types.MappingProxyType({
'WPS216': int(not _PY39),
'WPS224': int(not _PY39),

'WPS307': int(not _PY39),
'WPS332': 0, # TODO: pyflakes fails at `:=` at the moment

'WPS416': int(_PY_OLD), # only works for `< python3.8`
'WPS451': int(_PY38), # only works for `== python3.8`
'WPS452': int(_PY38), # only works for `== python3.8`
'WPS466': int(_PY39), # only works for `== python3.9`

'WPS602': 2 * int(not _PY39),
'noqa_pre38': {
'WPS216': 1,
'WPS224': 1,
'WPS307': 1,
'WPS416': 1,
'WPS602': 2,
},
'noqa38': {
'WPS216': 1,
'WPS224': 1,
'WPS307': 1,
'WPS332': 1,
'WPS451': 1,
'WPS452': 1,
'WPS602': 2,
},
'noqa39': {
'WPS466': 1,
},
'noqa310': {
'WPS110': 1,
'WPS111': 1,
'WPS122': 1,
},
})

#: Number and count of violations that would be raised.
Expand Down Expand Up @@ -349,22 +362,28 @@ def test_codes(all_violations):
('noqa.py', SHOULD_BE_RAISED, True),
pytest.param(
'noqa_pre38.py',
VERSION_SPECIFIC,
VERSION_SPECIFIC['noqa_pre38'],
0,
marks=pytest.mark.skipif(not _PY_OLD, reason='ast changes on 3.8'),
),
pytest.param(
'noqa38.py',
VERSION_SPECIFIC,
VERSION_SPECIFIC['noqa38'],
0,
marks=pytest.mark.skipif(not _PY38, reason='ast changes on 3.8'),
),
pytest.param(
'noqa39.py',
VERSION_SPECIFIC,
VERSION_SPECIFIC['noqa39'],
0,
marks=pytest.mark.skipif(not _PY39, reason='ast changes on 3.9'),
),
pytest.param(
'noqa310.py',
VERSION_SPECIFIC['noqa310'],
0,
marks=pytest.mark.skipif(not _PY310, reason='ast changes on 3.10'),
),
])
def test_noqa_fixture_disabled(
absolute_path,
Expand Down
43 changes: 42 additions & 1 deletion tests/test_visitors/test_ast/test_naming/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

from wemake_python_styleguide.compat.constants import PY38
from wemake_python_styleguide.compat.constants import PY38, PY310
from wemake_python_styleguide.constants import UNUSED_PLACEHOLDER

# Imports:

Expand Down Expand Up @@ -161,6 +162,27 @@ def container():
raise
"""

# Pattern matching:

match_variable = """
match some_value:
case {0}:
...
"""

match_inner = """
match some_value:
case [{0}]:
...
"""

# This is the only case where we don't allow unused variables.
match_as_explicit = """
match some_value:
case [] as {0}:
...
"""


# Fixtures:

Expand Down Expand Up @@ -220,6 +242,12 @@ def container():
lambda_posonly_argument,
assignment_expression,
}
if PY310:
_ALL_FIXTURES |= {
match_variable,
match_as_explicit,
match_inner,
}

_FOREIGN_NAMING_PATTERNS = frozenset((
foreign_attribute,
Expand Down Expand Up @@ -261,6 +289,10 @@ def container():
_FORBIDDEN_BOTH_RAW_AND_PROTECTED_UNUSED |= {
assignment_expression,
}
if PY310:
_FORBIDDEN_BOTH_RAW_AND_PROTECTED_UNUSED |= {
match_as_explicit,
}

_FORBIDDEN_RAW_UNUSED = _FORBIDDEN_BOTH_RAW_AND_PROTECTED_UNUSED | {
static_attribute,
Expand Down Expand Up @@ -331,3 +363,12 @@ def forbidden_protected_unused_template(request):
def allowed_protected_unused_template(request):
"""Returns template that allows defining protected unused variables."""
return request.param


@pytest.fixture()
def skip_match_case_syntax_error():
"""Returns a helper that skips tests when `_` is used with pattern match."""
def factory(template: str, var_name: str) -> None:
if var_name == UNUSED_PLACEHOLDER and template == match_as_explicit:
pytest.skip('"_" cannot be used as "case" target')
return factory
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ def test_builtin_shadowing_whitelist(
assert_errors,
parse_ast_tree,
non_attribute_template,
skip_match_case_syntax_error,
default_options,
mode,
wrong_name,
):
"""Test names that shadow allowed builtins."""
skip_match_case_syntax_error(non_attribute_template, wrong_name)
tree = parse_ast_tree(
mode(non_attribute_template.format(wrong_name)),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ def test_correct_unused_variable_name(
assert_errors,
parse_ast_tree,
naming_template,
skip_match_case_syntax_error,
default_options,
mode,
wrong_name,
):
"""Ensures that wrong names are not allowed."""
skip_match_case_syntax_error(naming_template, wrong_name)
tree = parse_ast_tree(mode(naming_template.format(wrong_name)))

visitor = WrongNameVisitor(default_options, tree=tree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,12 @@ def test_raw_unused_variable_definition(
indentation,
bad_name,
forbidden_raw_unused_template,
skip_match_case_syntax_error,
default_options,
mode,
):
"""Testing raw variable definition is forbidden in some cases."""
skip_match_case_syntax_error(forbidden_raw_unused_template, bad_name)
tree = parse_ast_tree(
mode(context.format(
indent(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest

from wemake_python_styleguide.compat.constants import PY310
from wemake_python_styleguide.violations.naming import (
UnusedVariableIsUsedViolation,
)
Expand Down Expand Up @@ -49,6 +50,11 @@ def function():

inheriting_variables = 'class ValidName({0}): ...'

pattern_match_usage = """
match {0}:
case []: ...
"""


@pytest.mark.parametrize('bad_name', [
'value', # blacklisted
Expand Down Expand Up @@ -84,9 +90,13 @@ def function():
awaiting_variable,
yielding_variable,
inheriting_variables,
pytest.param(
pattern_match_usage,
marks=pytest.mark.skipif(not PY310, reason='pm was added in 3.10'),
),
])
@pytest.mark.parametrize('visitor_class', [
# We tests it here,
# We test it here,
# since I am too lazy to refactor usage patterns to be a fixture.
WrongNameVisitor,
Expand Down
3 changes: 3 additions & 0 deletions wemake_python_styleguide/compat/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@

#: This indicates that we are running on python3.9+
PY39: Final = sys.version_info >= (3, 9)

#: This indicates that we are running on python3.10+
PY310: Final = sys.version_info >= (3, 10)
25 changes: 21 additions & 4 deletions wemake_python_styleguide/compat/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
from typing import Any, Optional

if sys.version_info >= (3, 8): # pragma: py-lt-38
from ast import Constant as Constant # noqa: WPS433, WPS113
from ast import NamedExpr as NamedExpr # noqa: WPS113, WPS433
from ast import Constant as Constant
from ast import NamedExpr as NamedExpr
else: # pragma: py-gte-38
class NamedExpr(ast.expr): # noqa: WPS440
class NamedExpr(ast.expr):
"""
Fallback for python that does not have ``ast.NamedExpr``.
Expand All @@ -23,7 +23,7 @@ class NamedExpr(ast.expr): # noqa: WPS440
value: ast.expr # noqa: WPS110
target: ast.expr

class Constant(ast.expr): # noqa: WPS440
class Constant(ast.expr):
"""
Fallback for python that does not have ``ast.Constant``.
Expand All @@ -43,3 +43,20 @@ class Constant(ast.expr): # noqa: WPS440

s: Any # noqa: WPS111
n: complex # noqa: WPS111

if sys.version_info >= (3, 10): # pragma: py-lt-310
from ast import Match as Match
from ast import MatchAs as MatchAs
from ast import match_case as match_case
else: # pragma: py-gte-310
class Match(ast.stmt):
"""Used for ``match`` keyword and its body."""

class match_case(ast.AST): # noqa: N801
"""Used as a top level wrapper of pattern matched cases."""

class MatchAs(ast.AST):
"""Used to declare variables in pattern matched code."""

name: Optional[str] # noqa: WPS110
pattern: Optional[ast.AST]
20 changes: 20 additions & 0 deletions wemake_python_styleguide/logic/tree/pattern_matching.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from typing import Iterable

from wemake_python_styleguide.compat.nodes import Match, MatchAs, match_case
from wemake_python_styleguide.logic.nodes import get_parent
from wemake_python_styleguide.logic.walk import get_subnodes_by_type


def get_explicit_as_names(
node: Match,
) -> Iterable[MatchAs]: # pragma: py-lt-310
"""
Returns variable names defined as ``case ... as var_name``.
Does not return variables defined as ``case var_name``.
Or in any other forms.
"""
for match_as in get_subnodes_by_type(node, MatchAs):
if isinstance(get_parent(match_as), match_case):
if match_as.pattern and match_as.name:
yield match_as
9 changes: 9 additions & 0 deletions wemake_python_styleguide/violations/naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@
- Generic types should be called clearly and properly,
not just ``TT`` or ``KT`` or ``VT``
Pattern matching
~~~~~~~~~~~~~~~~
- All rules from local variables apply
- Explicit ``as`` patterns must be used:
``case ... as _var_name`` is not allowed
- However, mapping or sequence patterns can contain unused variables:
``case {"a_key": _not_used_value}:``
.. currentmodule:: wemake_python_styleguide.violations.naming
Summary
Expand Down
Loading

0 comments on commit 7b34423

Please sign in to comment.