Enforce consistent use of Literal and None (#435)

Co-authored-by: Alex Waygood <[email protected]>

Author: tomasr8

CHANGELOG.md

@@ -9,6 +9,8 @@ New error codes:
 * Introduce Y059: `Generic[]` should always be the last base class, if it is
   present in the bases of a class.
 * Introduce Y060, which flags redundant inheritance from `Generic[]`.
+* Introduce Y061: Do not use `None` inside a `Literal[]` slice.
+  For example, use `Literal["foo"] | None` instead of `Literal["foo", None]`.
 
 Other changes:
 * The undocumented `pyi.__version__` and `pyi.PyiTreeChecker.version`

ERRORCODES.md

@@ -63,6 +63,7 @@ The following warnings are currently emitted by default:
 | Y058 | Use `Iterator` rather than `Generator` as the return value for simple `__iter__` methods, and `AsyncIterator` rather than `AsyncGenerator` as the return value for simple `__aiter__` methods. Using `(Async)Iterator` for these methods is simpler and more elegant, and reflects the fact that the precise kind of iterator returned from an `__iter__` method is usually an implementation detail that could change at any time, and should not be relied upon.
 | Y059 | `Generic[]` should always be the last base class, if it is present in a class's bases tuple. At runtime, if `Generic[]` is not the final class in a the bases tuple, this [can cause the class creation to fail](https://github.com/python/cpython/issues/106102). In a stub file, however, this rule is enforced purely for stylistic consistency.
 | Y060 | Redundant inheritance from `Generic[]`. For example, `class Foo(Iterable[_T], Generic[_T]): ...` can be written more simply as `class Foo(Iterable[_T]): ...`.<br><br>To avoid false-positive errors, and to avoid complexity in the implementation, this check is deliberately conservative: it only looks at classes that have exactly two bases.
+| Y061 | Do not use `None` inside a `Literal[]` slice. For example, use `Literal["foo"] \| None` instead of `Literal["foo", None]`. While both are legal according to [PEP 586](https://peps.python.org/pep-0586/), the former is preferred for stylistic consistency.
 
 ## Warnings disabled by default
 

pyi.py

@@ -1359,7 +1359,7 @@ def visit_Subscript(self, node: ast.Subscript) -> None:
         self.visit(subscripted_object)
         if subscripted_object_name == "Literal":
             with self.string_literals_allowed.enabled():
-                self.visit(node.slice)
+                self._visit_typing_Literal(node)
             return
 
         if isinstance(node.slice, ast.Tuple):
@@ -1369,6 +1369,25 @@ def visit_Subscript(self, node: ast.Subscript) -> None:
             if subscripted_object_name in {"tuple", "Tuple"}:
                 self._Y090_error(node)
 
+    def _visit_typing_Literal(self, node: ast.Subscript) -> None:
+        if isinstance(node.slice, ast.Constant) and _is_None(node.slice):
+            # Special case for `Literal[None]`
+            self.error(node.slice, Y061.format(suggestion="None"))
+        elif isinstance(node.slice, ast.Tuple):
+            elts = node.slice.elts
+            for i, elt in enumerate(elts):
+                if _is_None(elt):
+                    elts_without_none = elts[:i] + elts[i + 1 :]
+                    if len(elts_without_none) == 1:
+                        new_literal_slice = unparse(elts_without_none[0])
+                    else:
+                        new_slice_node = ast.Tuple(elts=elts_without_none)
+                        new_literal_slice = unparse(new_slice_node).strip("()")
+                    suggestion = f"Literal[{new_literal_slice}] | None"
+                    self.error(elt, Y061.format(suggestion=suggestion))
+                    break  # Only report the first `None`
+        self.visit(node.slice)
+
     def _visit_slice_tuple(self, node: ast.Tuple, parent: str | None) -> None:
         if parent == "Union":
             self._check_union_members(node.elts, is_pep_604_union=False)
@@ -2180,6 +2199,7 @@ def parse_options(options: argparse.Namespace) -> None:
     'Y060 Redundant inheritance from "Generic[]"; '
     "class would be inferred as generic anyway"
 )
+Y061 = 'Y061 None inside "Literal[]" expression. Replace with "{suggestion}"'
 Y090 = (
     'Y090 "{original}" means '
     '"a tuple of length 1, in which the sole element is of type {typ!r}". '

tests/literals.pyi

@@ -0,0 +1,5 @@
+from typing import Literal
+
+Literal[None]  # Y061 None inside "Literal[]" expression. Replace with "None"
+Literal[True, None]  # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
+Literal[1, None, "foo", None]  # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo', None] | None"