Sketch implementation of "optional-non-truthy" check for if x with x: None | str

docs/source/error_code_list2.rst

@@ -372,6 +372,41 @@ items`` check is actually valid. If that is the case, it is
 recommended to annotate ``items`` as ``Collection[int]`` instead of
 ``Iterable[int]``.
 
+.. _code-optional-non-truthy:
+
+Check that expression doesn't conflate None with other false expressions [optional-non-truthy]
+----------------------------------------------------------------------------------------------
+
+Warn when the type of an expression in a boolean context is optional
+(union with `None`) and also has other types that are boolean-like
+(implement ``__bool__`` or ``__len__``), such as `int`, `str` or `list`.
+
+In this case, the `if` block is likely to to be intending to just
+guard the `None` case, but falsy values like `0`, `""` or `[]` will
+behave the same as `None`. Instead use `... is None` or `... is not None`
+if one is wanting to only guard `None`, or `bool(...)` if the falsey
+values should indeed behave like `None`.
+
+.. code-block:: python
+
+    # mypy: enable-error-code="optional-non-truthy"
+
+    foo: None | int = 0
+    # Error: "foo" has type "Optional[int]" where both None and some other values (of type: "int") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+    if foo:
+        ... # executes when foo is an int != 0
+    else:
+        ... # executes when foo is None or 0
+
+For instance, in the case that `None` and `0` should behave differently:
+
+.. code-block:: python
+
+    if foo is not None:
+        ... # executes when foo is any int, including 0
+    else:
+        ... # executes when foo is None
+
 .. _code-ignore-without-code:
 
 Check that ``# type: ignore`` include an error code [ignore-without-code]

mypy/checker.py

@@ -5674,7 +5674,22 @@ def _is_truthy_type(self, t: ProperType) -> bool:
             )
         )
 
-    def check_for_truthy_type(self, t: Type, expr: Expression) -> None:
+    def _format_expr_type(self, t: Type, expr: Expression) -> str:
+        typ = format_type(t, self.options)
+        if isinstance(expr, MemberExpr):
+            return f'Member "{expr.name}" has type {typ}'
+        elif isinstance(expr, RefExpr) and expr.fullname:
+            return f'"{expr.fullname}" has type {typ}'
+        elif isinstance(expr, CallExpr):
+            if isinstance(expr.callee, MemberExpr):
+                return f'"{expr.callee.name}" returns {typ}'
+            elif isinstance(expr.callee, RefExpr) and expr.callee.fullname:
+                return f'"{expr.callee.fullname}" returns {typ}'
+            return f"Call returns {typ}"
+        else:
+            return f"Expression has type {typ}"
+
+    def _check_for_truthy_type(self, t: Type, expr: Expression) -> None:
         """
         Check if a type can have a truthy value.
 
@@ -5692,19 +5707,7 @@ def check_for_truthy_type(self, t: Type, expr: Expression) -> None:
             return
 
         def format_expr_type() -> str:
-            typ = format_type(t, self.options)
-            if isinstance(expr, MemberExpr):
-                return f'Member "{expr.name}" has type {typ}'
-            elif isinstance(expr, RefExpr) and expr.fullname:
-                return f'"{expr.fullname}" has type {typ}'
-            elif isinstance(expr, CallExpr):
-                if isinstance(expr.callee, MemberExpr):
-                    return f'"{expr.callee.name}" returns {typ}'
-                elif isinstance(expr.callee, RefExpr) and expr.callee.fullname:
-                    return f'"{expr.callee.fullname}" returns {typ}'
-                return f"Call returns {typ}"
-            else:
-                return f"Expression has type {typ}"
+            return self._format_expr_type(t, expr)
 
         def get_expr_name() -> str:
             if isinstance(expr, (NameExpr, MemberExpr)):
@@ -5728,6 +5731,56 @@ def get_expr_name() -> str:
         else:
             self.fail(message_registry.TYPE_ALWAYS_TRUE.format(format_expr_type()), expr)
 
+    def _check_for_optional_non_truthy_type(self, t: Type, expr: Expression) -> None:
+        """Check if a type involves both None and types that aren't always true, catching
+        suspicious cases of falsy values being lumped together with None.
+
+        Used in checks like::
+
+            if x: # <---
+
+            not x  # <---
+
+        """
+        t = get_proper_type(t)
+        if not isinstance(t, UnionType):
+            return  # not a Optional or Union at all
+
+        item_types = get_proper_types(t.items)
+        without_nones = [t for t in item_types if not isinstance(t, NoneType)]
+        any_none = len(without_nones) < len(item_types)
+        if not any_none:
+            return  # no None in it
+
+        non_truthy = [t for t in without_nones if not self._is_truthy_type(t)]
+        if not non_truthy:
+            return  # all the other types are just 'normal' types, not collections or ints, etc.
+
+        non_truthy_formatted = ", ".join(format_type(t, self.options) for t in non_truthy)
+        self.fail(
+            message_registry.OPTIONAL_WITH_NON_TRUTHY.format(
+                self._format_expr_type(t, expr),
+                "type" if len(non_truthy) == 1 else "types",
+                non_truthy_formatted,
+            ),
+            expr,
+        )
+
+    def check_for_appropriate_truthiness_in_boolean_context(
+        self, t: Type, expr: Expression
+    ) -> None:
+        """Check if a type is truthy or potentially-falsy when it shouldn't be.
+
+        Used in checks like::
+
+            if x: # <---
+
+            not x  # <---
+
+        """
+        self._check_for_truthy_type(t, expr)
+        self._check_for_optional_non_truthy_type(t, expr)
+
     def find_type_equals_check(
         self, node: ComparisonExpr, expr_indices: list[int]
     ) -> tuple[TypeMap, TypeMap]:
@@ -6174,7 +6227,7 @@ def has_no_custom_eq_checks(t: Type) -> bool:
         if in_boolean_context:
             # We don't check `:=` values in expressions like `(a := A())`,
             # because they produce two error messages.
-            self.check_for_truthy_type(original_vartype, node)
+            self.check_for_appropriate_truthiness_in_boolean_context(original_vartype, node)
         vartype = try_expanding_sum_type_to_union(original_vartype, "builtins.bool")
 
         if_type = true_only(vartype)

mypy/checkexpr.py

@@ -4296,7 +4296,7 @@ def visit_unary_expr(self, e: UnaryExpr) -> Type:
         op = e.op
         if op == "not":
             result: Type = self.bool_type()
-            self.chk.check_for_truthy_type(operand_type, e.expr)
+            self.chk.check_for_appropriate_truthiness_in_boolean_context(operand_type, e.expr)
         else:
             method = operators.unary_op_methods[op]
             result, method_type = self.check_method_call_by_name(method, operand_type, [], [], e)

mypy/errorcodes.py

@@ -214,6 +214,12 @@ def __hash__(self) -> int:
     "General",
     default_enabled=False,
 )
+OPTIONAL_NON_TRUTHY: Final[ErrorCode] = ErrorCode(
+    "optional-non-truthy",
+    "Warn about expressions involving both None and other values that can be false in boolean contexts",
+    "General",
+    default_enabled=False,
+)
 NAME_MATCH: Final = ErrorCode(
     "name-match", "Check that type definition has consistent naming", "General"
 )

mypy/message_registry.py

@@ -167,6 +167,10 @@ def with_additional_msg(self, info: str) -> ErrorMessage:
     "{} which can always be true in boolean context. Consider using {} instead.",
     code=codes.TRUTHY_ITERABLE,
 )
+OPTIONAL_WITH_NON_TRUTHY: Final = ErrorMessage(
+    "{} where both None and some other values (of {}: {}) may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.",
+    code=codes.OPTIONAL_NON_TRUTHY,
+)
 NOT_CALLABLE: Final = "{} not callable"
 TYPE_MUST_BE_USED: Final = "Value of type {} must be used"
 

test-data/unit/check-errorcodes.test

@@ -942,6 +942,84 @@ def func(var: Iterable[str]) -> None:
 
     not var  # E: "var" has type "Iterable[str]" which can always be true in boolean context. Consider using "Collection[str]" instead.  [truthy-iterable]
 
+[case testOptionalNonTruthy]
+# flags: --enable-error-code optional-non-truthy
+from __future__ import annotations
+
+from typing import TypeVar
+
+class Truthy: pass
+class Truthy2: pass
+
+class NonTruthy:
+    def __bool__(self) -> bool: return False
+
+def no_error_direct(var: None | Truthy) -> None:
+    if var:
+        ...
+
+    not var
+
+OnlyTruthy = TypeVar("OnlyTruthy", Truthy, Truthy2)
+
+def no_error_typevar(var: None | OnlyTruthy) -> None:
+    if var:
+        ...
+
+    not var
+
+def error_int(var: None | int) -> None:
+    if var:  # E: "var" has type "Optional[int]" where both None and some other values (of type: "int") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Optional[int]" where both None and some other values (of type: "int") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+def error_bool(var: None | bool) -> None:
+    if var:  # E: "var" has type "Optional[bool]" where both None and some other values (of type: "bool") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Optional[bool]" where both None and some other values (of type: "bool") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+def error_list(var: None | list[Truthy]) -> None:
+    if var:  # E: "var" has type "Optional[List[Truthy]]" where both None and some other values (of type: "List[Truthy]") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Optional[List[Truthy]]" where both None and some other values (of type: "List[Truthy]") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+def error_user_defined(var: None | NonTruthy) -> None:
+    if var:  # E: "var" has type "Optional[NonTruthy]" where both None and some other values (of type: "NonTruthy") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Optional[NonTruthy]" where both None and some other values (of type: "NonTruthy") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+def error_union(var: None | int | Truthy | NonTruthy) -> None:
+    if var:  # E: "var" has type "Union[int, Truthy, NonTruthy, None]" where both None and some other values (of types: "int", "NonTruthy") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Union[int, Truthy, NonTruthy, None]" where both None and some other values (of types: "int", "NonTruthy") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+Unconstrained = TypeVar("Unconstrained")
+
+def error_unconstrained_var(var: None | Unconstrained) -> None:
+    if var:  # E: "var" has type "Optional[Unconstrained]" where both None and some other values (of type: "Unconstrained") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Optional[Unconstrained]" where both None and some other values (of type: "Unconstrained") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+Constrained = TypeVar("Constrained", str, int, Truthy, NonTruthy)
+
+def error_constrained_var(var: None | Constrained) -> None:
+    if var:  # E: "var" has type "Optional[str]" where both None and some other values (of type: "str") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy] \
+             # E: "var" has type "Optional[int]" where both None and some other values (of type: "int") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy] \
+             # E: "var" has type "Optional[NonTruthy]" where both None and some other values (of type: "NonTruthy") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+        ...
+
+    not var  # E: "var" has type "Optional[str]" where both None and some other values (of type: "str") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy] \
+             # E: "var" has type "Optional[int]" where both None and some other values (of type: "int") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy] \
+             # E: "var" has type "Optional[NonTruthy]" where both None and some other values (of type: "NonTruthy") may behave as false in boolean contexts. Consider being explicit about the behaviour for None vs other falsy values.  [optional-non-truthy]
+
+[builtins fixtures/list.pyi]
+
 [case testNoOverloadImplementation]
 from typing import overload