Skip to content

Commit 4e40fb7

Browse files
committed
Skip check for chained comparisons
1 parent 7b28cf4 commit 4e40fb7

File tree

1 file changed

+92
-85
lines changed

1 file changed

+92
-85
lines changed

pylint/checkers/refactoring/implicit_booleaness_checker.py

Lines changed: 92 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -190,108 +190,115 @@ def visit_compare(self, node: nodes.Compare) -> None:
190190
self._check_compare_to_str_or_zero(node)
191191

192192
def _check_compare_to_str_or_zero(self, node: nodes.Compare) -> None:
193+
# Skip check for chained comparisons
194+
if len(node.ops) != 1:
195+
return
196+
193197
# note: astroid.Compare has the left most operand in node.left
194198
# while the rest are a list of tuples in node.ops
195199
# the format of the tuple is ('compare operator sign', node)
196200
# here we squash everything into `ops` to make it easier for processing later
197201
ops: list[tuple[str, nodes.NodeNG]] = [("", node.left), *node.ops]
198202
iter_ops = iter(ops)
199203
all_ops = list(itertools.chain(*iter_ops))
200-
for ops_idx in range(len(all_ops) - 2):
201-
op_2 = all_ops[ops_idx + 1]
202-
if op_2 not in self._operators:
203-
continue
204-
op_1 = all_ops[ops_idx]
205-
op_3 = all_ops[ops_idx + 2]
206-
if self.linter.is_message_enabled(
207-
"use-implicit-booleaness-not-comparison-to-zero"
208-
):
209-
op = None
210-
# 0 ?? X
211-
if _is_constant_zero(op_1):
212-
op = op_3
213-
# X ?? 0
214-
elif _is_constant_zero(op_3):
215-
op = op_1
216-
if op is not None:
217-
original = f"{op_1.as_string()} {op_2} {op_3.as_string()}"
218-
suggestion = (
219-
op.as_string()
220-
if op_2 in {"!=", "is not"}
221-
else f"not {op.as_string()}"
222-
)
223-
self.add_message(
224-
"use-implicit-booleaness-not-comparison-to-zero",
225-
args=(original, suggestion),
226-
node=node,
227-
confidence=HIGH,
228-
)
229-
if self.linter.is_message_enabled(
230-
"use-implicit-booleaness-not-comparison-to-str"
231-
):
232-
node_name = None
233-
# x ?? ""
234-
if utils.is_empty_str_literal(op_1):
235-
node_name = op_3.as_string()
236-
# '' ?? X
237-
elif utils.is_empty_str_literal(op_3):
238-
node_name = op_1.as_string()
239-
if node_name is not None:
240-
suggestion = (
241-
f"not {node_name}" if op_2 in {"==", "is"} else node_name
242-
)
243-
self.add_message(
244-
"use-implicit-booleaness-not-comparison-to-string",
245-
args=(node.as_string(), suggestion),
246-
node=node,
247-
confidence=HIGH,
248-
)
204+
_, left_operand, operator, right_operand = all_ops
205+
206+
if operator not in self._operators:
207+
return
208+
if self.linter.is_message_enabled(
209+
"use-implicit-booleaness-not-comparison-to-zero"
210+
):
211+
operand = None
212+
# 0 ?? X
213+
if _is_constant_zero(left_operand):
214+
operand = right_operand
215+
# X ?? 0
216+
elif _is_constant_zero(right_operand):
217+
operand = left_operand
218+
if operand is not None:
219+
original = f"{left_operand.as_string()} {operator} {right_operand.as_string()}"
220+
suggestion = (
221+
operand.as_string()
222+
if operator in {"!=", "is not"}
223+
else f"not {operand.as_string()}"
224+
)
225+
self.add_message(
226+
"use-implicit-booleaness-not-comparison-to-zero",
227+
args=(original, suggestion),
228+
node=node,
229+
confidence=HIGH,
230+
)
231+
if self.linter.is_message_enabled(
232+
"use-implicit-booleaness-not-comparison-to-str"
233+
):
234+
node_name = None
235+
# x ?? ""
236+
if utils.is_empty_str_literal(left_operand):
237+
node_name = right_operand.as_string()
238+
# '' ?? X
239+
elif utils.is_empty_str_literal(right_operand):
240+
node_name = left_operand.as_string()
241+
if node_name is not None:
242+
suggestion = (
243+
f"not {node_name}" if operator in {"==", "is"} else node_name
244+
)
245+
self.add_message(
246+
"use-implicit-booleaness-not-comparison-to-string",
247+
args=(node.as_string(), suggestion),
248+
node=node,
249+
confidence=HIGH,
250+
)
249251

250252
def _check_use_implicit_booleaness_not_comparison(
251253
self, node: nodes.Compare
252254
) -> None:
253255
"""Check for left side and right side of the node for empty literals."""
256+
# Skip check for chained comparisons
257+
if len(node.ops) != 1:
258+
return
259+
260+
# Check both left-hand side and right-hand side for literals
261+
operator, comparator = node.ops[0]
254262
is_left_empty_literal = utils.is_base_container(
255263
node.left
256264
) or utils.is_empty_dict_literal(node.left)
265+
is_right_empty_literal = utils.is_base_container(
266+
comparator
267+
) or utils.is_empty_dict_literal(comparator)
257268

258-
# Check both left-hand side and right-hand side for literals
259-
for operator, comparator in node.ops:
260-
is_right_empty_literal = utils.is_base_container(
261-
comparator
262-
) or utils.is_empty_dict_literal(comparator)
263-
# Using Exclusive OR (XOR) to compare between two side.
264-
# If two sides are both literal, it should be different error.
265-
if is_right_empty_literal ^ is_left_empty_literal:
266-
# set target_node to opposite side of literal
267-
target_node = node.left if is_right_empty_literal else comparator
268-
literal_node = comparator if is_right_empty_literal else node.left
269-
# Infer node to check
270-
target_instance = utils.safe_infer(target_node)
271-
if target_instance is None:
272-
continue
273-
mother_classes = self.base_names_of_instance(target_instance)
274-
is_base_comprehension_type = any(
275-
t in mother_classes for t in ("tuple", "list", "dict", "set")
276-
)
269+
# If both sides are literals/non-literals, it should be different error.
270+
if not (is_left_empty_literal ^ is_right_empty_literal):
271+
return
272+
273+
# Set target_node to opposite side of literal
274+
target_node = node.left if is_right_empty_literal else comparator
275+
literal_node = comparator if is_right_empty_literal else node.left
276+
# Infer node to check
277+
target_instance = utils.safe_infer(target_node)
278+
if target_instance is None:
279+
return
280+
mother_classes = self.base_names_of_instance(target_instance)
281+
is_base_comprehension_type = any(
282+
t in mother_classes for t in ("tuple", "list", "dict", "set")
283+
)
277284

278-
# Only time we bypass check is when target_node is not inherited by
279-
# collection literals and have its own __bool__ implementation.
280-
if not is_base_comprehension_type and self.instance_has_bool(
281-
target_instance
282-
):
283-
continue
284-
285-
# No need to check for operator when visiting compare node
286-
if operator in {"==", "!=", ">=", ">", "<=", "<"}:
287-
self.add_message(
288-
"use-implicit-booleaness-not-comparison",
289-
args=self._implicit_booleaness_message_args(
290-
literal_node, operator, target_node
291-
),
292-
node=node,
293-
confidence=HIGH,
294-
)
285+
# Only time we bypass check is when target_node is not inherited by
286+
# collection literals and have its own __bool__ implementation.
287+
if not is_base_comprehension_type and self.instance_has_bool(
288+
target_instance
289+
):
290+
return
291+
292+
# No need to check for operator when visiting compare node
293+
if operator in {"==", "!=", ">=", ">", "<=", "<"}:
294+
self.add_message(
295+
"use-implicit-booleaness-not-comparison",
296+
args=self._implicit_booleaness_message_args(
297+
literal_node, operator, target_node
298+
),
299+
node=node,
300+
confidence=HIGH,
301+
)
295302

296303
def _get_node_description(self, node: nodes.NodeNG) -> str:
297304
return {

0 commit comments

Comments
 (0)