From 14be74fc069e57cd02429a05bb2d91b3605ab280 Mon Sep 17 00:00:00 2001 From: areveny Date: Tue, 11 Oct 2022 20:36:37 -0500 Subject: [PATCH 1/6] Implement chained comparison changes --- .../refactoring/refactoring_checker.py | 215 +++++++++++++----- pylint/graph.py | 174 +++++++++++++- .../s/simplify_chained_comparison.py | 50 +++- .../s/simplify_chained_comparison.txt | 48 ++-- 4 files changed, 400 insertions(+), 87 deletions(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index ed1b7a9380..22d0ef314f 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -9,7 +9,7 @@ import itertools import sys import tokenize -from collections.abc import Iterator +from collections.abc import Iterator, Sequence from functools import reduce from re import Pattern from typing import TYPE_CHECKING, Any, NamedTuple, Union @@ -21,6 +21,7 @@ from pylint import checkers from pylint.checkers import utils from pylint.checkers.utils import node_frame_class +from pylint.graph import get_cycles, get_paths from pylint.interfaces import HIGH if TYPE_CHECKING: @@ -348,7 +349,7 @@ class RefactoringChecker(checkers.BaseTokenChecker): "more idiomatic, although sometimes a bit slower", ), "R1716": ( - "Simplify chained comparison between the operands", + "Simplify chained comparison between the operands: %s", "chained-comparison", "This message is emitted when pylint encounters boolean operation like " '"a < b and b < c", suggesting instead to refactor it to "a < b < c"', @@ -476,6 +477,17 @@ class RefactoringChecker(checkers.BaseTokenChecker): "value by index lookup. " "The value can be accessed directly instead.", ), + "R1737": ( + "Simplify cycle to ==", + "comparison-all-equal", + "Emitted when items in a boolean condition are all <= or >=" + "This is equivalent to asking if they all equal.", + ), + "R1738": ( + "This comparison always evalutes to False", + "impossible-comparison", + "Emitted when there a comparison that is always False.", + ), } options = ( ( @@ -1296,61 +1308,154 @@ def _check_consider_using_in(self, node: nodes.BoolOp) -> None: confidence=HIGH, ) - def _check_chained_comparison(self, node: nodes.BoolOp) -> None: - """Check if there is any chained comparison in the expression. - - Add a refactoring message if a boolOp contains comparison like a < b and b < c, - which can be chained as a < b < c. - - Care is taken to avoid simplifying a < b < c and b < d. - """ - if node.op != "and" or len(node.values) < 2: + def _check_comparisons(self, node: nodes.BoolOp) -> None: + graph_info = self._get_graph_from_comparison_nodes(node) + if not graph_info: + return + ( + graph_dict, + symbol_dict, + indegree_dict, + frequency_dict, + ) = graph_info + + # Convert graph_dict to all strings to access the get_cycles API + str_dict = { + str(key): {str(dest) for dest in graph_dict[key]} for key in graph_dict + } + cycles = get_cycles(str_dict) + if cycles: + self._handle_cycles(node, symbol_dict, cycles) return - def _find_lower_upper_bounds( - comparison_node: nodes.Compare, - uses: collections.defaultdict[str, dict[str, set[nodes.Compare]]], - ) -> None: - left_operand = comparison_node.left - for operator, right_operand in comparison_node.ops: - for operand in (left_operand, right_operand): - value = None - if isinstance(operand, nodes.Name): - value = operand.name - elif isinstance(operand, nodes.Const): - value = operand.value - - if value is None: - continue + paths = get_paths(graph_dict, indegree_dict, frequency_dict) - if operator in {"<", "<="}: - if operand is left_operand: - uses[value]["lower_bound"].add(comparison_node) - elif operand is right_operand: - uses[value]["upper_bound"].add(comparison_node) - elif operator in {">", ">="}: - if operand is left_operand: - uses[value]["upper_bound"].add(comparison_node) - elif operand is right_operand: - uses[value]["lower_bound"].add(comparison_node) - left_operand = right_operand - - uses: collections.defaultdict[ - str, dict[str, set[nodes.Compare]] - ] = collections.defaultdict( - lambda: {"lower_bound": set(), "upper_bound": set()} - ) - for comparison_node in node.values: - if isinstance(comparison_node, nodes.Compare): - _find_lower_upper_bounds(comparison_node, uses) - - for bounds in uses.values(): - num_shared = len(bounds["lower_bound"].intersection(bounds["upper_bound"])) - num_lower_bounds = len(bounds["lower_bound"]) - num_upper_bounds = len(bounds["upper_bound"]) - if num_shared < num_lower_bounds and num_shared < num_upper_bounds: - self.add_message("chained-comparison", node=node) - break + if len(paths) < len(node.values): + suggestions = [] + for path in paths: + cur_statement = str(path[0]) + for i in range(len(path) - 1): + cur_statement += ( + " " + symbol_dict[path[i], path[i + 1]] + " " + str(path[i + 1]) + ) + suggestions.append(cur_statement) + args = " and ".join(sorted(suggestions)) + self.add_message("chained-comparison", node=node, args=(args,)) + + def _get_graph_from_comparison_nodes( + self, node: nodes.BoolOp + ) -> None | tuple[ + dict[str | int | float, set[str | int | float]], + dict[tuple[str | int | float, str | int | float], str], + dict[str | int | float, int], + dict[tuple[str | int | float, str | int | float], int], + ]: + if node.op != "and" or len(node.values) < 2: + return None + + graph_dict: dict[ + str | int | float, set[str | int | float] + ] = collections.defaultdict(set) + symbol_dict: dict[ + tuple[str | int | float, str | int | float], str + ] = collections.defaultdict(lambda: ">") + frequency_dict: dict[ + tuple[str | int | float, str | int | float], int + ] = collections.defaultdict(int) + indegree_dict: dict[str | int | float, int] = collections.defaultdict(int) + const_values: list[int | float] = [] + + for statement in node.values: + if not isinstance(statement, nodes.Compare): + return None + ops = list(statement.ops) + left_statement = statement.left + while ops: + left = self._get_compare_operand_value(left_statement, const_values) + # Pop from ops or else we never advance along the statement + operator, right_statement = ops.pop(0) + # The operand is not a constant or variable or the operator is not a comparison + if operator not in {"<", ">", "==", "<=", ">="} or left is None: + return None + right = self._get_compare_operand_value(right_statement, const_values) + if right is None: + return None + + # Make the graph always point from larger to smaller + if operator == "<": + operator = ">" + left, right = right, left + elif operator == "<=": + operator = ">=" + left, right = right, left + + # Update maps + graph_dict[left].add(right) + if not graph_dict[right]: + graph_dict[right] = set() # Ensure the node exists in graph + symbol_dict[(left, right)] = operator + indegree_dict[left] += 0 # Make sure every node has an entry + indegree_dict[right] += 1 + frequency_dict[(left, right)] += 1 + + # advance onto the next comprison if it exists + left_statement = right_statement + + # Nothing was added and we have no recommendations + if ( + not graph_dict + or not symbol_dict + or all(val == "==" for val in symbol_dict.values()) + ): + return None + + # Link up constant nodes, i.e. create synthetic nodes between 1 and 5 such that 5 > 1 + sorted_consts = sorted(const_values) + while sorted_consts: + largest = sorted_consts.pop() + for smaller in set(sorted_consts): + if smaller < largest: + symbol_dict[(largest, smaller)] = ">" + indegree_dict[smaller] += 1 + frequency_dict[(largest, smaller)] += 1 + graph_dict[largest].add(smaller) + + # Remove paths from the larger number to the smaller number's adjacent nodes + # This prevents duplicated paths in the output + for adj in graph_dict[smaller]: + if isinstance(adj, str): + graph_dict[largest].discard(adj) + + return (graph_dict, symbol_dict, indegree_dict, frequency_dict) + + def _get_compare_operand_value( + self, node: nodes.Compare, const_values: list[int | float] + ) -> str | int | float | None: + value = None + if isinstance(node, nodes.Name) and isinstance(node.name, str): + value = node.name + elif isinstance(node, nodes.Const) and isinstance(node.value, (int, float)): + value = node.value + const_values.append(value) + return value + + def _handle_cycles( + self, + node: nodes.BoolOp, + symbol_dict: dict[tuple[str | int | float, str | int | float], str], + cycles: Sequence[list[str]], + ) -> None: + for cycle in cycles: + all_geq = all( + symbol_dict[(cur_item, cycle[i + 1])] == ">=" + for (i, cur_item) in enumerate(cycle) + if i < len(cycle) - 1 + ) + all_geq = all_geq and symbol_dict[cycle[-1], cycle[0]] == ">=" + if all_geq: + self.add_message("comparison-all-equal", node=node) + else: + self.add_message("impossible-comparison", node=node) @staticmethod def _apply_boolean_simplification_rules( @@ -1441,7 +1546,7 @@ def _check_simplifiable_condition(self, node: nodes.BoolOp) -> None: def visit_boolop(self, node: nodes.BoolOp) -> None: self._check_consider_merging_isinstance(node) self._check_consider_using_in(node) - self._check_chained_comparison(node) + self._check_comparisons(node) self._check_simplifiable_condition(node) @staticmethod diff --git a/pylint/graph.py b/pylint/graph.py index 21b5b2fee8..e07dee7f03 100644 --- a/pylint/graph.py +++ b/pylint/graph.py @@ -16,7 +16,7 @@ import sys import tempfile from collections.abc import Sequence -from typing import Any +from typing import Any, Union def target_info_from_filename(filename: str) -> tuple[str, str, str]: @@ -193,7 +193,7 @@ def _get_cycles( for node in path[::-1]: if node == vertice: break - cycle.insert(0, node) + cycle.insert(0, str(node)) # make a canonical representation start_from = min(cycle) index = cycle.index(start_from) @@ -212,3 +212,173 @@ def _get_cycles( except KeyError: pass path.pop() + +node_types = Union[str, int, float] + +def get_paths( + graph_dict: dict[node_types, set[node_types]], + indegree_dict: dict[node_types, int], + frequency_dict: dict[tuple[node_types, node_types], int], +) -> list[tuple[node_types]]: + """Gets the minimum number of paths that span all the edges in graph_dict.""" + to_visit = {node for node in indegree_dict if indegree_dict[node] == 0} + paths = set() + while to_visit: + symbols_in_longest_path: dict[node_types, int] = {} + nodes_in_longest_path: dict[node_types, int] = {} + + # Count the longest possible paths rooted at each node + for root in to_visit: + count_nodes( + root, + graph_dict, + symbols_in_longest_path, + nodes_in_longest_path, + frequency_dict, + ) + path: list[node_types] = [] + + # Get the node that can give us the longest path + longest_path_item = get_longest_path_item( + to_visit, symbols_in_longest_path, nodes_in_longest_path + ) + to_visit.remove(longest_path_item) + get_path( + path, + graph_dict, + longest_path_item, + to_visit, + frequency_dict, + symbols_in_longest_path, + nodes_in_longest_path, + ) + + # Decrement the times we can use each node we visited so they are not revisited + for i, item in enumerate(path): + for val in path[:i]: + frequency_dict[(val, item)] = max(frequency_dict[(val, item)] - 1, 0) + + path = strip_path(path) + if len(path) > 1: + paths.add(tuple(path)) + + return sorted(list(paths), key = str) # type: ignore[arg-type] + + +def get_longest_path_item( + items: set[node_types], + symbols_in_longest_path: dict[node_types, int], + nodes_in_longest_path: dict[node_types, int], +) -> node_types: + """Return the item that is at the root of the longest path, prioritizing the number + of symbols (a, b, c) and breaking ties with the total number of nodes in the path. + + and then the node value alphabetically. + """ + return sorted( + items, + reverse=True, + key=lambda x: (symbols_in_longest_path[x], nodes_in_longest_path[x], str(x)), + )[0] + + +def get_path( + path: list[node_types], + graph_dict: dict[node_types, set[node_types]], + node: node_types, + to_visit: set[node_types], + frequency_dict: dict[tuple[node_types, node_types], int], + symbols_in_longest_path: dict[node_types, int], + nodes_in_longest_path: dict[node_types, int], +) -> None: + """Appends to path the longest possible path in graph_dict starting at node.""" + path.append(node) + # Find viable neighbors that can be in the path + adj = {a for a in graph_dict[node] if frequency_dict[(node, a)] != 0} + if len(adj) >= 1: + # Select the neighbor that will yield the longest path + next_item = get_longest_path_item( + adj, symbols_in_longest_path, nodes_in_longest_path + ) + # Recursively get the path through that neighbor + get_path( + path, + graph_dict, + next_item, + to_visit, + frequency_dict, + symbols_in_longest_path, + nodes_in_longest_path, + ) + + # If there are other adjacent nodes, or this node has more paths going through it, we need to revisit it + if len(adj) >= 2 or frequency_dict[(node, next_item)] >= 2: + to_visit.add(node) + + +def count_nodes( + node: node_types, + graph_dict: dict[node_types, set[node_types]], + symbols_in_longest_path: dict[node_types, int], + nodes_in_longest_path: dict[node_types, int], + frequency_dict: dict[tuple[node_types, node_types], int], +) -> tuple[int, int]: + """Calculates the number of symbols and nodes in the longest path reachable from + node and stores them in symbols_in_longest_path and nodes_in_longest_path. + """ + # Already calculated + if node in symbols_in_longest_path and node in nodes_in_longest_path: + return (symbols_in_longest_path[node], nodes_in_longest_path[node]) + + adj = [a for a in graph_dict[node] if frequency_dict[(node, a)] != 0] + cur_node_symbol_count = ( + 1 if isinstance(node, str) else 0 + ) # Indicates if the current node is a symbol + if not adj: # Base case if there are no neighbors + max_symbols_path = cur_node_symbol_count + max_nodes_path = 1 + else: # Calculate the paths recursively based on the maximum lengths of neighbor nodes + adj_maximums = [ + count_nodes( + a, + graph_dict, + symbols_in_longest_path, + nodes_in_longest_path, + frequency_dict, + ) + for a in adj + ] + max_symbols_path = ( + max(adj_maxes[0] for adj_maxes in adj_maximums) + cur_node_symbol_count + ) + max_nodes_path = max(adj_maxes[1] for adj_maxes in adj_maximums) + 1 + + # Update the graph and return + symbols_in_longest_path[node] = max_symbols_path + nodes_in_longest_path[node] = max_nodes_path + return (max_symbols_path, max_nodes_path) + + +def strip_path(path: list[node_types]) -> list[node_types]: + """Removes redundant constant comparisons at the ends of a path, e.g. simplies {a, + 3, 0} to {a, 3}. + """ + low = 0 + high = len(path) - 1 + if path and isinstance(path[0], (int, float)): + while ( + low < len(path) - 1 + and isinstance(path[low], (int, float)) + and isinstance(path[low + 1], (int, float)) + ): + low += 1 + + if path and isinstance(path[-1], (int, float)): + while ( + high > 0 + and high > low + and isinstance(path[high], (int, float)) + and isinstance(path[high - 1], (int, float)) + ): + high -= 1 + return path[low : high + 1] diff --git a/tests/functional/s/simplify_chained_comparison.py b/tests/functional/s/simplify_chained_comparison.py index 4be9fbe3d5..2681c76c69 100644 --- a/tests/functional/s/simplify_chained_comparison.py +++ b/tests/functional/s/simplify_chained_comparison.py @@ -26,14 +26,18 @@ def test_simplify_chained_comparison_3(): pass elif a > 1 and a <= 10: # [chained-comparison] pass + elif a > 1 and a == 10: # [chained-comparison] + pass + elif a > 1 and a > 10: # [chained-comparison] + pass + elif a < 100 and a < 10: # [chained-comparison] + pass + elif a < b and a == 1 and a < c: # [chained-comparison] + pass elif a > 1 and a < 10 and b == 2: # [chained-comparison] pass elif a > 1 and c == b and a < 10: # [chained-comparison] pass - elif a > 100 and c == b and a < 10: # [chained-comparison] - # In this comparison we are not checking whether left bound is actually - # lower than right bound or not. - pass elif a < b and b < c: # [chained-comparison] pass elif a > b and b > c: # [chained-comparison] @@ -54,6 +58,8 @@ def test_simplify_chained_comparison_3(): pass elif a < b < c and a < b and b < c: # [chained-comparison] pass + elif a < b < c and a < c: # [chained-comparison] + pass def test_not_simplify_chained_comparison_1(): @@ -65,18 +71,10 @@ def test_not_simplify_chained_comparison_1(): pass elif a > 1 and b < 10: pass - elif a > 1 and a == 10: - pass elif a == 1 and b == 2: pass - elif a > 1 and a > 10: - pass - elif a < 100 and a < 10: - pass elif a < b and a < c: pass - elif a < b and a == 1 and a < c: - pass elif a < b and a < c and c == 786: pass elif a < b < c and b < d: @@ -87,3 +85,31 @@ def test_not_simplify_chained_comparison_1(): pass elif b < c < d and a < d: pass + +def test_impossible_comparison(): + a = 1 + b = 2 + c = 3 + d = 4 + if a > b and b > a: # [impossible-comparison] + pass + elif a > 100 and a < b and b < 15: # [impossible-comparison] + pass + elif a > b and b > c and c > a: # [impossible-comparison] + pass + elif a > b and b > c and c > d and d >= a: # [impossible-comparison] + pass + elif a > 100 and c == b and a < 10: # [impossible-comparison] + pass + +def test_all_equal(): + a = 1 + b = 2 + c = 3 + d = 4 + if a >= b and b >= a: # [comparison-all-equal] + pass + elif a >= b and b >= c and c >= a: # [comparison-all-equal] + pass + elif a <= b and b <= c and c <= d and d <= a: # [comparison-all-equal] + pass diff --git a/tests/functional/s/simplify_chained_comparison.txt b/tests/functional/s/simplify_chained_comparison.txt index 52fbb9d9ce..312721438b 100644 --- a/tests/functional/s/simplify_chained_comparison.txt +++ b/tests/functional/s/simplify_chained_comparison.txt @@ -1,18 +1,30 @@ -chained-comparison:10:11:10:26:test_simplify_chained_comparison_1:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:15:11:15:27:test_simplify_chained_comparison_2:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:23:7:23:23:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:25:9:25:25:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:27:9:27:26:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:29:9:29:36:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:31:9:31:36:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:33:9:33:38:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:37:9:37:24:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:39:9:39:24:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:41:9:41:35:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:43:9:43:37:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:45:9:45:37:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:47:9:47:37:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:49:9:49:37:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:51:9:51:28:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:53:9:53:28:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED -chained-comparison:55:9:55:38:test_simplify_chained_comparison_3:Simplify chained comparison between the operands:UNDEFINED +chained-comparison:10:11:10:26:test_simplify_chained_comparison_1:"Simplify chained comparison between the operands: c > b > a":UNDEFINED +chained-comparison:15:11:15:27:test_simplify_chained_comparison_2:"Simplify chained comparison between the operands: 10 > a > 1":UNDEFINED +chained-comparison:23:7:23:23:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: 10 > a > 1":UNDEFINED +chained-comparison:25:9:25:25:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: 10 > a > 1":UNDEFINED +chained-comparison:27:9:27:26:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: 10 >= a > 1":UNDEFINED +chained-comparison:29:9:29:26:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: a == 10":UNDEFINED +chained-comparison:31:9:31:25:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: a > 10":UNDEFINED +chained-comparison:33:9:33:27:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: 10 > a":UNDEFINED +chained-comparison:35:9:35:35:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: b > a and c > a == 1":UNDEFINED +chained-comparison:37:9:37:36:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: 10 > a > 1 and b == 2":UNDEFINED +chained-comparison:39:9:39:36:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: 10 > a > 1 and c == b":UNDEFINED +chained-comparison:41:9:41:24:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c > b > a":UNDEFINED +chained-comparison:43:9:43:24:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: a > b > c":UNDEFINED +chained-comparison:45:9:45:35:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c > b > a == 1":UNDEFINED +chained-comparison:47:9:47:37:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c == 786 and c > b > a":UNDEFINED +chained-comparison:49:9:49:37:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c == 786 > 0 > b > a":UNDEFINED +chained-comparison:51:9:51:37:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c == 786 > 0 > b > a":UNDEFINED +chained-comparison:53:9:53:37:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c == 786 > 0 > b > a":UNDEFINED +chained-comparison:55:9:55:28:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: d > c > b > a":UNDEFINED +chained-comparison:57:9:57:28:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: d > c > b > a":UNDEFINED +chained-comparison:59:9:59:38:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c > b > a":UNDEFINED +chained-comparison:61:9:61:28:test_simplify_chained_comparison_3:"Simplify chained comparison between the operands: c > b > a":UNDEFINED +impossible-comparison:94:7:94:22:test_impossible_comparison:This comparison always evalutes to False:UNDEFINED +impossible-comparison:96:9:96:37:test_impossible_comparison:This comparison always evalutes to False:UNDEFINED +impossible-comparison:98:9:98:34:test_impossible_comparison:This comparison always evalutes to False:UNDEFINED +impossible-comparison:100:9:100:45:test_impossible_comparison:This comparison always evalutes to False:UNDEFINED +impossible-comparison:102:9:102:38:test_impossible_comparison:This comparison always evalutes to False:UNDEFINED +comparison-all-equal:110:7:110:24:test_all_equal:Simplify cycle to ==:UNDEFINED +comparison-all-equal:112:9:112:37:test_all_equal:Simplify cycle to ==:UNDEFINED +comparison-all-equal:114:9:114:48:test_all_equal:Simplify cycle to ==:UNDEFINED From f41f4cb9828c4b00cff35693134da2eac2b28be2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 12 Oct 2022 02:02:08 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/graph.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pylint/graph.py b/pylint/graph.py index e07dee7f03..af1f6c6f53 100644 --- a/pylint/graph.py +++ b/pylint/graph.py @@ -213,8 +213,10 @@ def _get_cycles( pass path.pop() + node_types = Union[str, int, float] + def get_paths( graph_dict: dict[node_types, set[node_types]], indegree_dict: dict[node_types, int], @@ -262,7 +264,7 @@ def get_paths( if len(path) > 1: paths.add(tuple(path)) - return sorted(list(paths), key = str) # type: ignore[arg-type] + return sorted(list(paths), key=str) # type: ignore[arg-type] def get_longest_path_item( From 6f33ed63f29ed933a3a98c830954b1724670fb02 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Mon, 16 Jan 2023 09:48:20 +0100 Subject: [PATCH 3/6] Fix the checks that failed --- pylint/checkers/refactoring/refactoring_checker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 1094d351e0..7216b1022b 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -491,7 +491,7 @@ class RefactoringChecker(checkers.BaseTokenChecker): ), "R1737": ( "Simplify cycle to ==", - "comparison-all-equal", + "chained-comparison-all-equal", "Emitted when items in a boolean condition are all <= or >=" "This is equivalent to asking if they all equal.", ), @@ -1439,7 +1439,7 @@ def _get_graph_from_comparison_nodes( indegree_dict[right] += 1 frequency_dict[(left, right)] += 1 - # advance onto the next comprison if it exists + # advance onto the next comparison if it exists left_statement = right_statement # Nothing was added and we have no recommendations @@ -1470,9 +1470,9 @@ def _get_graph_from_comparison_nodes( return (graph_dict, symbol_dict, indegree_dict, frequency_dict) def _get_compare_operand_value( - self, node: nodes.Compare, const_values: list[int | float] + self, node: nodes.Compare, const_values: list[int | float |None] ) -> str | int | float | None: - value = None + value: str | int | float | None = None if isinstance(node, nodes.Name) and isinstance(node.name, str): value = node.name elif isinstance(node, nodes.Const) and isinstance(node.value, (int, float)): From 409e25e902c8f648162db62063479d5dc965a36b Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Mon, 16 Jan 2023 09:48:41 +0100 Subject: [PATCH 4/6] Update pylint/graph.py --- pylint/graph.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/graph.py b/pylint/graph.py index 26b0d2e933..8075455b2c 100644 --- a/pylint/graph.py +++ b/pylint/graph.py @@ -360,7 +360,7 @@ def count_nodes( def strip_path(path: list[node_types]) -> list[node_types]: - """Removes redundant constant comparisons at the ends of a path, e.g. simplies {a, + """Removes redundant constant comparisons at the ends of a path, e.g. simplifies {a, 3, 0} to {a, 3}. """ low = 0 From b1d38d938782c9c1a3e509c024331441af1ccb9f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 16 Jan 2023 08:49:18 +0000 Subject: [PATCH 5/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/refactoring/refactoring_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 7216b1022b..e2ab4981da 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -1470,7 +1470,7 @@ def _get_graph_from_comparison_nodes( return (graph_dict, symbol_dict, indegree_dict, frequency_dict) def _get_compare_operand_value( - self, node: nodes.Compare, const_values: list[int | float |None] + self, node: nodes.Compare, const_values: list[int | float | None] ) -> str | int | float | None: value: str | int | float | None = None if isinstance(node, nodes.Name) and isinstance(node.name, str): From 466a92a5c8e70ab2d0411ba6d830fb001874507b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 1 Sep 2023 15:30:45 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../checkers/refactoring/refactoring_checker.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index f009c302b2..0b53167399 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -1381,12 +1381,15 @@ def _check_comparisons(self, node: nodes.BoolOp) -> None: def _get_graph_from_comparison_nodes( self, node: nodes.BoolOp - ) -> None | tuple[ - dict[str | int | float, set[str | int | float]], - dict[tuple[str | int | float, str | int | float], str], - dict[str | int | float, int], - dict[tuple[str | int | float, str | int | float], int], - ]: + ) -> ( + None + | tuple[ + dict[str | int | float, set[str | int | float]], + dict[tuple[str | int | float, str | int | float], str], + dict[str | int | float, int], + dict[tuple[str | int | float, str | int | float], int], + ] + ): if node.op != "and" or len(node.values) < 2: return None