Skip to content

[flake8-pyi]: Stabilize: Provide more automated fixes for duplicate-union-members (PYI016) #15342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MichaReiser merged 2 commits into from
Jan 8, 2025
+1,042 −119
Merged

[flake8-pyi]: Stabilize: Provide more automated fixes for duplicate-union-members (PYI016) #15342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
246c452
[`flake8-pyi`]: Stabilize: Provide more automated fixes for `duplicat…
MichaReiser Jan 8, 2025
358ebb7
Review feedback
MichaReiser Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI016.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,12 @@ def func2() -> str | str: # PYI016: Duplicate union member `str`

# Test case for mixed union type
field34: typing.Union[list[int], str] | typing.Union[bytes, list[int]] # Error

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
60 changes: 29 additions & 31 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_union_member.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,40 +93,38 @@ pub(crate) fn duplicate_union_member<'a>(checker: &mut Checker, expr: &'a Expr)
return;
}

if checker.settings.preview.is_enabled() {
// Mark [`Fix`] as unsafe when comments are in range.
let applicability = if checker.comment_ranges().intersects(expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
};

// Generate the flattened fix once.
let fix = if let &[edit_expr] = unique_nodes.as_slice() {
// Generate a [`Fix`] for a single type expression, e.g. `int`.
Some(Fix::applicable_edit(
Edit::range_replacement(checker.generator().expr(edit_expr), expr.range()),
// Mark [`Fix`] as unsafe when comments are in range.
Copy link
Copy Markdown
Member Author

@MichaReiser MichaReiser Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like #14270 incorrectly removed the fix for non-preview 🤷 So, technically, this PR makes the rule fixable again

Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe, onwards and upwards!

let applicability = if checker.comment_ranges().intersects(expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
};

// Generate the flattened fix once.
let fix = if let &[edit_expr] = unique_nodes.as_slice() {
// Generate a [`Fix`] for a single type expression, e.g. `int`.
Some(Fix::applicable_edit(
Edit::range_replacement(checker.generator().expr(edit_expr), expr.range()),
applicability,
))
} else {
match union_type {
// See redundant numeric union
UnionKind::PEP604 => Some(generate_pep604_fix(
checker,
unique_nodes,
expr,
applicability,
))
} else {
match union_type {
// See redundant numeric union
UnionKind::PEP604 => Some(generate_pep604_fix(
checker,
unique_nodes,
expr,
applicability,
)),
UnionKind::TypingUnion => {
generate_union_fix(checker, unique_nodes, expr, applicability).ok()
}
)),
UnionKind::TypingUnion => {
generate_union_fix(checker, unique_nodes, expr, applicability).ok()
}
};
}
};

if let Some(fix) = fix {
for diagnostic in &mut diagnostics {
diagnostic.set_fix(fix.clone());
}
if let Some(fix) = fix {
for diagnostic in &mut diagnostics {
diagnostic.set_fix(fix.clone());
}
}

Expand Down
Loading