[flake8-pyi] Expand Optional[A] to A | None (PYI016) (#18572)
Some checks are pending
CI / cargo fuzz build (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run

## Summary
Under preview 🧪 I've expanded rule `PYI016` to also flag type
union duplicates containing `None` and `Optional`.

## Test Plan
Examples/tests have been added. I've made sure that the existing
examples did not change unless preview is enabled.

## Relevant Issues
* https://github.com/astral-sh/ruff/issues/18508 (discussing
introducing/extending a rule to flag `Optional[None]`)
* https://github.com/astral-sh/ruff/issues/18546 (where I discussed this
addition with @AlexWaygood)

---------

Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
Robsdedude 2025-06-27 15:43:11 +00:00 committed by GitHub
parent 96f3c8d1ab
commit 6802c4702f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 2747 additions and 25 deletions

View file

@ -119,4 +119,26 @@ field35: "int | str | int" # Error
# Technically, this falls into the domain of the rule but it is an unlikely edge case,
# only works if you have from `__future__ import annotations` at the top of the file,
# and stringified annotations are discouraged in stub files.
field36: "int | str" | int # Ok
field36: "int | str" | int # Ok
# https://github.com/astral-sh/ruff/issues/18546
# Expand Optional[T] to Union[T, None]
# OK
field37: typing.Optional[int]
field38: typing.Union[int, None]
# equivalent to None
field39: typing.Optional[None]
# equivalent to int | None
field40: typing.Union[typing.Optional[int], None]
field41: typing.Optional[typing.Union[int, None]]
field42: typing.Union[typing.Optional[int], typing.Optional[int]]
field43: typing.Optional[int] | None
field44: typing.Optional[int | None]
field45: typing.Optional[int] | typing.Optional[int]
# equivalent to int | dict | None
field46: typing.Union[typing.Optional[int], typing.Optional[dict]]
field47: typing.Optional[int] | typing.Optional[dict]
# avoid reporting twice
field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
field49: typing.Optional[complex | complex] | complex

View file

@ -111,3 +111,25 @@ field33: typing.Union[typing.Union[int | int] | typing.Union[int | int]] # Error
# Test case for mixed union type
field34: typing.Union[list[int], str] | typing.Union[bytes, list[int]] # Error
# https://github.com/astral-sh/ruff/issues/18546
# Expand Optional[T] to Union[T, None]
# OK
field37: typing.Optional[int]
field38: typing.Union[int, None]
# equivalent to None
field39: typing.Optional[None]
# equivalent to int | None
field40: typing.Union[typing.Optional[int], None]
field41: typing.Optional[typing.Union[int, None]]
field42: typing.Union[typing.Optional[int], typing.Optional[int]]
field43: typing.Optional[int] | None
field44: typing.Optional[int | None]
field45: typing.Optional[int] | typing.Optional[int]
# equivalent to int | dict | None
field46: typing.Union[typing.Optional[int], typing.Optional[dict]]
field47: typing.Optional[int] | typing.Optional[dict]
# avoid reporting twice
field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
field49: typing.Optional[complex | complex] | complex

View file

@ -7,6 +7,7 @@ use ruff_python_semantic::analyze::typing;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::preview::is_optional_as_none_in_union_enabled;
use crate::registry::Rule;
use crate::rules::{
airflow, flake8_2020, flake8_async, flake8_bandit, flake8_boolean_trap, flake8_bugbear,
@ -90,7 +91,13 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if checker.is_rule_enabled(Rule::UnnecessaryLiteralUnion) {
flake8_pyi::rules::unnecessary_literal_union(checker, expr);
}
if checker.is_rule_enabled(Rule::DuplicateUnionMember) {
if checker.is_rule_enabled(Rule::DuplicateUnionMember)
// Avoid duplicate checks inside `Optional`
&& !(
is_optional_as_none_in_union_enabled(checker.settings())
&& checker.semantic.inside_optional()
)
{
flake8_pyi::rules::duplicate_union_member(checker, expr);
}
if checker.is_rule_enabled(Rule::RedundantLiteralUnion) {
@ -1430,6 +1437,11 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if !checker.semantic.in_nested_union() {
if checker.is_rule_enabled(Rule::DuplicateUnionMember)
&& checker.semantic.in_type_definition()
// Avoid duplicate checks inside `Optional`
&& !(
is_optional_as_none_in_union_enabled(checker.settings())
&& checker.semantic.inside_optional()
)
{
flake8_pyi::rules::duplicate_union_member(checker, expr);
}

View file

@ -90,6 +90,11 @@ pub(crate) const fn is_ignore_init_files_in_useless_alias_enabled(
settings.preview.is_enabled()
}
// https://github.com/astral-sh/ruff/pull/18572
pub(crate) const fn is_optional_as_none_in_union_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
}
// https://github.com/astral-sh/ruff/pull/18547
pub(crate) const fn is_invalid_async_mock_access_check_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()

View file

@ -11,6 +11,7 @@ mod tests {
use crate::registry::Rule;
use crate::rules::pep8_naming;
use crate::settings::types::PreviewMode;
use crate::test::test_path;
use crate::{assert_diagnostics, settings};
@ -172,4 +173,23 @@ mod tests {
assert_diagnostics!(snapshot, diagnostics);
Ok(())
}
#[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))]
#[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("flake8_pyi").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_diagnostics!(snapshot, diagnostics);
Ok(())
}
}

View file

@ -1,17 +1,16 @@
use std::collections::HashSet;
use rustc_hash::FxHashSet;
use std::collections::HashSet;
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::{Expr, ExprBinOp, Operator, PythonVersion};
use ruff_python_semantic::analyze::typing::traverse_union;
use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
use ruff_python_ast::{AtomicNodeIndex, Expr, ExprBinOp, ExprNoneLiteral, Operator, PythonVersion};
use ruff_python_semantic::analyze::typing::{traverse_union, traverse_union_and_optional};
use ruff_text_size::{Ranged, TextRange, TextSize};
use super::generate_union_fix;
use crate::checkers::ast::Checker;
use crate::preview::is_optional_as_none_in_union_enabled;
use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
/// ## What it does
/// Checks for duplicate union members.
@ -71,21 +70,35 @@ pub(crate) fn duplicate_union_member<'a>(checker: &Checker, expr: &'a Expr) {
union_type = UnionKind::PEP604;
}
let virtual_expr = if is_optional_as_none_in_union_enabled(checker.settings())
&& is_optional_type(checker, expr)
{
// If the union member is an `Optional`, add a virtual `None` literal.
&VIRTUAL_NONE_LITERAL
} else {
expr
};
// If we've already seen this union member, raise a violation.
if seen_nodes.insert(expr.into()) {
unique_nodes.push(expr);
if seen_nodes.insert(virtual_expr.into()) {
unique_nodes.push(virtual_expr);
} else {
diagnostics.push(checker.report_diagnostic(
DuplicateUnionMember {
duplicate_name: checker.generator().expr(expr),
duplicate_name: checker.generator().expr(virtual_expr),
},
// Use the real expression's range for diagnostics,
expr.range(),
));
}
};
// Traverse the union, collect all diagnostic members
traverse_union(&mut check_for_duplicate_members, checker.semantic(), expr);
if is_optional_as_none_in_union_enabled(checker.settings()) {
traverse_union_and_optional(&mut check_for_duplicate_members, checker.semantic(), expr);
} else {
traverse_union(&mut check_for_duplicate_members, checker.semantic(), expr);
}
if diagnostics.is_empty() {
return;
@ -178,3 +191,12 @@ fn generate_pep604_fix(
applicability,
)
}
static VIRTUAL_NONE_LITERAL: Expr = Expr::NoneLiteral(ExprNoneLiteral {
node_index: AtomicNodeIndex::dummy(),
range: TextRange::new(TextSize::new(0), TextSize::new(0)),
});
fn is_optional_type(checker: &Checker, expr: &Expr) -> bool {
checker.semantic().match_typing_expr(expr, "Optional")
}

View file

@ -914,4 +914,79 @@ PYI016.py:115:23: PYI016 [*] Duplicate union member `int`
115 |+field35: "int | str" # Error
116 116 |
117 117 |
118 118 |
118 118 |
PYI016.py:134:45: PYI016 [*] Duplicate union member `typing.Optional[int]`
|
132 | field40: typing.Union[typing.Optional[int], None]
133 | field41: typing.Optional[typing.Union[int, None]]
134 | field42: typing.Union[typing.Optional[int], typing.Optional[int]]
| ^^^^^^^^^^^^^^^^^^^^ PYI016
135 | field43: typing.Optional[int] | None
136 | field44: typing.Optional[int | None]
|
= help: Remove duplicate union member `typing.Optional[int]`
Safe fix
131 131 | # equivalent to int | None
132 132 | field40: typing.Union[typing.Optional[int], None]
133 133 | field41: typing.Optional[typing.Union[int, None]]
134 |-field42: typing.Union[typing.Optional[int], typing.Optional[int]]
134 |+field42: typing.Optional[int]
135 135 | field43: typing.Optional[int] | None
136 136 | field44: typing.Optional[int | None]
137 137 | field45: typing.Optional[int] | typing.Optional[int]
PYI016.py:137:33: PYI016 [*] Duplicate union member `typing.Optional[int]`
|
135 | field43: typing.Optional[int] | None
136 | field44: typing.Optional[int | None]
137 | field45: typing.Optional[int] | typing.Optional[int]
| ^^^^^^^^^^^^^^^^^^^^ PYI016
138 | # equivalent to int | dict | None
139 | field46: typing.Union[typing.Optional[int], typing.Optional[dict]]
|
= help: Remove duplicate union member `typing.Optional[int]`
Safe fix
134 134 | field42: typing.Union[typing.Optional[int], typing.Optional[int]]
135 135 | field43: typing.Optional[int] | None
136 136 | field44: typing.Optional[int | None]
137 |-field45: typing.Optional[int] | typing.Optional[int]
137 |+field45: typing.Optional[int]
138 138 | # equivalent to int | dict | None
139 139 | field46: typing.Union[typing.Optional[int], typing.Optional[dict]]
140 140 | field47: typing.Optional[int] | typing.Optional[dict]
PYI016.py:143:61: PYI016 [*] Duplicate union member `complex`
|
142 | # avoid reporting twice
143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
| ^^^^^^^ PYI016
144 | field49: typing.Optional[complex | complex] | complex
|
= help: Remove duplicate union member `complex`
Safe fix
140 140 | field47: typing.Optional[int] | typing.Optional[dict]
141 141 |
142 142 | # avoid reporting twice
143 |-field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
143 |+field48: typing.Union[typing.Optional[complex], complex]
144 144 | field49: typing.Optional[complex | complex] | complex
PYI016.py:144:36: PYI016 [*] Duplicate union member `complex`
|
142 | # avoid reporting twice
143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
144 | field49: typing.Optional[complex | complex] | complex
| ^^^^^^^ PYI016
|
= help: Remove duplicate union member `complex`
Safe fix
141 141 |
142 142 | # avoid reporting twice
143 143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
144 |-field49: typing.Optional[complex | complex] | complex
144 |+field49: typing.Optional[complex] | complex

View file

@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
snapshot_kind: text
---
PYI016.pyi:7:15: PYI016 [*] Duplicate union member `str`
|
@ -883,6 +882,8 @@ PYI016.pyi:113:61: PYI016 [*] Duplicate union member `list[int]`
112 | # Test case for mixed union type
113 | field34: typing.Union[list[int], str] | typing.Union[bytes, list[int]] # Error
| ^^^^^^^^^ PYI016
114 |
115 | # https://github.com/astral-sh/ruff/issues/18546
|
= help: Remove duplicate union member `list[int]`
@ -892,3 +893,81 @@ PYI016.pyi:113:61: PYI016 [*] Duplicate union member `list[int]`
112 112 | # Test case for mixed union type
113 |-field34: typing.Union[list[int], str] | typing.Union[bytes, list[int]] # Error
113 |+field34: typing.Union[list[int], str, bytes] # Error
114 114 |
115 115 | # https://github.com/astral-sh/ruff/issues/18546
116 116 | # Expand Optional[T] to Union[T, None]
PYI016.pyi:125:45: PYI016 [*] Duplicate union member `typing.Optional[int]`
|
123 | field40: typing.Union[typing.Optional[int], None]
124 | field41: typing.Optional[typing.Union[int, None]]
125 | field42: typing.Union[typing.Optional[int], typing.Optional[int]]
| ^^^^^^^^^^^^^^^^^^^^ PYI016
126 | field43: typing.Optional[int] | None
127 | field44: typing.Optional[int | None]
|
= help: Remove duplicate union member `typing.Optional[int]`
Safe fix
122 122 | # equivalent to int | None
123 123 | field40: typing.Union[typing.Optional[int], None]
124 124 | field41: typing.Optional[typing.Union[int, None]]
125 |-field42: typing.Union[typing.Optional[int], typing.Optional[int]]
125 |+field42: typing.Optional[int]
126 126 | field43: typing.Optional[int] | None
127 127 | field44: typing.Optional[int | None]
128 128 | field45: typing.Optional[int] | typing.Optional[int]
PYI016.pyi:128:33: PYI016 [*] Duplicate union member `typing.Optional[int]`
|
126 | field43: typing.Optional[int] | None
127 | field44: typing.Optional[int | None]
128 | field45: typing.Optional[int] | typing.Optional[int]
| ^^^^^^^^^^^^^^^^^^^^ PYI016
129 | # equivalent to int | dict | None
130 | field46: typing.Union[typing.Optional[int], typing.Optional[dict]]
|
= help: Remove duplicate union member `typing.Optional[int]`
Safe fix
125 125 | field42: typing.Union[typing.Optional[int], typing.Optional[int]]
126 126 | field43: typing.Optional[int] | None
127 127 | field44: typing.Optional[int | None]
128 |-field45: typing.Optional[int] | typing.Optional[int]
128 |+field45: typing.Optional[int]
129 129 | # equivalent to int | dict | None
130 130 | field46: typing.Union[typing.Optional[int], typing.Optional[dict]]
131 131 | field47: typing.Optional[int] | typing.Optional[dict]
PYI016.pyi:134:61: PYI016 [*] Duplicate union member `complex`
|
133 | # avoid reporting twice
134 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
| ^^^^^^^ PYI016
135 | field49: typing.Optional[complex | complex] | complex
|
= help: Remove duplicate union member `complex`
Safe fix
131 131 | field47: typing.Optional[int] | typing.Optional[dict]
132 132 |
133 133 | # avoid reporting twice
134 |-field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
134 |+field48: typing.Union[typing.Optional[complex], complex]
135 135 | field49: typing.Optional[complex | complex] | complex
PYI016.pyi:135:36: PYI016 [*] Duplicate union member `complex`
|
133 | # avoid reporting twice
134 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
135 | field49: typing.Optional[complex | complex] | complex
| ^^^^^^^ PYI016
|
= help: Remove duplicate union member `complex`
Safe fix
132 132 |
133 133 | # avoid reporting twice
134 134 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
135 |-field49: typing.Optional[complex | complex] | complex
135 |+field49: typing.Optional[complex] | complex

View file

@ -24,8 +24,8 @@ pub struct AtomicNodeIndex(AtomicU32);
impl AtomicNodeIndex {
/// Returns a placeholder `AtomicNodeIndex`.
pub fn dummy() -> AtomicNodeIndex {
AtomicNodeIndex(AtomicU32::from(u32::MAX))
pub const fn dummy() -> AtomicNodeIndex {
AtomicNodeIndex(AtomicU32::new(u32::MAX))
}
/// Load the current value of the `AtomicNodeIndex`.

View file

@ -428,12 +428,52 @@ pub fn is_sys_version_block(stmt: &ast::StmtIf, semantic: &SemanticModel) -> boo
pub fn traverse_union<'a, F>(func: &mut F, semantic: &SemanticModel, expr: &'a Expr)
where
F: FnMut(&'a Expr, &'a Expr),
{
traverse_union_options(func, semantic, expr, UnionTraversalOptions::default());
}
/// Traverse a "union" type annotation, applying `func` to each union member.
///
/// Supports traversal of `Union`, `|`, and `Optional` union expressions.
///
/// The function is called with each expression in the union (excluding declarations of nested
/// unions) and the parent expression.
pub fn traverse_union_and_optional<'a, F>(func: &mut F, semantic: &SemanticModel, expr: &'a Expr)
where
F: FnMut(&'a Expr, &'a Expr),
{
traverse_union_options(
func,
semantic,
expr,
UnionTraversalOptions {
traverse_optional: true,
},
);
}
#[derive(Debug, Clone, Copy, Default)]
/// Options for traversing union types.
///
/// See also [`traverse_union_options`].
struct UnionTraversalOptions {
traverse_optional: bool,
}
fn traverse_union_options<'a, F>(
func: &mut F,
semantic: &SemanticModel,
expr: &'a Expr,
options: UnionTraversalOptions,
) where
F: FnMut(&'a Expr, &'a Expr),
{
fn inner<'a, F>(
func: &mut F,
semantic: &SemanticModel,
expr: &'a Expr,
parent: Option<&'a Expr>,
options: UnionTraversalOptions,
) where
F: FnMut(&'a Expr, &'a Expr),
{
@ -456,25 +496,31 @@ where
// in the order they appear in the source code.
// Traverse the left then right arms
inner(func, semantic, left, Some(expr));
inner(func, semantic, right, Some(expr));
inner(func, semantic, left, Some(expr), options);
inner(func, semantic, right, Some(expr), options);
return;
}
// Ex) `Union[x, y]`
if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr {
// Ex) `Union[x, y]`
if semantic.match_typing_expr(value, "Union") {
if let Expr::Tuple(tuple) = &**slice {
// Traverse each element of the tuple within the union recursively to handle cases
// such as `Union[..., Union[...]]`
tuple
.iter()
.for_each(|elem| inner(func, semantic, elem, Some(expr)));
.for_each(|elem| inner(func, semantic, elem, Some(expr), options));
return;
}
// Ex) `Union[Union[a, b]]` and `Union[a | b | c]`
inner(func, semantic, slice, Some(expr));
inner(func, semantic, slice, Some(expr), options);
return;
}
// Ex) `Optional[x]`
if options.traverse_optional && semantic.match_typing_expr(value, "Optional") {
inner(func, semantic, value, Some(expr), options);
inner(func, semantic, slice, Some(expr), options);
return;
}
}
@ -485,7 +531,7 @@ where
}
}
inner(func, semantic, expr, None);
inner(func, semantic, expr, None, options);
}
/// Traverse a "literal" type annotation, applying `func` to each literal member.

View file

@ -1604,7 +1604,7 @@ impl<'a> SemanticModel<'a> {
let mut parent_expressions = self.current_expressions().skip(1);
match parent_expressions.next() {
// The parent expression is of the inner union is a single `typing.Union`.
// The parent expression of the inner union is a single `typing.Union`.
// Ex) `Union[Union[a, b]]`
Some(Expr::Subscript(parent)) => self.match_typing_expr(&parent.value, "Union"),
// The parent expression is of the inner union is a tuple with two or more
@ -1624,6 +1624,18 @@ impl<'a> SemanticModel<'a> {
}
}
/// Return `true` if the model is directly inside an Optional (e.g., the inner `Union` in
/// `Optional[Union[int, str]]`).
pub fn inside_optional(&self) -> bool {
let mut parent_expressions = self.current_expressions().skip(1);
matches!(
parent_expressions.next(),
// The parent expression is a single `typing.Optional`.
// Ex) `Optional[EXPR]`
Some(Expr::Subscript(parent)) if self.match_typing_expr(&parent.value, "Optional")
)
}
/// Return `true` if the model is in a nested literal expression (e.g., the inner `Literal` in
/// `Literal[Literal[int, str], float]`).
pub fn in_nested_literal(&self) -> bool {