Skip to content
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

Added new checks C413, C414, C415, and C416. #190

Merged
merged 7 commits into from
Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions HISTORY.rst
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
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
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
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't follow this so I've rewritten it with a for loop and an extra message piece.


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