Skip to content
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.

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

RUF038 Surprising fix for Literal[True, False] in functions with overloads #16129

Open
Geo5 opened this issue Feb 12, 2025 · 6 comments
Open
Labels
rule Implementing or modifying a lint rule

Comments

@Geo5
Copy link

Geo5 commented Feb 12, 2025

Description

I stumbled upon some code of mine where it seems to me that mypy and ruff do not agree whether Literal[True, False] == bool and rule RUF038 suggest a fix which introduces a mypy error.

Edit: I read the docs for the rule again and this general case seems to be mentioned in the "Fix safety" already. Maybe this example is different, but if you feel like it is already covered well enough, feel free to close the issue!


Consider this code containing two @overloads and illustrating perhaps two different problems:

import random
from typing import Literal, overload


class Foo:
    @overload
    def foo(self, *, bar: Literal[True]) -> int: ...

    @overload
    def foo(self, *, bar: Literal[False]) -> float: ...

    def foo(self, *, bar: Literal[True, False]) -> int | float:
        return 0


class FooSub(Foo):
    @overload
    def foo(self, *, bar: Literal[True]) -> int: ...

    @overload
    def foo(self, *, bar: Literal[False]) -> float: ...

    def foo(self, *, bar: Literal[True, False]) -> int | float:
        return super().foo(bar=bar)


def compute_bool() -> bool:
    return random.random() > 10


Foo().foo(bar=compute_bool())  # Problem 1: For this line mypy raises an error
$ mypy --strict test.py
test.py:31: error: No overload variant of "foo" of "Foo" matches argument type "bool"  [call-overload]
test.py:31: note: Possible overload variants:
test.py:31: note:     def foo(self, *, bar: Literal[True]) -> int
test.py:31: note:     def foo(self, *, bar: Literal[False]) -> float
Found 1 error in 1 file (checked 1 source file)

And ruff suggest fixing the annotations like this:

$ ruff check --isolated --select RUF --preview test.py
test.py:12:27: RUF038 `Literal[True, False]` can be replaced with `bool`
   |
10 |     def foo(self, *, bar: Literal[False]) -> float: ...
11 |
12 |     def foo(self, *, bar: Literal[True, False]) -> int | float:
   |                           ^^^^^^^^^^^^^^^^^^^^ RUF038
13 |         return 0
   |
   = help: Replace with `bool`

test.py:23:27: RUF038 `Literal[True, False]` can be replaced with `bool`
   |
21 |     def foo(self, *, bar: Literal[False]) -> float: ...
22 |
23 |     def foo(self, *, bar: Literal[True, False]) -> int | float:
   |                           ^^^^^^^^^^^^^^^^^^^^ RUF038
24 |         return super().foo(bar=bar)
   |
   = help: Replace with `bool`

Found 2 errors.
No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option).

Resulting in:

import random
from typing import Literal, overload


class Foo:
    @overload
    def foo(self, *, bar: Literal[True]) -> int: ...

    @overload
    def foo(self, *, bar: Literal[False]) -> float: ...

    def foo(self, *, bar: bool) -> int | float:
        return 0


class FooSub(Foo):
    @overload
    def foo(self, *, bar: Literal[True]) -> int: ...

    @overload
    def foo(self, *, bar: Literal[False]) -> float: ...

    def foo(self, *, bar: bool) -> int | float:
        return super().foo(bar=bar)  # Problem 2 introduced


def compute_bool() -> bool:
    return random.random() > 10


Foo().foo(bar=compute_bool()) # Problem 1 again, same as unfixed code

And again mypy on the fixed code:

$ mypy --strict test_fixed.py
test_fixed.py:24: error: Returning Any from function declared to return "int | float"  [no-any-return]
test_fixed.py:24: error: No overload variant of "foo" of "Foo" matches argument type "bool"  [call-overload]
test_fixed.py:24: note: Possible overload variants:
test_fixed.py:24: note:     def foo(self, *, bar: Literal[True]) -> int
test_fixed.py:24: note:     def foo(self, *, bar: Literal[False]) -> float
test_fixed.py:31: error: No overload variant of "foo" of "Foo" matches argument type "bool"  [call-overload]
test_fixed.py:31: note: Possible overload variants:
test_fixed.py:31: note:     def foo(self, *, bar: Literal[True]) -> int
test_fixed.py:31: note:     def foo(self, *, bar: Literal[False]) -> float
Found 3 errors in 1 file (checked 1 source file)

$ python --version
Python 3.11.10
$ ruff --version
ruff 0.9.6
$ mypy --version
mypy 1.15.0 (compiled: yes)
@Geo5
Copy link
Author

Geo5 commented Feb 12, 2025

Oh i am so sorry! I didn't read the doc for rule RUF038 close enough and did not look at the issues linked there! I think the issues for mypy and pyright might cover this?

@Geo5 Geo5 changed the title RUF038 Literal[True, False] might not always be equal to bool RUF038 Incorrect fix for Literal[True, False] in functions with overloads Feb 12, 2025
@Geo5 Geo5 changed the title RUF038 Incorrect fix for Literal[True, False] in functions with overloads RUF038 Surprising fix for Literal[True, False] in functions with overloads Feb 13, 2025
@MichaReiser
Copy link
Member

MichaReiser commented Feb 13, 2025

Yes, this part is covered by the section you call out. However, it does make me wonder if we should skip providing a fix alltogether if a method is marked as @overload. @AlexWaygood wdyt?

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Feb 13, 2025
@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 13, 2025

It's not totally clear to me whether mypy is correct in rejecting the overloads after they've been autofixed by Ruff here. Overloads aren't currently clearly specified in the typing spec, but there is a draft PR to provide a detailed specification for them, which I think is nearly over the line now. The rendered version of the draft spec is here, and it describes how type checkers should perform "argument type expansion" for overloads, which for bool means that it should implicitly convert bool into union Literal[True] | Literal[False]. Assuming that the draft overload chapter is accepted to the spec without major revisions, I think that might put mypy's behaviour here in violation of the new spec. @carljm can correct me if I'm wrong!

Whether mypy is right or wrong, though, is perhaps irrelevant here. We perhaps shouldn't be making changes that would cause your CI to fail, and here we clearly are, since mypy now emits more errors after Ruff's autofix than it did initially. So I'd be okay with ignoring any @overload-decorated function definitions (or any function definitions that shadow @overload-decorated function definitions) for this rule. I think that makes sense.

@carljm
Copy link
Contributor

carljm commented Feb 13, 2025

According to the draft spec for overload handling, mypy is in the wrong here by failing to expand bool to Literal[True, False] when checking a call with an argument typed as bool to a function with overloads taking Literal[True] and Literal[False].

Both the error in the initial code and the new error in the fixed code are examples of the exact same mypy limitation, the problem just occurs in a new location (inside the implementation of FooSub.bar) when the annotation on FooSub.bar is changed to bool.

If mypy were fixed to perform expansion of bool, neither version of the code would have any errors.

In general (even not considering overloads specifically) anywhere a type checker fails to treat bool and Literal[True, False] the same, it's a type checker bug IMO -- those types are equivalent. (Side note: our fix safety docs for this rule are simply wrong when they claim in the second bullet point that bool and Literal[True, False] are not strictly equivalent because bool is a subtype of int -- Literal[True, False] is also a subtype of int.)

The fix safety docs do already mention this exact problem in the fix safety section, and even link to relevant mypy bugs.

I think we could decline to autofix this in this particular overload case, as a concession to mypy, but I'm not sure how many real-world scenarios would be helped by that. An overload like this is quite likely already causing errors under mypy, because it's almost certain you'd be calling it somewhere with a value typed as bool (as is indeed the case in version 1 of the code here, before the autofix.) So personally my feeling is that calling it out in "fix safety" is sufficient concession to the mypy bug.

@AlexWaygood
Copy link
Member

(Side note: our fix safety docs for this rule are simply wrong when they claim in the second bullet point that bool and Literal[True, False] are not strictly equivalent because bool is a subtype of int -- Literal[True, False] is also a subtype of int.)

Yeah I also noticed that -- I agree, I'm not sure what those docs are trying to say there

@Geo5
Copy link
Author

Geo5 commented Feb 16, 2025

I think we could decline to autofix this in this particular overload case, as a concession to mypy, but I'm not sure how many real-world scenarios would be helped by that. An overload like this is quite likely already causing errors under mypy, because it's almost certain you'd be calling it somewhere with a value typed as bool (as is indeed the case in version 1 of the code here, before the autofix.) So personally my feeling is that calling it out in "fix safety" is sufficient concession to the mypy bug.

I agree with everything you said, thank you for the writeup!
Just for some additional context, i didn't hit the mypy bug without the fix, as i only ever called the overloaded function as foo(..., True) or foo(..., False), but i agree that this is probably rare.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants