Implement PEP 604 rewrites for isinstance and issubclass checks

[michaeloliverx]

PEP 604 introduced simplified syntax for union types e.g. var: str | int for which we already have support for in #369. It also allows this syntax to be used inside isinstance and issubclass checks, it would be great if these could be autofixed too!

# before
isinstance("", (int, str))
# after
isinstance("", int | str)


# before
issubclass(bool, (int, float))
# after
issubclass(bool, int | float)

[bluetech]

Not that it affects the usefulness of such a rule, but note that | is slower:

$ python -m timeit 'isinstance("", (int, str))'
2000000 loops, best of 5: 103 nsec per loop

$ python -m timeit 'isinstance("", int | str)'
1000000 loops, best of 5: 217 nsec per loop

This is on Python 3.10, might be faster on newer versions.

[michaeloliverx]

Not that it affects the usefulness of such a rule, but note that | is slower:

$ python -m timeit 'isinstance("", (int, str))'
2000000 loops, best of 5: 103 nsec per loop

$ python -m timeit 'isinstance("", int | str)'
1000000 loops, best of 5: 217 nsec per loop

This is on Python 3.10, might be faster on newer versions.

Interesting! Definitely worth putting a note beside the rule.

[wbolster]

python 3.11 has optimizations related to exactly this; see python/cpython#91603

on python 3.10:

$ a='isinstance("", (int, str))'
$ b='isinstance("", int | str)'
$ python3.10 -m timeit "$a"; python3.10 -m timeit "$b"
5000000 loops, best of 5: 48.6 nsec per loop
2000000 loops, best of 5: 108 nsec per loop

python 3.11 is significantly faster in both cases, and the difference between the two approaches has shrunk:

$ python3.11 -m timeit "$a"; python3.11 -m timeit "$b"
5000000 loops, best of 5: 40.6 nsec per loop
5000000 loops, best of 5: 63.2 nsec per loop

note that isinstance() checks (which in simple cases boil down to a pointer comparison in cpython) are very unlikely to dominate any real-world workloads.

[martinlehoux]

Can I take a look at this one?

[charliermarsh]

Sure! We can probably make it part of UP007.

[Skylion007]

@charliermarsh I'd be in favor of separating it out into it's on rule code as one could imagine stylistically a user preferring this syntax, but not like replacing Union with | and vice versa.

[charliermarsh]

@Skylion007 - I'm a little torn because that rule is intended to capture PEP 604 adoption / usage, and this behavior is explicitly part of PEP 604.

[martinlehoux]

It's been a while since I contributed last, so I don't remember everything. How would this interact with SIM101 rewriting isinstance(a, int) or isinstance(a, float) to isinstance(a, (int, float))?

[charliermarsh]

It's okay to "ignore" that, since if both rules are enabled, we'd end up correcting isinstance(a, (int, float)) to isinstance(a, int | float) in a second pass (we fix iteratively until there are no more fixable rules).

[martinlehoux]

I have a first draft, but not sure about code architecture: it doesn't to fit in the same function / ast level

[charliermarsh]

This was added in #3280.