Fix use-implicit-booleaness-not-comparison crash (#5176)

* Fix use-implicit-booleaness-not-comparison crash

`use-implicit-booleaness-not-comparison` caused a crash due to
`target_node` not being an instance of `NodeNG.Name`
This fix will allow the checker to have a default handling when it
encounters `target_node` other than `Calls`,`Name`, or `Attribute`

* Added more comprehensive test for implicit_booealness_checker
* Make implicit_booealness_checker to have default `variable_name`
* Handle `Calls`,`Name`, or `Attribute`

* Fix typing in ImplicitBooleanessChecker.base_classes_of_node

* [implicit-booleaness] Add call nodes name in warnings

* Use `BaseContainer` to check for empty tuple/list and use `as_string` for `Attribute` and `Name` nodes for message

Using `BaseContainer` for checking for empty `tuple` and `list`.
In addition, `is_base_container` checks for `FrozenSet` and `Set` as
well.

* Update test cases with cr concerns
* Use `BaseContainer` when checking for empty list or tuple
* Update `is_literal_tuple/list` to use `is_base_container`
* Use `as_string` when giving out function or variable name for message.

* Fix broken baseContainer test

* Use safe_infer for inferencing `target_instance`

* Swap opreators message

* Address CR comments; no more try/catch for infer & Add more test cases

* Add more test cases and changed few cases to cover more cases.
* Remove `try/catch` from `safe_infer` since `safe_infer` will return
  `None` when it encounters exceptions.
* Comparison from infer to be more explicit; using `None` instead of
  relying on `bool`

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

5 files changed, 114 insertions, 58 deletions

diff --git a/pylint/checkers/refactoring/implicit_booleaness_checker.py b/pylint/checkers/refactoring/implicit_booleaness_checker.py
index b5d0b277f..fbcb29536 100644
--- a/pylint/checkers/refactoring/implicit_booleaness_checker.py
+++ b/pylint/checkers/refactoring/implicit_booleaness_checker.py
@@ -145,19 +145,15 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
         self, node: nodes.Compare
     ) -> None:
         """Check for left side and right side of the node for empty literals"""
-        is_left_empty_literal = (
-            utils.is_empty_list_literal(node.left)
-            or utils.is_empty_tuple_literal(node.left)
-            or utils.is_empty_dict_literal(node.left)
-        )
+        is_left_empty_literal = utils.is_base_container(
+            node.left
+        ) or utils.is_empty_dict_literal(node.left)
 
         # Check both left hand side and right hand side for literals
         for operator, comparator in node.ops:
-            is_right_empty_literal = (
-                utils.is_empty_list_literal(comparator)
-                or utils.is_empty_tuple_literal(comparator)
-                or utils.is_empty_dict_literal(comparator)
-            )
+            is_right_empty_literal = utils.is_base_container(
+                comparator
+            ) or utils.is_empty_dict_literal(comparator)
             # Using Exclusive OR (XOR) to compare between two side.
             # If two sides are both literal, it should be different error.
             if is_right_empty_literal ^ is_left_empty_literal:
@@ -165,14 +161,12 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
                 target_node = node.left if is_right_empty_literal else comparator
                 literal_node = comparator if is_right_empty_literal else node.left
                 # Infer node to check
-                try:
-                    target_instance = next(target_node.infer())
-                except astroid.InferenceError:
-                    # Probably undefined-variable, continue with check
+                target_instance = utils.safe_infer(target_node)
+                if target_instance is None:
                     continue
                 mother_classes = self.base_classes_of_node(target_instance)
                 is_base_comprehension_type = any(
-                    t in mother_classes for t in ("tuple", "list", "dict")
+                    t in mother_classes for t in ("tuple", "list", "dict", "set")
                 )
 
                 # Only time we bypass check is when target_node is not inherited by
@@ -183,20 +177,26 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
                     continue
 
                 # No need to check for operator when visiting compare node
-                if operator in ["==", "!=", ">=", ">", "<=", "<"]:
+                if operator in ("==", "!=", ">=", ">", "<=", "<"):
                     collection_literal = "{}"
                     if isinstance(literal_node, nodes.List):
                         collection_literal = "[]"
                     if isinstance(literal_node, nodes.Tuple):
                         collection_literal = "()"
-                    variable_name = target_node.name
+
+                    instance_name = "x"
+                    if isinstance(target_node, nodes.Call) and target_node.func:
+                        instance_name = f"{target_node.func.as_string()}(...)"
+                    elif isinstance(target_node, (nodes.Attribute, nodes.Name)):
+                        instance_name = target_node.as_string()
+
                     original_comparison = (
-                        f"{variable_name} {operator} {collection_literal}"
+                        f"{instance_name} {operator} {collection_literal}"
                     )
                     suggestion = (
-                        f"not {variable_name}"
+                        f"{instance_name}"
                         if operator == "!="
-                        else f"{variable_name}"
+                        else f"not {instance_name}"
                     )
                     self.add_message(
                         "use-implicit-booleaness-not-comparison",
@@ -208,7 +208,7 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
                     )
 
     @staticmethod
-    def base_classes_of_node(instance: nodes.ClassDef) -> List[nodes.Name]:
+    def base_classes_of_node(instance: nodes.ClassDef) -> List[str]:
         """Return all the classes names that a ClassDef inherit from including 'object'."""
         try:
             return [instance.name] + [x.name for x in instance.ancestors()]
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 0fee05af9..43157d0f9 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1626,12 +1626,8 @@ def is_function_body_ellipsis(node: nodes.FunctionDef) -> bool:
     )
 
 
-def is_empty_list_literal(node: Optional[nodes.NodeNG]) -> bool:
-    return isinstance(node, nodes.List) and not node.elts
-
-
-def is_empty_tuple_literal(node: Optional[nodes.NodeNG]) -> bool:
-    return isinstance(node, nodes.Tuple) and not node.elts
+def is_base_container(node: Optional[nodes.NodeNG]) -> bool:
+    return isinstance(node, nodes.BaseContainer) and not node.elts
 
 
 def is_empty_dict_literal(node: Optional[nodes.NodeNG]) -> bool:
diff --git a/tests/checkers/unittest_utils.py b/tests/checkers/unittest_utils.py
index cb10d4fce..2902d0de5 100644
--- a/tests/checkers/unittest_utils.py
+++ b/tests/checkers/unittest_utils.py
@@ -467,14 +467,14 @@ def test_if_typing_guard() -> None:
 
 def test_is_empty_literal() -> None:
     list_node = astroid.extract_node("a = []")
-    assert utils.is_empty_list_literal(list_node.value)
+    assert utils.is_base_container(list_node.value)
     not_empty_list_node = astroid.extract_node("a = [1,2,3]")
-    assert not utils.is_empty_list_literal(not_empty_list_node.value)
+    assert not utils.is_base_container(not_empty_list_node.value)
 
     tuple_node = astroid.extract_node("a = ()")
-    assert utils.is_empty_tuple_literal(tuple_node.value)
+    assert utils.is_base_container(tuple_node.value)
     not_empty_tuple_node = astroid.extract_node("a = (1,2)")
-    assert not utils.is_empty_tuple_literal(not_empty_tuple_node.value)
+    assert not utils.is_base_container(not_empty_tuple_node.value)
 
     dict_node = astroid.extract_node("a = {}")
     assert utils.is_empty_dict_literal(dict_node.value)
diff --git a/tests/functional/u/use/use_implicit_booleaness_not_comparison.py b/tests/functional/u/use/use_implicit_booleaness_not_comparison.py
index 18ca9c370..ce64fd1ed 100644
--- a/tests/functional/u/use/use_implicit_booleaness_not_comparison.py
+++ b/tests/functional/u/use/use_implicit_booleaness_not_comparison.py
@@ -1,5 +1,6 @@
 # pylint: disable=missing-docstring, missing-module-docstring, invalid-name
 # pylint: disable=too-few-public-methods, line-too-long, dangerous-default-value
+# pylint: disable=wrong-import-order
 # https://github.com/PyCQA/pylint/issues/4774
 
 def github_issue_4774():
@@ -102,14 +103,13 @@ something_else = NoBool()
 empty_literals = [[], {}, ()]
 is_empty = any(field == something_else for field in empty_literals)
 
-# this should work, but it doesn't since, input parameter only get the latest one, not all when inferred()
 h, i, j = 1, None, [1,2,3]
 
 def test(k):
     print(k == {})
 
 def test_with_default(k={}):
-    print(k == {}) # [use-implicit-booleaness-not-comparison]
+    print(k == {})
     print(k == 1)
 
 test(h)
@@ -120,6 +120,35 @@ test_with_default(h)
 test_with_default(i)
 test_with_default(j)
 
+
+class A:
+    lst = []
+
+    @staticmethod
+    def test(b=1):
+        print(b)
+        return []
+
+
+if A.lst == []:  # [use-implicit-booleaness-not-comparison]
+    pass
+
+
+if [] == A.lst:  # [use-implicit-booleaness-not-comparison]
+    pass
+
+
+if A.test("b") == []:  # [use-implicit-booleaness-not-comparison]
+    pass
+
+
+def test_function():
+    return []
+
+
+if test_function() == []:  # [use-implicit-booleaness-not-comparison]
+    pass
+
 # pylint: disable=import-outside-toplevel, wrong-import-position, import-error
 # Numpy has its own implementation of __bool__, but base class has list, that's why the comparison check is happening
 import numpy
@@ -139,4 +168,29 @@ if pandas_df == []:
 if pandas_df != ():
     pass
 if pandas_df <= []:
-    print("truth value of a dataframe is ambiguous")
+    print("don't emit warning if variable can't safely be inferred")
+
+from typing import Union
+from random import random
+
+var: Union[dict, bool, None] = {}
+if random() > 0.5:
+    var = True
+
+if var == {}:
+    pass
+
+data = {}
+
+if data == {}: # [use-implicit-booleaness-not-comparison]
+    print("This will be printed")
+if data != {}: # [use-implicit-booleaness-not-comparison]
+    print("This will also be printed")
+
+if data or not data:
+    print("This however won't be")
+
+# literal string check
+long_test = {}
+if long_test == {        }: # [use-implicit-booleaness-not-comparison]
+    pass
diff --git a/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt b/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt
index 1baca0b42..1aace6be8 100644
--- a/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt
+++ b/tests/functional/u/use/use_implicit_booleaness_not_comparison.txt
@@ -1,24 +1,30 @@
-use-implicit-booleaness-not-comparison:13:7:github_issue_4774:'bad_list == []' can be simplified to 'bad_list' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:21:3::'empty_tuple == ()' can be simplified to 'empty_tuple' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:24:3::'empty_list == []' can be simplified to 'empty_list' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:27:3::'empty_dict == {}' can be simplified to 'empty_dict' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:30:3::'empty_tuple == ()' can be simplified to 'empty_tuple' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:33:3::'empty_list == []' can be simplified to 'empty_list' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:36:3::'empty_dict == {}' can be simplified to 'empty_dict' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:41:11:bad_tuple_return:'t == ()' can be simplified to 't' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:45:11:bad_list_return:'b == []' can be simplified to 'b' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:49:11:bad_dict_return:'c == {}' can be simplified to 'c' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:51:7::'empty_tuple == ()' can be simplified to 'empty_tuple' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:52:7::'empty_list == []' can be simplified to 'empty_list' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:53:7::'empty_dict != {}' can be simplified to 'not empty_dict' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:54:7::'empty_tuple < ()' can be simplified to 'empty_tuple' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:55:7::'empty_list <= []' can be simplified to 'empty_list' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:56:7::'empty_tuple > ()' can be simplified to 'empty_tuple' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:57:7::'empty_list >= []' can be simplified to 'empty_list' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:82:3::'a == []' can be simplified to 'a' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:94:3::'e == []' can be simplified to 'e' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:94:15::'f == {}' can be simplified to 'f' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:112:10:test_with_default:'k == {}' can be simplified to 'k' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:127:3::'numpy_array == []' can be simplified to 'numpy_array' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:129:3::'numpy_array != []' can be simplified to 'not numpy_array' as an empty sequence is falsey:HIGH
-use-implicit-booleaness-not-comparison:131:3::'numpy_array >= ()' can be simplified to 'numpy_array' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:14:7:github_issue_4774:'bad_list == []' can be simplified to 'not bad_list' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:22:3::'empty_tuple == ()' can be simplified to 'not empty_tuple' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:25:3::'empty_list == []' can be simplified to 'not empty_list' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:28:3::'empty_dict == {}' can be simplified to 'not empty_dict' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:31:3::'empty_tuple == ()' can be simplified to 'not empty_tuple' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:34:3::'empty_list == []' can be simplified to 'not empty_list' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:37:3::'empty_dict == {}' can be simplified to 'not empty_dict' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:42:11:bad_tuple_return:'t == ()' can be simplified to 'not t' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:46:11:bad_list_return:'b == []' can be simplified to 'not b' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:50:11:bad_dict_return:'c == {}' can be simplified to 'not c' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:52:7::'empty_tuple == ()' can be simplified to 'not empty_tuple' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:53:7::'empty_list == []' can be simplified to 'not empty_list' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:54:7::'empty_dict != {}' can be simplified to 'empty_dict' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:55:7::'empty_tuple < ()' can be simplified to 'not empty_tuple' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:56:7::'empty_list <= []' can be simplified to 'not empty_list' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:57:7::'empty_tuple > ()' can be simplified to 'not empty_tuple' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:58:7::'empty_list >= []' can be simplified to 'not empty_list' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:83:3::'a == []' can be simplified to 'not a' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:95:3::'e == []' can be simplified to 'not e' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:95:15::'f == {}' can be simplified to 'not f' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:133:3::'A.lst == []' can be simplified to 'not A.lst' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:137:3::'A.lst == []' can be simplified to 'not A.lst' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:141:3::'A.test(...) == []' can be simplified to 'not A.test(...)' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:149:3::'test_function(...) == []' can be simplified to 'not test_function(...)' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:156:3::'numpy_array == []' can be simplified to 'not numpy_array' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:158:3::'numpy_array != []' can be simplified to 'numpy_array' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:160:3::'numpy_array >= ()' can be simplified to 'not numpy_array' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:185:3::'data == {}' can be simplified to 'not data' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:187:3::'data != {}' can be simplified to 'data' as an empty sequence is falsey:HIGH
+use-implicit-booleaness-not-comparison:195:3::'long_test == {}' can be simplified to 'not long_test' as an empty sequence is falsey:HIGH