Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 54 additions & 3 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import typing

# 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`
Expand All @@ -33,3 +32,55 @@ def func2() -> str | str: # PYI016: Duplicate union member `str`

# Should emit for nested unions.
field11: dict[int | int, str]

# Should emit for unions with more than two cases
field12: int | int | int # Error
field13: int | int | int | int # Error

# Should emit for unions with more than two cases, even if not directly adjacent
field14: int | int | str | int # Error

# Should emit for duplicate literal types; also covered by PYI030
field15: typing.Literal[1] | typing.Literal[1] # Error

# Shouldn't emit if in new parent type
field16: int | dict[int, str] # OK

# Shouldn't emit if not in a union parent
field17: dict[int, int] # OK

# Should emit in cases with newlines
field18: typing.Union[
set[
int # foo
],
set[
int # bar
],
] # Error, newline and comment will not be emitted in message

# Should emit in cases with `typing.Union` instead of `|`
field19: typing.Union[int, int] # Error

# Should emit in cases with nested `typing.Union`
field20: typing.Union[int, typing.Union[int, str]] # Error

# Should emit in cases with mixed `typing.Union` and `|`
field21: typing.Union[int, int | str] # Error

# Should emit only once in cases with multiple nested `typing.Union`
field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error

# Should emit in cases with newlines
field23: set[ # foo
int] | set[int]

# Should emit twice (once for each `int` in the nested union, both of which are
# duplicates of the outer `int`), but not three times (which would indicate that
# we incorrectly re-checked the nested union).
field24: typing.Union[int, typing.Union[int, int]] # PYI016: Duplicate union member `int`

# Should emit twice (once for each `int` in the nested union, both of which are
# duplicates of the outer `int`), but not three times (which would indicate that
# we incorrectly re-checked the nested union).
field25: typing.Union[int, int | int] # PYI016: Duplicate union member `int`
Copy link
Member Author

Choose a reason for hiding this comment

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

(This file should be in-sync with crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.pyi, the edits here are just copying over crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.pyi, I recommend ignoring this file and its snapshot changes.)

10 changes: 10 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI016.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,13 @@ field22: typing.Union[int, typing.Union[int, typing.Union[int, int]]] # Error
# Should emit in cases with newlines
field23: set[ # foo
int] | set[int]

# Should emit twice (once for each `int` in the nested union, both of which are
# duplicates of the outer `int`), but not three times (which would indicate that
# we incorrectly re-checked the nested union).
field24: typing.Union[int, typing.Union[int, int]] # PYI016: Duplicate union member `int`
Copy link
Member Author

Choose a reason for hiding this comment

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

Prior to #6345, we emitted three times here (incorrectly). See: #6285.


# Should emit twice (once for each `int` in the nested union, both of which are
# duplicates of the outer `int`), but not three times (which would indicate that
# we incorrectly re-checked the nested union).
field25: typing.Union[int, int | int] # PYI016: Duplicate union member `int`
Copy link
Member Author

Choose a reason for hiding this comment

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

Prior to adding in_nested_union in this PR, we emitted duplicate violations here.

50 changes: 18 additions & 32 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::RedundantLiteralUnion,
Rule::UnnecessaryTypeUnion,
]) {
// Avoid duplicate checks if the parent is an `Union[...]` since these rules
// Avoid duplicate checks if the parent is a union, since these rules already
// traverse nested unions.
let is_unchecked_union = checker
.semantic
.current_expression_grandparent()
.and_then(Expr::as_subscript_expr)
.map_or(true, |parent| {
!checker.semantic.match_typing_expr(&parent.value, "Union")
});

if is_unchecked_union {
if !checker.semantic.in_nested_union() {
if checker.enabled(Rule::UnnecessaryLiteralUnion) {
flake8_pyi::rules::unnecessary_literal_union(checker, expr);
}
Expand Down Expand Up @@ -1084,29 +1076,23 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}

// Avoid duplicate checks if the parent is an `|` since these rules
// Avoid duplicate checks if the parent is a union, since these rules already
// traverse nested unions.
let is_unchecked_union = !matches!(
checker.semantic.current_expression_parent(),
Some(Expr::BinOp(ast::ExprBinOp {
op: Operator::BitOr,
..
}))
);
if checker.enabled(Rule::DuplicateUnionMember)
&& checker.semantic.in_type_definition()
&& is_unchecked_union
{
flake8_pyi::rules::duplicate_union_member(checker, expr);
}
if checker.enabled(Rule::UnnecessaryLiteralUnion) && is_unchecked_union {
flake8_pyi::rules::unnecessary_literal_union(checker, expr);
}
if checker.enabled(Rule::RedundantLiteralUnion) && is_unchecked_union {
flake8_pyi::rules::redundant_literal_union(checker, expr);
}
if checker.enabled(Rule::UnnecessaryTypeUnion) && is_unchecked_union {
flake8_pyi::rules::unnecessary_type_union(checker, expr);
if !checker.semantic.in_nested_union() {
if checker.enabled(Rule::DuplicateUnionMember)
&& checker.semantic.in_type_definition()
{
flake8_pyi::rules::duplicate_union_member(checker, expr);
}
if checker.enabled(Rule::UnnecessaryLiteralUnion) {
flake8_pyi::rules::unnecessary_literal_union(checker, expr);
}
if checker.enabled(Rule::RedundantLiteralUnion) {
flake8_pyi::rules::redundant_literal_union(checker, expr);
}
if checker.enabled(Rule::UnnecessaryTypeUnion) {
flake8_pyi::rules::unnecessary_type_union(checker, expr);
}
}
}
Expr::UnaryOp(ast::ExprUnaryOp {
Expand Down
Loading