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

Revert "Extended consider-using-tuple check to cover 'in' comparisons (#4768)" #4832

Merged
merged 2 commits into from Aug 12, 2021
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
4 changes: 0 additions & 4 deletions ChangeLog
Expand Up @@ -48,10 +48,6 @@ Release date: TBA

Closes #4365

* ``CodeStyleChecker``

* Extended ``consider-using-tuple`` check to cover ``in`` comparisons.

* Added ``forgotten-debug-statement``: Emitted when ``breakpoint``, ``pdb.set_trace`` or ``sys.breakpointhook`` calls are found

Closes #3692
Expand Down
8 changes: 0 additions & 8 deletions doc/whatsnew/2.10.rst
Expand Up @@ -29,14 +29,6 @@ New checkers
Closes #3692


Extensions
==========

* ``CodeStyleChecker``

* Extended ``consider-using-tuple`` check to cover ``in`` comparisons.


Other Changes
=============

Expand Down
4 changes: 2 additions & 2 deletions pylint/checkers/classes.py
Expand Up @@ -999,10 +999,10 @@ def _check_unused_private_attributes(self, node: astroid.ClassDef) -> None:
if attribute.attrname != assign_attr.attrname:
continue

if assign_attr.expr.name == "cls" and attribute.expr.name in (
if assign_attr.expr.name == "cls" and attribute.expr.name in [
"cls",
"self",
):
]:
# If assigned to cls.attrib, can be accessed by cls/self
break

Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/python3.py
Expand Up @@ -156,7 +156,7 @@ def _in_iterating_context(node):
elif (
isinstance(parent, astroid.Compare)
and len(parent.ops) == 1
and parent.ops[0][0] in ("in", "not in")
and parent.ops[0][0] in ["in", "not in"]
):
return True
# Also if it's an `yield from`, that's fair
Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/refactoring/refactoring_checker.py
Expand Up @@ -955,7 +955,7 @@ def _check_consider_using_generator(self, node):
# remove square brackets '[]'
inside_comp = node.args[0].as_string()[1:-1]
call_name = node.func.name
if call_name in ("any", "all"):
if call_name in ["any", "all"]:
self.add_message(
"use-a-generator",
node=node,
Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/stdlib.py
Expand Up @@ -568,7 +568,7 @@ def _check_redundant_assert(self, node, infer):
isinstance(infer, astroid.BoundMethod)
and node.args
and isinstance(node.args[0], astroid.Const)
and infer.name in ("assertTrue", "assertFalse")
and infer.name in ["assertTrue", "assertFalse"]
):
self.add_message(
"redundant-unittest-assert",
Expand Down
4 changes: 2 additions & 2 deletions pylint/checkers/typecheck.py
Expand Up @@ -518,7 +518,7 @@ def _emit_no_member(node, owner, owner_name, ignored_mixins=True, ignored_none=T
and isinstance(owner.parent, astroid.ClassDef)
and owner.parent.name == "EnumMeta"
and owner_name == "__members__"
and node.attrname in ("items", "values", "keys")
and node.attrname in ["items", "values", "keys"]
):
# Avoid false positive on Enum.__members__.{items(), values, keys}
# See https://github.com/PyCQA/pylint/issues/4123
Expand Down Expand Up @@ -1781,7 +1781,7 @@ def visit_compare(self, node):
return

op, right = node.ops[0]
if op in ("in", "not in"):
if op in ["in", "not in"]:
self._check_membership_test(right)

@check_messages(
Expand Down
25 changes: 10 additions & 15 deletions pylint/extensions/code_style.py
@@ -1,4 +1,4 @@
from typing import List, Set, Tuple, Type
from typing import List, Set, Tuple, Type, Union

import astroid
from astroid.node_classes import NodeNG
Expand Down Expand Up @@ -53,18 +53,11 @@ def visit_dict(self, node: astroid.Dict) -> None:

@check_messages("consider-using-tuple")
def visit_for(self, node: astroid.For) -> None:
self._check_inplace_defined_list_set(node.iter)
self._check_inplace_defined_list_set(node)

@check_messages("consider-using-tuple")
def visit_comprehension(self, node: astroid.Comprehension) -> None:
self._check_inplace_defined_list_set(node.iter)

@check_messages("consider-using-tuple")
def visit_compare(self, node: astroid.Compare) -> None:
for op, comparator in node.ops:
if op != "in":
continue
self._check_inplace_defined_list_set(comparator)
self._check_inplace_defined_list_set(node)

def _check_dict_consider_namedtuple_dataclass(self, node: astroid.Dict) -> None:
"""Check if dictionary values can be replaced by Namedtuple or Dataclass."""
Expand Down Expand Up @@ -135,15 +128,17 @@ def _check_dict_consider_namedtuple_dataclass(self, node: astroid.Dict) -> None:
self.add_message("consider-using-namedtuple-or-dataclass", node=node)
return

def _check_inplace_defined_list_set(self, node: NodeNG) -> None:
def _check_inplace_defined_list_set(
self, node: Union[astroid.For, astroid.Comprehension]
) -> None:
"""Check if inplace defined list / set can be replaced by a tuple."""
if isinstance(node, (astroid.List, astroid.Set)) and not any(
isinstance(item, astroid.Starred) for item in node.elts
if isinstance(node.iter, (astroid.List, astroid.Set)) and not any(
isinstance(item, astroid.Starred) for item in node.iter.elts
):
self.add_message(
"consider-using-tuple",
node=node,
args=(f" instead of {node.__class__.__qualname__.lower()}",),
node=node.iter,
args=(f" instead of {node.iter.__class__.__qualname__.lower()}",),
)


Expand Down
2 changes: 1 addition & 1 deletion pylint/lint/pylinter.py
Expand Up @@ -721,7 +721,7 @@ def any_fail_on_issues(self):
def disable_noerror_messages(self):
for msgcat, msgids in self.msgs_store._msgs_by_category.items():
# enable only messages with 'error' severity and above ('fatal')
if msgcat in ("E", "F"):
if msgcat in ["E", "F"]:
for msgid in msgids:
self.enable(msgid)
else:
Expand Down
2 changes: 1 addition & 1 deletion script/bump_changelog.py
Expand Up @@ -136,7 +136,7 @@ def do_checks(content, next_version, version, version_type):
wn_next_version = get_whats_new(next_version)
wn_this_version = get_whats_new(version)
# There is only one field where the release date is TBA
if version_type in (VersionType.MAJOR, VersionType.MINOR):
if version_type in [VersionType.MAJOR, VersionType.MINOR]:
assert (
content.count(RELEASE_DATE_TEXT) <= 1
), f"There should be only one release date 'TBA' ({version}) {err}"
Expand Down
Expand Up @@ -27,30 +27,3 @@

[x for x in [*var, 2]]
[x for x in {*var, 2}]


# -----
# Suggest tuple for `in` comparisons
x in var
x in (1, 2, 3)
x in [1, 2, 3] # [consider-using-tuple]

if x in var:
pass
if x in (1, 2, 3):
pass
if x in [1, 2, 3]: # [consider-using-tuple]
pass
if x in {1, 2, 3}: # [consider-using-tuple]
pass

42 if x in [1, 2, 3] else None # [consider-using-tuple]
assert x in [1, 2, 3] # [consider-using-tuple]
(x for x in var if x in [1, 2, 3]) # [consider-using-tuple]
while x in [1, 2, 3]: # [consider-using-tuple]
break

# Stacked operators, rightmost pair is evaluated first
# Doesn't make much sense in practice since `in` will only return `bool`
True == x in [1, 2, 3] # [consider-using-tuple] # noqa: E712
1 >= x in [1, 2, 3] # [consider-using-tuple] # noqa: E712
@@ -1,13 +1,4 @@
consider-using-tuple:9:9::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:14:12::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:15:12::Consider using an in-place tuple instead of set:HIGH
consider-using-tuple:19:12::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:36:5::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:42:8::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:44:8::Consider using an in-place tuple instead of set:HIGH
consider-using-tuple:47:11::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:48:12::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:49:24::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:50:11::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:55:13::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:56:10::Consider using an in-place tuple instead of list:HIGH
consider-using-tuple:9:9::Consider using an in-place tuple instead of list
consider-using-tuple:14:12::Consider using an in-place tuple instead of list
consider-using-tuple:15:12::Consider using an in-place tuple instead of set
consider-using-tuple:19:12::Consider using an in-place tuple instead of list