[Bug]: FURB158 suggests change that can alter code semantics

dscrofts commented 8 months ago

Has your issue already been fixed?

[X] Have you checked to see if your issue still exists on the master branch? See the docs for instructions on how to setup a local build of Refurb.
[X] Have you looked at the open/closed issues to see if anyone has already reported your issue?

The Bug

The following code:

selected: int | None = 5

match selected:
    case int() as i:
        print(i)

Emits the following error:

$ refurb file.py
[FURB158]: Replace `int() as i` with `int(i)`

But it should not be emitting an error because in this instance the code semantics would change. int() as i is used to match against integer values and bind the matched value to the variable selected. Whereas int(i) is a function call that tries to convert the value of selected to an integer. If selected is None, it will raise a TypeError because None cannot be converted to an integer.

Version Info

Refurb: v2.0.0
Mypy: v1.9.0

Python Version

Python 3.12.2

Config File

# N/A

Extra Info

None

dosisod commented 8 months ago

This is not how the match statement works. int() as i checks if selected is an int, and if so, binds that value to i. int(i) is a shorthand for this. Quoting from PEP636:

For many builtin classes (see PEP 634 for the whole list), you can use a positional parameter as a shorthand, writing str(c) rather than str() as c.

The below code example shows that int(i) does not raise a type error, and has the same semantics as int() as i:

def f(selected: int | None):
    match selected:
        case int() as i:
            print(i)

def f2(selected: int | None):
    match selected:
        case int(i):
            print(i)

f(1)
f(None)
f2(1)
f2(None)

Prints:

1
1

dscrofts commented 8 months ago

@dosisod thank you for the clarification. I was under the impression that the shorthand code only worked for builtins and not optional types, but your code proves otherwise.

dosisod / refurb