Skip to content
Open
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
137 changes: 137 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI041_3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
from typing import (
TYPE_CHECKING,
Union,
)

from typing_extensions import (
TypeAlias,
)

TA0: TypeAlias = "int"
TA1: TypeAlias = "int | float | bool"
TA2: TypeAlias = "Union[int, float, bool]"


def good1(arg: "int") -> "int | bool":
...
Comment on lines +15 to +16
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test for a type annotation consisting of a concatenated string literal:

Suggested change
def good1(arg: "int") -> "int | bool":
...
def good1(arg: "int") -> "int" "| bool":
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! While detection works, the fix did indeed drop the quotation marks.

Copy link
Contributor Author

@robsdedude robsdedude Jul 7, 2025

Choose a reason for hiding this comment

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

This is non-trivial to fix. I'll have to go on a side-mission fixing #19184 first.

My approach was to alter parse_simple_type_annotation (just like parse_complex_type_annotation already does) such that the resulting resolved virtual expression spans that whole string. That way, the rule can just replace the whole string and wrap it in quotes if the original annotation was a forward reference.

But alas, that change to parse_simple_type_annotation amplifies the linked bug to also affect non-concatenated forward refs.

Copy link
Member

@MichaReiser MichaReiser Jul 7, 2025

Choose a reason for hiding this comment

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

I believe this was part of the reason why we decided not to support string annotations in the initial implementation as they aren't as common anymore and we didn't felt like they're worth all the complexity they add.

Could we disable the fix for problematic stringified annotations (I think you can also use triple quoted strings or put them across multiple lines). We should try to add tests for all of those if we decide that supporting them is worth the complexity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tripple quoted type annotations actually work fine. There are some in the test set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up addressing this in a similar way as RUF013 handles is:

  • analysis on stringized annotations: will do
  • suggesting fixes on stringized annotations: only if simple sting (i.e., non-concatendated) and always mark as unsafe.



def good2(arg: "int", arg2: "int | bool") -> "None":
...


def f0(arg1: "float | int") -> "None":
...


def f1(arg1: "float", *, arg2: "float | list[str] | type[bool] | complex") -> "None":
...


def f2(arg1: "int", /, arg2: "int | int | float") -> "None":
...


def f3(arg1: "int", *args: "Union[int | int | float]") -> "None":
...


async def f4(**kwargs: "int | int | float") -> "None":
...


def f5(arg1: "int", *args: "Union[int, int, float]") -> "None":
...


def f6(arg1: "int", *args: "Union[Union[int, int, float]]") -> "None":
...


def f7(arg1: "int", *args: "Union[Union[Union[int, int, float]]]") -> "None":
...


def f8(arg1: "int", *args: "Union[Union[Union[int | int | float]]]") -> "None":
...


def f9(
arg: """Union[ # comment
float, # another
complex, int]"""
) -> "None":
...

def f10(
arg: """
int | # comment
float | # another
complex
"""
) -> "None":
...


class Foo:
def good(self, arg: "int") -> "None":
...

def bad(self, arg: "int | float | complex") -> "None":
...

def bad2(self, arg: "int | Union[float, complex]") -> "None":
...

def bad3(self, arg: "Union[Union[float, complex], int]") -> "None":
...

def bad4(self, arg: "Union[float | complex, int]") -> "None":
...

def bad5(self, arg: "int | (float | complex)") -> "None":
...


# https://github.com/astral-sh/ruff/issues/18298
# fix must not yield runtime `None | None | ...` (TypeError)
class Issue18298:
def f1(self, arg: "None | int | None | float" = None) -> "None": # PYI041 - no fix
pass

if TYPE_CHECKING:

def f2(self, arg: "None | int | None | float" = None) -> "None": ... # PYI041 - with fix

else:

def f2(self, arg=None) -> "None":
pass

def f3(self, arg: "None | float | None | int | None" = None) -> "None": # PYI041 - with fix
pass


class FooStringConcat:
def good(self, arg: "i" "nt") -> "None":
...

def bad(self, arg: "int " "| float | com" "plex") -> "None":
...

def bad2(self, arg: "int | Union[flo" "at, complex]") -> "None":
...

def bad3(self, arg: "Union[Union[float, com" "plex], int]") -> "None":
...

def bad4(self, arg: "Union[float | complex, in" "t ]") -> "None":
...

def bad5(self, arg: "int | "
"(float | complex)") -> "None":
...

def bad6(self, arg: "in\
t | (float | compl" "ex)") -> "None":
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from typing import Union as Uno


def f1(a: "U" "no[int, fl" "oat, Foo]") -> "None": ...
def f2(a: "Uno[int, float, Foo]") -> "None": ...
def f3(a: """Uno[int, float, Foo]""") -> "None": ...
def f4(a: "Uno[in\
t, float, Foo]") -> "None": ...
17 changes: 17 additions & 0 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,23 @@ impl<'a> Checker<'a> {
}
}

/// Given a type annotation [`Expr`], abstracting over the fact that the annotation expression
/// might be "stringized".
///
/// A stringized annotation is one enclosed in string quotes:
/// `foo: "typing.Any"` means the same thing to a type checker as `foo: typing.Any`.
pub(crate) fn map_maybe_stringized_annotation<'b>(&self, expr: &'b ast::Expr) -> &'b ast::Expr
where
'a: 'b,
{
if let ast::Expr::StringLiteral(string_annotation) = expr {
if let Ok(parsed_annotation) = self.parse_type_annotation(string_annotation) {
return parsed_annotation.expression();
}
}
expr
}

/// Push `diagnostic` if the checker is not in a `@no_type_check` context.
pub(crate) fn report_type_diagnostic<T: Violation>(&self, kind: T, range: TextRange) {
if !self.semantic.in_no_type_check() {
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ mod tests {
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041_1.py"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041_1.pyi"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041_2.py"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041_3.py"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041_4.py"))]
#[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.py"))]
#[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.pyi"))]
#[test_case(Rule::StrOrReprDefinedInStub, Path::new("PYI029.py"))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,15 @@ impl Violation for RedundantNumericUnion {
/// PYI041
pub(crate) fn redundant_numeric_union(checker: &Checker, parameters: &Parameters) {
for annotation in parameters.iter().filter_map(AnyParameterRef::annotation) {
check_annotation(checker, annotation);
check_annotation(
checker,
checker.map_maybe_stringized_annotation(annotation),
annotation,
);
}
}

fn check_annotation<'a>(checker: &Checker, annotation: &'a Expr) {
fn check_annotation<'a>(checker: &Checker, annotation: &'a Expr, unresolved_annotation: &'a Expr) {
let mut numeric_flags = NumericFlags::empty();

let mut find_numeric_type = |expr: &Expr, _parent: &Expr| {
Expand Down Expand Up @@ -141,12 +145,20 @@ fn check_annotation<'a>(checker: &Checker, annotation: &'a Expr) {
return;
}

let string_annotation = unresolved_annotation.as_string_literal_expr();
if string_annotation.is_some_and(|s| s.value.is_implicit_concatenated()) {
// No fix for concatenated string literals. They're rare and too complex to handle.
// https://github.com/astral-sh/ruff/issues/19184#issuecomment-3047695205
return;
}

// Mark [`Fix`] as unsafe when comments are in range.
let applicability = if checker.comment_ranges().intersects(annotation.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
};
let applicability =
if string_annotation.is_some() || checker.comment_ranges().intersects(annotation.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
};

// Generate the flattened fix once.
let fix = if let &[edit_expr] = necessary_nodes.as_slice() {
Expand Down
Loading