diff --git a/ChangeLog b/ChangeLog index feb18d1c51..29e172ba78 100644 --- a/ChangeLog +++ b/ChangeLog @@ -36,6 +36,10 @@ Release date: TBA Closes #3878 +* ``CodeStyleChecker`` + + * Extended ``consider-using-tuple`` check to cover ``in`` comparisons. + What's New in Pylint 2.9.6? =========================== diff --git a/doc/whatsnew/2.10.rst b/doc/whatsnew/2.10.rst index 131cf0e033..534b791303 100644 --- a/doc/whatsnew/2.10.rst +++ b/doc/whatsnew/2.10.rst @@ -17,6 +17,14 @@ New checkers Closes #3826 +Extensions +========== + +* ``CodeStyleChecker`` + + * Extended ``consider-using-tuple`` check to cover ``in`` comparisons. + + Other Changes ============= diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 8f983d50ab..65ebe3423c 100644 --- a/pylint/checkers/classes.py +++ b/pylint/checkers/classes.py @@ -981,7 +981,7 @@ def _check_unused_private_attributes(self, node: astroid.ClassDef) -> None: ( # If assigned to cls.attrib, can be accessed by cls/self assign_attr.expr.name == "cls" - and attribute.expr.name in ["cls", "self"] + and attribute.expr.name in ("cls", "self") ) # If assigned to self.attrib, can only be accessed by self # Or if __new__ was used, the returned object names are acceptable diff --git a/pylint/checkers/python3.py b/pylint/checkers/python3.py index ccce989017..3fd635a23d 100644 --- a/pylint/checkers/python3.py +++ b/pylint/checkers/python3.py @@ -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 diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 3512451aca..9a4309a460 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -943,7 +943,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, diff --git a/pylint/checkers/stdlib.py b/pylint/checkers/stdlib.py index 1fba67afd4..4e6813c6a0 100644 --- a/pylint/checkers/stdlib.py +++ b/pylint/checkers/stdlib.py @@ -558,7 +558,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", diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 360676b6e5..6ac6a5740f 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -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 @@ -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( diff --git a/pylint/extensions/code_style.py b/pylint/extensions/code_style.py index 1938f38893..9ab67a618b 100644 --- a/pylint/extensions/code_style.py +++ b/pylint/extensions/code_style.py @@ -1,4 +1,4 @@ -from typing import List, Set, Tuple, Type, Union +from typing import List, Set, Tuple, Type import astroid from astroid.node_classes import NodeNG @@ -53,11 +53,18 @@ 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) + self._check_inplace_defined_list_set(node.iter) @check_messages("consider-using-tuple") def visit_comprehension(self, node: astroid.Comprehension) -> None: - self._check_inplace_defined_list_set(node) + 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) def _check_dict_consider_namedtuple_dataclass(self, node: astroid.Dict) -> None: """Check if dictionary values can be replaced by Namedtuple or Dataclass.""" @@ -128,17 +135,15 @@ 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: Union[astroid.For, astroid.Comprehension] - ) -> None: + def _check_inplace_defined_list_set(self, node: NodeNG) -> None: """Check if inplace defined list / set can be replaced by a tuple.""" - if isinstance(node.iter, (astroid.List, astroid.Set)) and not any( - isinstance(item, astroid.Starred) for item in node.iter.elts + if isinstance(node, (astroid.List, astroid.Set)) and not any( + isinstance(item, astroid.Starred) for item in node.elts ): self.add_message( "consider-using-tuple", - node=node.iter, - args=(f" instead of {node.iter.__class__.__qualname__.lower()}"), + node=node, + args=(f" instead of {node.__class__.__qualname__.lower()}",), ) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 25ae29c02a..bbcefe9240 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -717,7 +717,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: diff --git a/script/bump_changelog.py b/script/bump_changelog.py index 41c8c3c8a9..71c9c88333 100644 --- a/script/bump_changelog.py +++ b/script/bump_changelog.py @@ -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}" diff --git a/tests/functional/ext/code_style/code_style_consider_using_tuple.py b/tests/functional/ext/code_style/code_style_consider_using_tuple.py index 679ef77d2d..c4436450b1 100644 --- a/tests/functional/ext/code_style/code_style_consider_using_tuple.py +++ b/tests/functional/ext/code_style/code_style_consider_using_tuple.py @@ -27,3 +27,30 @@ [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 diff --git a/tests/functional/ext/code_style/code_style_consider_using_tuple.txt b/tests/functional/ext/code_style/code_style_consider_using_tuple.txt index df796b683e..b42dd37fd7 100644 --- a/tests/functional/ext/code_style/code_style_consider_using_tuple.txt +++ b/tests/functional/ext/code_style/code_style_consider_using_tuple.txt @@ -1,4 +1,13 @@ -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 +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