Skip to content

Commit

Permalink
Added new checks C413, C414, C415, and C416. (#190)
Browse files Browse the repository at this point in the history 
* Added C413 check for unnecessary calls around sorted().

Fixes #60.

* Added C414 check for unnecessary calls within list/set/sorted/tuple().

* Added C415 check for unnecessary subscript reversal of iterable within reversed/set/sorted().

* Added C416 check for unnecessary list/set comprehension - rewrite using list/set().

Fixes #163.
  • Loading branch information
@ngnpope @adamchainz
ngnpope authored and adamchainz committed Nov 15, 2019
1 parent a144cc9 commit f2aeae2
Show file tree
Hide file tree
Showing 4 changed files with 415 additions and 23 deletions.
11 changes: 11 additions & 0 deletions HISTORY.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ Pending Release
* Update Python support to 3.5-3.8.
* Fix false positives for C404 for list comprehensions not directly creating
tuples.
* Add ``C413`` rule that checks for unnecessary use of ``list()`` or
``reversed()`` around ``sorted()``.
* Add ``C414`` rule that checks for unnecessary use of the following:
* ``list()``, ``reversed()``, ``sorted()``, or ``tuple()`` within ``set``
or ``sorted()``
* ``list()`` or ``tuple()`` within ``list()`` or ``tuple()``
* ``set()`` within ``set``
* Add ``C415`` rule that checks for unnecessary reversal of an iterable via
subscript within ``reversed()``, ``set()``, or ``sorted()``.
* Add ``C416`` rule that checks for unnecessary list or set comprehensions that
can be rewritten using ``list()`` or ``set()``.

3.0.1 (2019-10-28)
------------------
Expand Down
62 changes: 62 additions & 0 deletions README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ C409 Unnecessary (list/tuple) passed to tuple() - (remove the outer call to tupl
C410 Unnecessary (list/tuple) passed to list() - (remove the outer call to list()/rewrite as a list literal).
C411 Unnecessary list call - remove the outer call to list().
C412 Unnecessary list comprehension - 'in' can take a generator.
C413 Unnecessary list call around sorted().
C413 Unnecessary reversed call around sorted() - (use sorted(..., reverse=(True/False))/toggle reverse argument to sorted()).
C414 Unnecessary (list/reversed/set/sorted/tuple) call within list/set/sorted/tuple().
C415 Unnecessary subscript reversal of iterable within reversed/set/sorted().
C416 Unnecessary (list/set) comprehension - rewrite using list/set().
==== ====

Examples
Expand Down Expand Up @@ -144,3 +149,60 @@ It's unnecessary to pass a list comprehension to 'in' that can take a
generator instead. For example:

* Rewrite ``y in [f(x) for x in foo]`` as ``y in (f(x) for x in foo)``

C413: Unnecessary list/reversed call around sorted().
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's unnecessary to use ``list()`` around ``sorted()`` as it already returns a
list. It is also suboptimal to use ``reversed()`` around ``sorted()`` as the
latter has a ``reverse`` argument. For example:

* Rewrite ``list(sorted([2, 3, 1]))`` as ``sorted([2, 3, 1])``
* Rewrite ``reversed(sorted([2, 3, 1]))`` as ``sorted([2, 3, 1], reverse=True)``
* Rewrite ``reversed(sorted([2, 3, 1], reverse=True))`` as ``sorted([2, 3, 1])``

C414: Unnecessary (list/reversed/set/sorted/tuple) call within list/set/sorted/tuple().
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's unnecessary to change the type of the iterable or change the order of
elements within certain other function calls that will themselves define the
order of the iterable or the type that is output. For example:

* Rewrite ``list(list(iterable))`` as ``list(iterable)``
* Rewrite ``list(tuple(iterable))`` as ``list(iterable)``
* Rewrite ``tuple(list(iterable))`` as ``tuple(iterable)``
* Rewrite ``tuple(tuple(iterable))`` as ``tuple(iterable)``
* Rewrite ``set(set(iterable))`` as ``set(iterable)``
* Rewrite ``set(list(iterable))`` as ``set(iterable)``
* Rewrite ``set(tuple(iterable))`` as ``set(iterable)``
* Rewrite ``set(sorted(iterable))`` as ``set(iterable)``
* Rewrite ``set(reversed(iterable))`` as ``set(iterable)``
* Rewrite ``sorted(list(iterable))`` as ``sorted(iterable)``
* Rewrite ``sorted(tuple(iterable))`` as ``sorted(iterable)``
* Rewrite ``sorted(sorted(iterable))`` as ``sorted(iterable)``
* Rewrite ``sorted(reversed(iterable))`` as ``sorted(iterable)``

C415: Unnecessary subscript reversal of iterable within reversed/set/sorted().
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's unnecessary to reverse the order of an iterable using a ``[::-1]`` before
passing it into ``set()`` which will randomize the order, ``sorted()`` which
will return a new sorted list, or ``reversed()`` which will effectively return
the original iterable. For example:

* Rewrite ``set(iterable[::-1])`` as ``set(iterable)``
* Rewrite ``sorted(iterable[::-1])`` as ``sorted(iterable, reverse=True)``
* Rewrite ``reversed(iterable[::-1])`` as ``iterable``

C416: Unnecessary (list/set) comprehension - rewrite using list/set().
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's unnecessary to use a list comprehension if the elements are unchanged. The
iterable should be wrapped in ``list()`` or ``set()`` instead. For example:

* Rewrite ``[x for x in iterable]`` as ``list(iterable)``
* Rewrite ``[(x, y) for x, y in iterable]`` as ``list(iterable)``
* Rewrite ``[(x, y) for (x, y) in iterable]`` as ``list(iterable)``
* Rewrite ``{x for x in iterable}`` as ``set(iterable)``
* Rewrite ``{(x, y) for x, y in iterable}`` as ``set(iterable)``
* Rewrite ``{(x, y) for (x, y) in iterable}`` as ``set(iterable)``
122 changes: 122 additions & 0 deletions src/flake8_comprehensions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def __init__(self, tree, *args, **kwargs):
"C410": "C410 Unnecessary {type} passed to list() - ",
"C411": "C411 Unnecessary list call - remove the outer call to list().",
"C412": "C412 Unnecessary list comprehension - 'in' can take a generator.",
"C413": "C413 Unnecessary {outer} call around {inner}(){remediation}.",
"C414": "C414 Unnecessary {inner} call within {outer}().",
"C415": "C415 Unnecessary subscript reversal of iterable within {func}().",
"C416": "C416 Unnecessary {type} comprehension - rewrite using {type}().",
}

def run(self):
Expand Down Expand Up @@ -178,6 +182,92 @@ def run(self):
self.messages["C408"].format(type=node.func.id),
type(self),
)

elif (
node.func.id in {"list", "reversed"}
and num_positional_args > 0
and isinstance(node.args[0], ast.Call)
and isinstance(node.args[0].func, ast.Name)
and node.args[0].func.id == "sorted"
):
remediation = ""
if node.func.id == "reversed":
reverse_flag_value = False
for keyword in node.args[0].keywords:
if keyword.arg != "reverse":
continue
if isinstance(keyword.value, ast.NameConstant):
reverse_flag_value = keyword.value.value
elif isinstance(keyword.value, ast.Num):
reverse_flag_value = bool(keyword.value.n)
else:
# Complex value
reverse_flag_value = None

if reverse_flag_value is None:
remediation = " - toggle reverse argument to sorted()"
else:
remediation = " - use sorted(..., reverse={!r})".format(
not reverse_flag_value
)

msg = self.messages["C413"].format(
inner=node.args[0].func.id,
outer=node.func.id,
remediation=remediation,
)
yield (
node.lineno,
node.col_offset,
msg,
type(self),
)

elif (
num_positional_args > 0
and isinstance(node.args[0], ast.Call)
and isinstance(node.args[0].func, ast.Name)
and (
(
node.func.id in {"set", "sorted"}
and node.args[0].func.id
in {"list", "reversed", "sorted", "tuple"}
)
or (
node.func.id in {"list", "tuple"}
and node.args[0].func.id in {"list", "tuple"}
)
or (node.func.id == "set" and node.args[0].func.id == "set")
)
):
yield (
node.lineno,
node.col_offset,
self.messages["C414"].format(
inner=node.args[0].func.id, outer=node.func.id
),
type(self),
)

elif (
node.func.id in {"reversed", "set", "sorted"}
and num_positional_args > 0
and isinstance(node.args[0], ast.Subscript)
and isinstance(node.args[0].slice, ast.Slice)
and node.args[0].slice.lower is None
and node.args[0].slice.upper is None
and isinstance(node.args[0].slice.step, ast.UnaryOp)
and isinstance(node.args[0].slice.step.op, ast.USub)
and isinstance(node.args[0].slice.step.operand, ast.Num)
and node.args[0].slice.step.operand.n == 1
):
yield (
node.lineno,
node.col_offset,
self.messages["C415"].format(func=node.func.id),
type(self),
)

elif isinstance(node, ast.Compare):
if (
len(node.ops) == 1
Expand All @@ -192,6 +282,38 @@ def run(self):
type(self),
)

elif isinstance(node, (ast.ListComp, ast.SetComp)):
if (
len(node.generators) == 1
and not node.generators[0].ifs
and (
(
isinstance(node.elt, ast.Name)
and isinstance(node.generators[0].target, ast.Name)
and node.elt.id == node.generators[0].target.id
)
or (
isinstance(node.elt, ast.Tuple)
and isinstance(node.generators[0].target, ast.Tuple)
and all(
isinstance(a, ast.Name)
and isinstance(b, ast.Name)
and a.id == b.id
for a, b in zip(
node.elt.elts, node.generators[0].target.elts
)
)
)
)
):
lookup = {ast.ListComp: "list", ast.SetComp: "set"}
yield (
node.lineno,
node.col_offset,
self.messages["C416"].format(type=lookup[node.__class__]),
type(self),
)


def has_star_args(call_node):
return any(isinstance(a, ast.Starred) for a in call_node.args)
Expand Down

0 comments on commit f2aeae2

Please sign in to comment.