[flake8-pyi] Implement duplicate types in unions (PYI016)

crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.py

@@ -0,0 +1,32 @@
+# Shouldn't affect non-union field types
+field1: str
+
+# Should emit for duplicate field types
+field2: str | str  # PYI016 Duplicate union member `str`
+
+
+# Should emit for union types in arguments
+def func1(arg1: int | int):  # PYI016 Duplicate union member `int`
+    print(arg1)
+
+
+# Should emit for unions in return types
+def func2() -> str | str:  # PYI016 Duplicate union member `str`
+    return "my string"
+
+
+# Should emit in longer unions, even if not directly adjacent
+field3: str | str | int  # PYI016 Duplicate union member `str`
+field4: int | int | str  # PYI016 Duplicate union member `int`
+field5: str | int | str  # PYI016 Duplicate union member `str`
+field6: int | bool | str | int  # PYI016 Duplicate union member `int`
+
+# Shouldn't emit for non-type unions
+field7 = str | str
+
+# Should emit for strangely-bracketed unions
+field8: int | (str | int)  # PYI016 Duplicate union member `int`
+
+# Should handle user brackets when autorixing
+field9: int | (int | str)  # PYI016 Duplicate union member `int`
+field10: (str | int) | str  # PYI016 Duplicate union member `str`

crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.pyi

@@ -0,0 +1,29 @@
+# Shouldn't affect non-union field types
+field1: str
+
+# Should emit for duplicate field types
+field2: str | str  # PYI016 Duplicate union member `str`
+
+# Should emit for union types in arguments
+def func1(arg1: int | int):  # PYI016 Duplicate union member `int`
+    print(arg1)
+
+# Should emit for unions in return types
+def func2() -> str | str:  # PYI016 Duplicate union member `str`
+    return "my string"
+
+# Should emit in longer unions, even if not directly adjacent
+field3: str | str | int  # PYI016 Duplicate union member `str`
+field4: int | int | str  # PYI016 Duplicate union member `int`
+field5: str | int | str  # PYI016 Duplicate union member `str`
+field6: int | bool | str | int  # PYI016 Duplicate union member `int`
+
+# Shouldn't emit for non-type unions
+field7 = str | str
+
+# Should emit for strangely-bracketed unions
+field8: int | (str | int)  # PYI016 Duplicate union member `int`
+
+# Should handle user brackets when autorixing
+field9: int | (int | str)  # PYI016 Duplicate union member `int`
+field10: (str | int) | str  # PYI016 Duplicate union member `str`

crates/ruff/src/checkers/ast/mod.rs

@@ -3214,6 +3214,19 @@ where
                     flake8_bandit::rules::hardcoded_sql_expression(self, expr);
                 }
             }
+            ExprKind::BinOp {
+                op: Operator::BitOr,
+                ..
+            } => {
+                if self.is_stub {
+                    if self.settings.rules.enabled(Rule::DuplicateUnionMember)
+                        && self.ctx.in_type_definition
+                        && self.ctx.current_expr_parent().is_none()
+                    {
+                        flake8_pyi::rules::duplicate_union_member(self, expr);
+                    }
+                }
+            }
             ExprKind::UnaryOp { op, operand } => {
                 let check_not_in = self.settings.rules.enabled(Rule::NotInTest);
                 let check_not_is = self.settings.rules.enabled(Rule::NotIsTest);

crates/ruff/src/codes.rs

@@ -573,6 +573,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Flake8Pyi, "012") => Rule::PassInClassBody,
         (Flake8Pyi, "014") => Rule::ArgumentDefaultInStub,
         (Flake8Pyi, "015") => Rule::AssignmentDefaultInStub,
+        (Flake8Pyi, "016") => Rule::DuplicateUnionMember,
         (Flake8Pyi, "021") => Rule::DocstringInStub,
         (Flake8Pyi, "033") => Rule::TypeCommentInStub,
 

crates/ruff/src/registry.rs

@@ -527,6 +527,7 @@ ruff_macros::register_rules!(
     rules::flake8_pyi::rules::UnrecognizedPlatformCheck,
     rules::flake8_pyi::rules::UnrecognizedPlatformName,
     rules::flake8_pyi::rules::PassInClassBody,
+    rules::flake8_pyi::rules::DuplicateUnionMember,
     // flake8-pytest-style
     rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle,
     rules::flake8_pytest_style::rules::PytestFixturePositionalArgs,

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

@@ -33,6 +33,8 @@ mod tests {
     #[test_case(Rule::ArgumentDefaultInStub, Path::new("PYI014.pyi"))]
     #[test_case(Rule::AssignmentDefaultInStub, Path::new("PYI015.py"))]
     #[test_case(Rule::AssignmentDefaultInStub, Path::new("PYI015.pyi"))]
+    #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))]
+    #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))]
     #[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))]
     #[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))]
     #[test_case(Rule::TypeCommentInStub, Path::new("PYI033.py"))]

crates/ruff/src/rules/flake8_pyi/rules/duplicate_union_member.rs

@@ -0,0 +1,92 @@
+use rustc_hash::FxHashSet;
+use rustpython_parser::ast::{Expr, ExprKind, Operator};
+
+use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::helpers::unparse_expr;
+use ruff_python_ast::types::Range;
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+
+#[violation]
+pub struct DuplicateUnionMember {
+    pub duplicate_name: String,
+}
+
+impl AlwaysAutofixableViolation for DuplicateUnionMember {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Duplicate union member `{}`", self.duplicate_name)
+    }
+
+    fn autofix_title(&self) -> String {
+        format!("Remove duplicate union member `{}`", self.duplicate_name)
+    }
+}
+
+/// PYI016
+pub fn duplicate_union_member(checker: &mut Checker, expr: &Expr) {
+    let mut seen_nodes = FxHashSet::default();
+    traverse_union(&mut seen_nodes, checker, expr, None);
+}
+
+fn traverse_union<'a>(
+    seen_nodes: &mut FxHashSet<ComparableExpr<'a>>,
+    checker: &mut Checker,
+    expr: &'a Expr,
+    parent: Option<&'a Expr>,
+) {
+    // The union data structure usually looks like this:
+    //  a | b | c -> (a | b) | c
+    //
+    // However, parenthesized expressions can coerce it into any structure:
+    //  a | (b | c)
+    //
+    // So we have to traverse both branches in order (left, then right), to report duplicates
+    // in the order they appear in the source code.
+    if let ExprKind::BinOp {
+        op: Operator::BitOr,
+        left,
+        right,
+    } = &expr.node
+    {
+        // Traverse left subtree, then the right subtree, propagating the previous node.
+        traverse_union(seen_nodes, checker, left, Some(expr));
+        traverse_union(seen_nodes, checker, right, Some(expr));
+    }
+
+    // If we've already seen this union member, raise a violation.
+    if !seen_nodes.insert(expr.into()) {
+        let mut diagnostic = Diagnostic::new(
+            DuplicateUnionMember {
+                duplicate_name: unparse_expr(expr, checker.stylist),
+            },
+            Range::from(expr),
+        );
+        if checker.patch(diagnostic.kind.rule()) {
+            // Delete the "|" character as well as the duplicate value by reconstructing the
+            // parent without the duplicate
+
+            // SAFETY: impossible to have a duplicate without a `parent` node.
+            let parent = parent.unwrap();
+
+            // SAFETY: Parent node must have been a BinOp in order for us to have traversed it
+            let ExprKind::BinOp { left, right, .. } = &parent.node
+            else {
+                unreachable!();
+            };
+
+            // Replace parent with the non-duplicate of its children
+            let fixed_parent = if expr.node == left.node { right } else { left };
+
+            diagnostic.set_fix(Edit::replacement(
+                unparse_expr(fixed_parent, checker.stylist),
+                parent.location,
+                parent.end_location.unwrap(),
+            ));
+        }
+        checker.diagnostics.push(diagnostic);
+    }
+}

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

@@ -1,5 +1,6 @@
 pub use bad_version_info_comparison::{bad_version_info_comparison, BadVersionInfoComparison};
 pub use docstring_in_stubs::{docstring_in_stubs, DocstringInStub};
+pub use duplicate_union_member::{duplicate_union_member, DuplicateUnionMember};
 pub use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody};
 pub use pass_in_class_body::{pass_in_class_body, PassInClassBody};
 pub use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody};
@@ -15,6 +16,7 @@ pub use unrecognized_platform::{
 
 mod bad_version_info_comparison;
 mod docstring_in_stubs;
+mod duplicate_union_member;
 mod non_empty_stub_body;
 mod pass_in_class_body;
 mod pass_statement_stub_body;

crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI016_PYI016.py.snap

@@ -0,0 +1,6 @@
+---
+source: crates/ruff/src/rules/flake8_pyi/mod.rs
+expression: diagnostics
+---
+[]
+