UP035 autofix breaks code when a typing-module alias is used in a PEP-604 union in combination with a string

[PhilipVinc]

Description

The following code is correct

from typing import Iterable
a = Iterable['ciao'] | 'ciao'

However it gets tagged as wrong by UP35 and automatically fixed to

from collections.abc import Iterable
a = Iterable['ciao'] | 'ciao'

And the fixed code is wrong.

[AlexWaygood]

I'm marking this issue as a bug since it's true that the autofix changes behaviour here, and ideally we should fix that. FWIW, though, I'd recommend always quoting the entire union if you're using forward references in a PEP-604 union, e.g.

from collections.abc import Iterable
a = 'Iterable[ciao] | ciao'

You can get unpredictable results (like this) otherwise. This is the official advice given at https://docs.python.org/3/library/stdtypes.html#types-union

[Daverball]

The original code is wrong as well, see runtime-string-union for why.

However, since you're using an implicit type alias, ruff can't know that using | here is not allowed. If you want ruff to be able to detect TC010 here, then you need to switch to an explicit type alias: https://play.ruff.rs/9bd2d888-0d78-43af-bc0e-2e7b8da5d5dc

[AlexWaygood]

The original code is wrong as well

I'd say that the original code is "ill-advised" @Daverball, but it's true that it does work at runtime :-)

[Daverball]

You're right. I didn't know the aliases in the typing module had some special magic to convert strings to ForwardRef.

[AlexWaygood]

I didn't know the aliases in the typing module had some special magic to convert strings to ForwardRef.

Yeah... and it's arguably somewhat regrettable that they do, since it leads to highly confusing situationsn like this :/ but it can't realistically be reversed now

[PhilipVinc]

Ah, I was not aware that even my code was 'ill-advised' wrong!
Thanks for letting me know!

However my point stands...
I had some 'working' (albeit incorrect) code that ruff broke when I added UP rules.
I was not aware of TC rules.
I imagine you'd like to avoid this happening.

It might be a good idea to suggest turning on TC010 with UP035 and some other UP rules as well?

[PhilipVinc]

(Yeah, I acknowledge that the 'right' and 'wrong' adjectives I used in the original post was a bit strong. Sorry, I was just trying to open the issue quickly because I had some other things to attend to)

[AlexWaygood]

However my point stands...
I had some 'working' (albeit incorrect) code that ruff broke when I added UP rules.
I was not aware of TC rules.
I imagine you'd like to avoid this happening.

Yes, we agree that this is a bug in Ruff that should be fixed :-) The autofix for this rule isn't meant to change the behaviour of your code!

It might be a good idea to suggest turning on TC010 with UP035 and some other UP rules as well?

Hmm, not sure -- this is the first time I've seen this come up, so I don't think many people run into this issue.

[Daverball]

The easiest way to address this would probably be to mark the fix as unsafe if the parent node to any of the references is a union BinOp and either the node directly to the left or directly to the right is a string.

Eventually if #15222 or #15635 gets merged, we could reuse that fix to extend the quotes and make the fix safe again if it doesn't get rid of any comments.