Skip to content

Commit

Permalink
Force using tuples in frozenset constructor (#993)
Browse files Browse the repository at this point in the history
* `WPS527` force using tuples as methods arguments (#920)

* `WPS527` force using tuples as methods arguments (#920)

* `WPS527` force using tuples as methods arguments (#920)

* `WPS527` force using tuples as methods arguments (#920)

* Update constants.py

* Update test_not_tuple_argument.py

* Update test_not_tuple_argument.py
  • Loading branch information
Stanislav Lapshin authored and sobolevn committed Nov 12, 2019
1 parent 3c0147a commit bde9752
Show file tree
Hide file tree
Showing 11 changed files with 211 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -30,6 +30,7 @@ We used to have incremental versioning before `0.1.0`.
- Forbids to start lines with `.`
- Enforces better `&`, `|`, `>>`, `<<`, `^` operators usage
- Forbids incorrect exception order
- Enforces tuples usage with frozenset constructor

This comment has been minimized.

Copy link
@AIGeneratedUsername

AIGeneratedUsername Aug 15, 2022

It would be interesting to know a reason for this.


### Bugfixes

Expand Down
3 changes: 3 additions & 0 deletions tests/fixtures/noqa.py
Expand Up @@ -598,3 +598,6 @@ def implicit_yield_from():
anti_z444 = 1
except ValueError:
anti_z444 = 1


bad_frozenset = frozenset([1]) # noqa: WPS527
1 change: 1 addition & 0 deletions tests/test_checker/test_noqa.py
Expand Up @@ -208,6 +208,7 @@
'WPS524': 1,
'WPS525': 2,
'WPS526': 1,
'WPS527': 1,

'WPS600': 1,
'WPS601': 1,
Expand Down
@@ -0,0 +1,90 @@
# -*- coding: utf-8 -*-

import pytest

from wemake_python_styleguide.constants import TUPLE_ARGUMENTS_METHODS
from wemake_python_styleguide.violations.refactoring import (
NotATupleArgumentViolation,
)
from wemake_python_styleguide.visitors.ast.statements import (
WrongMethodArgumentsVisitor,
)


@pytest.mark.parametrize('code', [
'a = {0}(())',
'a = {0}((1,))',
'a = {0}((1, 2, 3))',
'a = {0}((1,), b)',
'a = {0}((x for x in some))',
'a = {0}((x for x in some), b)',
])
@pytest.mark.parametrize('method', [
*TUPLE_ARGUMENTS_METHODS,
])
def test_passed(
assert_errors,
parse_ast_tree,
code,
method,
default_options,
):
"""Ensures that tuples arguments are passed."""
tree = parse_ast_tree(code.format(method))

visitor = WrongMethodArgumentsVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])


@pytest.mark.parametrize('code', [
'a = {0}([])',
'a = {0}({1}1{2})',
'a = {0}({1}1, 2, 3{2})',
'a = {0}({1}1{2}, b)',
'a = {0}({1}1{2}, b, c)',
'a = {0}({1}1{2}, b, {1}2{2})',
'a = {0}({1}x for x in some{2})',
])
@pytest.mark.parametrize('method', [
*TUPLE_ARGUMENTS_METHODS,
])
@pytest.mark.parametrize('braces', ['[]', '{}']) # noqa: P103
def test_no_passed(
assert_errors,
parse_ast_tree,
code,
method,
braces,
default_options,
):
"""Ensures that non-tuples arguments are violated."""
tree = parse_ast_tree(code.format(method, braces[0], braces[1]))

visitor = WrongMethodArgumentsVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [NotATupleArgumentViolation])


@pytest.mark.parametrize('code', [
'a = {0}_func([1])',
])
@pytest.mark.parametrize('method', [
*TUPLE_ARGUMENTS_METHODS,
])
def test_no_checkable(
assert_errors,
parse_ast_tree,
code,
method,
default_options,
):
"""Ensures that non checkable situations are skipped."""
tree = parse_ast_tree(code.format(method))

visitor = WrongMethodArgumentsVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])
5 changes: 5 additions & 0 deletions wemake_python_styleguide/constants.py
Expand Up @@ -341,6 +341,11 @@
'complex',
))

#: List of functions in which arguments must be tuples.
TUPLE_ARGUMENTS_METHODS = frozenset((
'frozenset',
))

# Internal variables
# They are not publicly documented since they are not used by the end user.

Expand Down
19 changes: 19 additions & 0 deletions wemake_python_styleguide/logic/arguments/call_args.py
@@ -0,0 +1,19 @@
# -*- coding: utf-8 -*-

import ast
from typing import Iterator, Sequence


def get_all_args(call: ast.Call) -> Sequence[ast.AST]:
"""Gets all arguments (args and kwargs) from ``ast.Call``."""
return [
*call.args,
*[kw.value for kw in call.keywords],
]


def get_starred_args(call: ast.Call) -> Iterator[ast.Starred]:
"""Gets ``ast.Starred`` arguments from ``ast.Call``."""
for argument in call.args:
if isinstance(argument, ast.Starred):
yield argument
28 changes: 18 additions & 10 deletions wemake_python_styleguide/logic/arguments/function_args.py
Expand Up @@ -2,17 +2,22 @@

import ast
from itertools import zip_longest
from typing import Dict, Iterator, List, Optional, Tuple
from typing import Dict, List, Optional, Tuple

from wemake_python_styleguide import types
from wemake_python_styleguide.logic.arguments import method_args
from wemake_python_styleguide import constants, types
from wemake_python_styleguide.logic.arguments.call_args import get_starred_args


def get_starred_args(call: ast.Call) -> Iterator[ast.Starred]:
"""Gets ``ast.Starred`` arguments from ``ast.Call``."""
for argument in call.args:
if isinstance(argument, ast.Starred):
yield argument
def get_args_without_special_argument(
node: types.AnyFunctionDefAndLambda,
) -> List[ast.arg]:
"""Gets ``node`` arguments excluding ``self``, ``cls``, ``mcs``."""
node_args = node.args.args
if not node_args or isinstance(node, ast.Lambda):
return node_args
if node_args[0].arg not in constants.SPECIAL_ARGUMENT_NAMES_WHITELIST:
return node_args
return node_args[1:]


def has_same_vararg(
Expand All @@ -32,7 +37,10 @@ def has_same_vararg(
return node.args.vararg == vararg_name # type: ignore


def has_same_kwarg(node: types.AnyFunctionDefAndLambda, call: ast.Call) -> bool:
def has_same_kwarg(
node: types.AnyFunctionDefAndLambda,
call: ast.Call,
) -> bool:
"""Tells whether ``call`` has the same kwargs as ``node``."""
kwarg_name: Optional[str] = None
null_arg_keywords = filter(lambda key: key.arg is None, call.keywords)
Expand All @@ -54,7 +62,7 @@ def has_same_args( # noqa: WPS231
call: ast.Call,
) -> bool:
"""Tells whether ``call`` has the same positional args as ``node``."""
node_args = method_args.get_args_without_special_argument(node)
node_args = get_args_without_special_argument(node)
paired_arguments = zip_longest(call.args, node_args)
for call_arg, func_arg in paired_arguments:
if isinstance(call_arg, ast.Starred):
Expand Down
18 changes: 0 additions & 18 deletions wemake_python_styleguide/logic/arguments/method_args.py

This file was deleted.

1 change: 1 addition & 0 deletions wemake_python_styleguide/presets/types/tree.py
Expand Up @@ -31,6 +31,7 @@
statements.PointlessStarredVisitor,
statements.WrongNamedKeywordVisitor,
statements.AssignmentPatternsVisitor,
statements.WrongMethodArgumentsVisitor,

keywords.WrongRaiseVisitor,
keywords.WrongKeywordVisitor,
Expand Down
34 changes: 34 additions & 0 deletions wemake_python_styleguide/violations/refactoring.py
Expand Up @@ -42,6 +42,7 @@
MisrefactoredAssignmentViolation
InCompareWithSingleItemContainerViolation
ImplicitYieldFromViolation
NotATupleArgumentViolation
Refactoring opportunities
-------------------------
Expand Down Expand Up @@ -73,6 +74,7 @@
.. autoclass:: MisrefactoredAssignmentViolation
.. autoclass:: InCompareWithSingleItemContainerViolation
.. autoclass:: ImplicitYieldFromViolation
.. autoclass:: NotATupleArgumentViolation
"""

Expand Down Expand Up @@ -1046,3 +1048,35 @@ class ImplicitYieldFromViolation(ASTViolation):

error_template = 'Found implicit `yield from` usage'
code = 526


@final
class NotATupleArgumentViolation(ASTViolation):
"""
Force using tuples as method arguments.
Reasoning:
For some methods, it is better to use tuples instead of another
iterable (list, sets,...) as arguments
Solution:
Use tuples as arguments
Example::
# Correct:
a = frozenset((2,))
# Wrong:
a = frozenset([2])
See
:py:data:`~wemake_python_styleguide.constants.TUPLE_ARGUMENTS_METHODS`
for full list of methods that we check for.
.. versionadded:: 0.13.0
"""

error_template = 'Found not a tuple used as an argument'
code = 527
40 changes: 39 additions & 1 deletion wemake_python_styleguide/visitors/ast/statements.py
Expand Up @@ -5,8 +5,10 @@

from typing_extensions import final

from wemake_python_styleguide import constants
from wemake_python_styleguide.compat.aliases import ForNodes, FunctionNodes
from wemake_python_styleguide.logic import functions, nodes, strings
from wemake_python_styleguide.logic.arguments import call_args
from wemake_python_styleguide.logic.collections import (
first,
normalize_dict_elements,
Expand All @@ -32,6 +34,7 @@
from wemake_python_styleguide.violations.refactoring import (
AlmostSwappedViolation,
MisrefactoredAssignmentViolation,
NotATupleArgumentViolation,
PointlessStarredViolation,
)
from wemake_python_styleguide.visitors.base import BaseNodeVisitor
Expand Down Expand Up @@ -280,7 +283,7 @@ def visit_collection(self, node: AnyCollection) -> None:

def visit_Call(self, node: ast.Call) -> None:
"""Checks call arguments indentation."""
all_args = [*node.args, *[kw.value for kw in node.keywords]]
all_args = call_args.get_all_args(node)
self._check_indentation(node, all_args)
self.generic_visit(node)

Expand Down Expand Up @@ -457,3 +460,38 @@ def _check_augmented_assign_pattern(

if name_nodes.is_same_variable(node.targets[0], node.value.left):
self.add_violation(AugmentedAssignPatternViolation(node))


@final
class WrongMethodArgumentsVisitor(BaseNodeVisitor):
"""Ensures that all arguments follow our rules."""

_no_tuples_collections: ClassVar[AnyNodes] = (
ast.List,
ast.ListComp,
ast.Set,
ast.SetComp,
)

def visit_Call(self, node: ast.Call) -> None:
"""Checks call arguments."""
self._check_tuple_arguments_types(node)
self.generic_visit(node)

def _check_tuple_arguments_types(
self,
node: ast.Call,
) -> None:
is_checkable = (
isinstance(node.func, ast.Name) and
node.func.id in constants.TUPLE_ARGUMENTS_METHODS
)

if not is_checkable:
return

all_args = call_args.get_all_args(node)
for arg in all_args:
if isinstance(arg, self._no_tuples_collections):
self.add_violation(NotATupleArgumentViolation(node))
break

0 comments on commit bde9752

Please sign in to comment.