Skip to content

Commit

Permalink
[WIP] Develop violation for pointless starred expressions (#734)
Browse files Browse the repository at this point in the history
* Develop violation for pointless starred expressions

* Fix style of code

* Add final decorator for visitor

* Fix incorrect docstrings

* Refactor of logic for finding useless starred expressions

* Fix coding style

* Fix coding style

* Fix coding style

* Change logic of checking starred arguments

* Fix coding style

* Improve visitor logic

* Fix typehints

* Fix coding style

* Fix coding style

* Fix docstring in new violation

* Fix noqa test

* Add feature description to CHANGELOG
  • Loading branch information
egorvdot authored and sobolevn committed Aug 11, 2019
1 parent 346c18c commit 80b8fe4
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ In this release we had a little focus on:
- Forbids to use ``type()`` for compares
- Forbids to have consecutive expressions with too deep access level
- Forbids to have too many public instance attributes
- Forbids to use pointless star operations: `print(*[])`

### Bugfixes

Expand Down
1 change: 1 addition & 0 deletions tests/test_checker/test_noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@
'WPS514': 1,
'WPS515': 1,
'WPS516': 1,
'WPS517': 0,

'WPS600': 1,
'WPS601': 1,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# -*- coding: utf-8 -*-

import pytest

from wemake_python_styleguide.violations.refactoring import (
PointlessStarredViolation,
)
from wemake_python_styleguide.visitors.ast.statements import (
PointlessStarredVisitor,
)


@pytest.mark.parametrize('code', [
'print(*[])',
'print(*())',
'print(*{})', # noqa: P103
'print(**{})', # noqa: P103
'print(*[1, 2])',
'print(*(1, 2))',
'print(*{1, 2})',
'print(**{"end": " "})',
])
def test_pointless_starred_arg(
assert_errors,
parse_ast_tree,
default_options,
code,
):
"""Testing that pointless starred expression is detected."""
tree = parse_ast_tree(code)

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

assert_errors(visitor, [PointlessStarredViolation])


@pytest.mark.parametrize('code', [
'print(*[], **{})', # noqa: P103
'print(*[], **{"1": 1})', # noqa: P103
'print(*[1], **{})', # noqa: P103
'print(*[1], **{"end": " "})',
])
def test_pointless_starred_arg_and_keyword(
assert_errors,
parse_ast_tree,
default_options,
code,
):
"""Testing that pointless starred expression is detected."""
tree = parse_ast_tree(code)

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

assert_errors(
visitor,
[PointlessStarredViolation, PointlessStarredViolation],
)


@pytest.mark.parametrize('code', [
"""
_list = [1, 2]
_dict = {"end": " "}
print(*_list, **_dict)
""",
])
def test_useful_starred_arg_and_keyword(
assert_errors,
parse_ast_tree,
default_options,
code,
):
"""Testing that pointless starred expression is detected."""
tree = parse_ast_tree(code)

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

assert_errors(visitor, [])
1 change: 1 addition & 0 deletions wemake_python_styleguide/presets/types/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
# General:
statements.StatementsWithBodiesVisitor,
statements.WrongParametersIndentationVisitor,
statements.PointlessStarredVisitor,

keywords.WrongRaiseVisitor,
keywords.WrongKeywordVisitor,
Expand Down
34 changes: 34 additions & 0 deletions wemake_python_styleguide/violations/refactoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
ImplicitInConditionViolation
OpenWithoutContextManagerViolation
TypeCompareViolation
PointlessStarredViolation
Refactoring opportunities
-------------------------
Expand All @@ -53,6 +54,7 @@
.. autoclass:: ImplicitInConditionViolation
.. autoclass:: OpenWithoutContextManagerViolation
.. autoclass:: TypeCompareViolation
.. autoclass:: PointlessStarredViolation
"""

Expand Down Expand Up @@ -608,6 +610,7 @@ class ImplicitInConditionViolation(ASTViolation):
Solution:
Refactor compares to use ``in`` or ``not in`` clauses.
Example::
# Correct:
Expand Down Expand Up @@ -687,3 +690,34 @@ class TypeCompareViolation(ASTViolation):

code = 516
error_template = 'Found `type()` used to compare types'


@final
class PointlessStarredViolation(ASTViolation):
"""
Forbids to have useless starred expressions.
Reasoning:
Using starred expression with constants is useless.
This piece of code can be rewritten to be flat.
Eg.: ``print(*[1, 2, 3])`` is ``print(1, 2, 3)``.
Solution:
Refactor your code not to use starred expressions
with ``list``, ``dict``, ``tuple``, and ``set`` constants.
Use regular argument passing instead.
Example::
# Correct:
my_list = [1, 2, 3, *other_iterable]
# Wrong:
print(*[1, 2, 3], **{{}})
.. versionadded:: 0.12.0
"""

code = 517
error_template = 'Found pointless starred expression'
41 changes: 41 additions & 0 deletions wemake_python_styleguide/visitors/ast/statements.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
ParametersIndentationViolation,
UselessNodeViolation,
)
from wemake_python_styleguide.violations.refactoring import (
PointlessStarredViolation,
)
from wemake_python_styleguide.visitors.base import BaseNodeVisitor
from wemake_python_styleguide.visitors.decorators import alias

Expand Down Expand Up @@ -286,3 +289,41 @@ def _check_indentation(
elements[index - 1].lineno,
multi_line_mode,
)


@final
class PointlessStarredVisitor(BaseNodeVisitor):
"""Responsible for absence of useless starred expressions."""

_pointless_star_nodes: ClassVar[AnyNodes] = (
ast.Dict,
ast.List,
ast.Set,
ast.Tuple,
)

def visit_Call(self, node: ast.Call) -> None:
"""Checks useless call arguments."""
self._check_starred_args(node.args)
self._check_double_starred_dict(node.keywords)
self.generic_visit(node)

def _is_pointless_star(self, node: ast.AST) -> bool:
return isinstance(node, self._pointless_star_nodes)

def _check_starred_args(
self,
args: Sequence[ast.AST],
) -> None:
for node in args:
if isinstance(node, ast.Starred):
if self._is_pointless_star(node.value):
self.add_violation(PointlessStarredViolation(node))

def _check_double_starred_dict(
self,
keywords: Sequence[ast.keyword],
) -> None:
for keyword in keywords:
if keyword.arg is None and self._is_pointless_star(keyword.value):
self.add_violation(PointlessStarredViolation(keyword.value))

0 comments on commit 80b8fe4

Please sign in to comment.