[flake8-pyi] Implement redundant-none-literal (PYI061) (astral-…

…sh#14316)

## Summary

`Literal[None]` can be simplified into `None` in type annotations.

Surprising to see that this is not that rare:
-
https://github.com/langchain-ai/langchain/blob/master/libs/langchain/langchain/chat_models/base.py#L54
-
https://github.com/sqlalchemy/sqlalchemy/blob/main/lib/sqlalchemy/sql/annotation.py#L69
- https://github.com/jax-ml/jax/blob/main/jax/numpy/__init__.pyi#L961
-
https://github.com/huggingface/huggingface_hub/blob/main/src/huggingface_hub/inference/_common.py#L179

## Test Plan

`cargo test`

Reviewed all ecosystem results, and they are true positives.

---------

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

crates/red_knot_workspace/tests/check.rs

@@ -256,6 +256,7 @@ const KNOWN_FAILURES: &[(&str, bool, bool)] = &[
     ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI052.py", true, true),
     ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py", true, true),
     ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py", true, true),
+    ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py", true, true),
     ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py", true, true),
     ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI062.py", true, true),
     ("crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI063.py", true, true),

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py

@@ -0,0 +1,60 @@
+from typing import Literal
+
+
+def func1(arg1: Literal[None]):
+    ...
+
+
+def func2(arg1: Literal[None] | int):
+    ...
+
+
+def func3() -> Literal[None]:
+    ...
+
+
+def func4(arg1: Literal[int, None, float]):
+    ...
+
+
+def func5(arg1: Literal[None, None]): 
+    ...
+
+
+def func6(arg1: Literal[
+    "hello",
+    None  # Comment 1
+    , "world"
+    ]): 
+    ...
+
+
+def func7(arg1: Literal[
+    None  # Comment 1
+    ]): 
+    ...
+
+
+# OK
+def good_func(arg1: Literal[int] | None):
+    ...
+
+
+# From flake8-pyi
+Literal[None]  # Y061 None inside "Literal[]" expression. Replace with "None"
+Literal[True, None]  # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
+
+###
+# The following rules here are slightly subtle,
+# but make sense when it comes to giving the best suggestions to users of flake8-pyi.
+###
+
+# If Y061 and Y062 both apply, but all the duplicate members are None,
+# only emit Y061...
+Literal[None, None]  # Y061 None inside "Literal[]" expression. Replace with "None"
+Literal[1, None, "foo", None]  # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
+
+# ... but if Y061 and Y062 both apply
+# and there are no None members in the Literal[] slice,
+# only emit Y062:
+Literal[None, True, None, True]  # Y062 Duplicate "Literal[]" member "True"

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi

@@ -0,0 +1,37 @@
+from typing import Literal
+
+
+def func1(arg1: Literal[None]): ...
+
+
+def func2(arg1: Literal[None] | int): ...
+
+
+def func3() -> Literal[None]: ...
+
+
+def func4(arg1: Literal[int, None, float]): ...
+
+
+def func5(arg1: Literal[None, None]): ...
+
+
+def func6(arg1: Literal[
+    "hello",
+    None  # Comment 1
+    , "world"
+]): ...
+
+
+def func7(arg1: Literal[
+    None  # Comment 1
+]): ...
+
+
+# OK
+def good_func(arg1: Literal[int] | None): ...
+
+
+# From flake8-pyi
+Literal[None]  # PYI061 None inside "Literal[]" expression. Replace with "None"
+Literal[True, None]  # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None"

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -104,9 +104,14 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             }
 
             // Ex) Literal[...]
-            if checker.enabled(Rule::DuplicateLiteralMember) {
+            if checker.any_enabled(&[Rule::DuplicateLiteralMember, Rule::RedundantNoneLiteral]) {
                 if !checker.semantic.in_nested_literal() {
-                    flake8_pyi::rules::duplicate_literal_member(checker, expr);
+                    if checker.enabled(Rule::DuplicateLiteralMember) {
+                        flake8_pyi::rules::duplicate_literal_member(checker, expr);
+                    }
+                    if checker.enabled(Rule::RedundantNoneLiteral) {
+                        flake8_pyi::rules::redundant_none_literal(checker, expr);
+                    }
                 }
             }
 

crates/ruff_linter/src/codes.rs

@@ -785,6 +785,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Pyi, "058") => (RuleGroup::Stable, rules::flake8_pyi::rules::GeneratorReturnFromIterMethod),
         (Flake8Pyi, "057") => (RuleGroup::Stable, rules::flake8_pyi::rules::ByteStringUsage),
         (Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass),
+        (Flake8Pyi, "061") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantNoneLiteral),
         (Flake8Pyi, "062") => (RuleGroup::Stable, rules::flake8_pyi::rules::DuplicateLiteralMember),
         (Flake8Pyi, "063") => (RuleGroup::Preview, rules::flake8_pyi::rules::PrePep570PositionalArgument),
         (Flake8Pyi, "064") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantFinalLiteral),

crates/ruff_linter/src/rules/flake8_pyi/mod.rs

@@ -124,6 +124,8 @@ mod tests {
     #[test_case(Rule::UnusedPrivateTypedDict, Path::new("PYI049.pyi"))]
     #[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.py"))]
     #[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.pyi"))]
+    #[test_case(Rule::RedundantNoneLiteral, Path::new("PYI061.py"))]
+    #[test_case(Rule::RedundantNoneLiteral, Path::new("PYI061.pyi"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs

@@ -27,6 +27,7 @@ pub(crate) use prefix_type_params::*;
 pub(crate) use quoted_annotation_in_stub::*;
 pub(crate) use redundant_final_literal::*;
 pub(crate) use redundant_literal_union::*;
+pub(crate) use redundant_none_literal::*;
 pub(crate) use redundant_numeric_union::*;
 pub(crate) use simple_defaults::*;
 pub(crate) use str_or_repr_defined_in_stub::*;
@@ -69,6 +70,7 @@ mod prefix_type_params;
 mod quoted_annotation_in_stub;
 mod redundant_final_literal;
 mod redundant_literal_union;
+mod redundant_none_literal;
 mod redundant_numeric_union;
 mod simple_defaults;
 mod str_or_repr_defined_in_stub;

crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs

@@ -0,0 +1,112 @@
+use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{Expr, ExprNoneLiteral};
+use ruff_python_semantic::analyze::typing::traverse_literal;
+use ruff_text_size::Ranged;
+
+use smallvec::SmallVec;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for redundant `Literal[None]` annotations.
+///
+/// ## Why is this bad?
+/// While `Literal[None]` is a valid type annotation, it is semantically equivalent to `None`.
+/// Prefer `None` over `Literal[None]` for both consistency and readability.
+///
+/// ## Example
+/// ```python
+/// from typing import Literal
+///
+/// Literal[None]
+/// Literal[1, 2, 3, "foo", 5, None]
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from typing import Literal
+///
+/// None
+/// Literal[1, 2, 3, "foo", 5] | None
+/// ```
+///
+/// ## References
+/// - [Typing documentation: Legal parameters for `Literal` at type check time](https://typing.readthedocs.io/en/latest/spec/literal.html#legal-parameters-for-literal-at-type-check-time)
+#[violation]
+pub struct RedundantNoneLiteral {
+    other_literal_elements_seen: bool,
+}
+
+impl Violation for RedundantNoneLiteral {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        if self.other_literal_elements_seen {
+            "`Literal[None, ...]` can be replaced with `Literal[...] | None`".to_string()
+        } else {
+            "`Literal[None]` can be replaced with `None`".to_string()
+        }
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some(if self.other_literal_elements_seen {
+            "Replace with `Literal[...] | None`".to_string()
+        } else {
+            "Replace with `None`".to_string()
+        })
+    }
+}
+
+/// RUF037
+pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) {
+    if !checker.semantic().seen_typing() {
+        return;
+    }
+
+    let mut none_exprs: SmallVec<[&ExprNoneLiteral; 1]> = SmallVec::new();
+    let mut other_literal_elements_seen = false;
+
+    let mut find_none = |expr: &'a Expr, _parent: &'a Expr| {
+        if let Expr::NoneLiteral(none_expr) = expr {
+            none_exprs.push(none_expr);
+        } else {
+            other_literal_elements_seen = true;
+        }
+    };
+
+    traverse_literal(&mut find_none, checker.semantic(), literal_expr);
+
+    if none_exprs.is_empty() {
+        return;
+    }
+
+    // Provide a [`Fix`] when the complete `Literal` can be replaced. Applying the fix
+    // can leave an unused import to be fixed by the `unused-import` rule.
+    let fix = if other_literal_elements_seen {
+        None
+    } else {
+        Some(Fix::applicable_edit(
+            Edit::range_replacement("None".to_string(), literal_expr.range()),
+            if checker.comment_ranges().intersects(literal_expr.range()) {
+                Applicability::Unsafe
+            } else {
+                Applicability::Safe
+            },
+        ))
+    };
+
+    for none_expr in none_exprs {
+        let mut diagnostic = Diagnostic::new(
+            RedundantNoneLiteral {
+                other_literal_elements_seen,
+            },
+            none_expr.range(),
+        );
+        if let Some(ref fix) = fix {
+            diagnostic.set_fix(fix.clone());
+        }
+        checker.diagnostics.push(diagnostic);
+    }
+}