[Bug]: python_version + multi-py-version library

kasium commented 1 month 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?
[X] If reporting a false positive/incorrect suggestion, have you double checked that the suggested fix changes the code semantics?

The Bug

Given a multi-py-version library (a library which e..g support python 3.8-3.12), using refurbs python-version can lead to issues. In my case, my documentation which is only generated with python 3.12 contains a custom sphinx plugin. The latest version of sphinx is only compatible with py 3.8 onwards but now refurb complains:

python-version is set to 3.8
the sphinx plugin is only executed with 3.12
the sphinx open source code contains syntax for py 3.9 only => error

Version Info

Refurb: v2.0.0
Mypy: v1.10.0

Python Version

3.12.0

Config File

# N/A

Extra Info

None

dosisod commented 5 days ago

Hi @kasium , thank you for opening this, and sorry for the massive delay in responding.

I can see how this might potentially be an issue, but I don't have enough to reproduce exactly what is going on. It sounds like there are three conflicting/interacting systems:

The python-version in Refurb (set to 3.8)
The sphinx package (requires 3.9+)
The sphinx plugin (requires 3.12+)

One of these is probably the culprit, though it is probably the python-version set at the Refurb level. I can't see what errors are being displayed, so I don't know if that's the case.

I'm not familiar with sphinx so I don't know how it is typically used, so it would help if you could describe your sphinx setup so I can better diagnose the error. Are you able to share the source code for this project? If not, can you provide a MVP of the issue?

Thanks!

kasium commented 5 days ago

Hi,

thanks for coming back to this issue 😄 . Let me give you some background on my use case:

my library supports python 3.8 till 3.13 (okay, now 3.8 is out of maintenance but lets just assume it isn't)
I need to se python_version to 3.8, else checks like FURB134 are reporting false positives
Some parts of the library will only be executed with python 3.13 (e.g. project internal utility). Please ignore the above mention of sphinx; let's just say I use the match statement which is not a valid syntax in python 3.8

Given this use case, there is no version I can set in refurb to support this scenario. On the one hand I get false positive errors, on the other I get a sytax error. The only real solution is, to ignore the false positives one by one for files and folders which support the older python versions and then to set python_version in refurb to 3.13

The config

enable_all = true
python_version = "3.8"  # or "3.13"
quiet = true

Python 3.8 compatible code py38.py

from functools import lru_cache

@lru_cache(maxsize=None)
def foo(): pass

python 3.13 compatible code py313.py

x = 1
match x:
    case 1:
        print("one")
    case 2:
        print("two")
    case _:
        print("other")

If you now use refurb py38.py py313.py you get these errors

with python_version=3.8

py313.py:10:24: error: Pattern matching is only supported in Python 3.10 and greater  [syntax]
            print("other")

with python_version=3.13

py38.py:6:2 [FURB134]: Replace `@lru_cache(maxsize=None)` with `@cache`

dosisod commented 5 days ago

Ah I see, your project supports Python 3.8+, but has internal parts which use the 3.13 syntax.

So the issue is that Refurb uses Mypy under the hood, and Mypy has a global --python-version flag for setting what Python version to use. There is no way to say "support 3.8+", or "support 3.8-3.13", it only supports using the syntax for a given version.

There are 2 ways of going about fixing this:

Allow for setting different python-versions for different parts of the codebase. For example, src uses 3.8, but test uses 3.13. This would require running Refurb/Mypy twice, once in 3.8 mode for src, and once in 3.13 mode for test.
Support a range of versions in python-version. With this method Refurb/Mypy would scan the code using the highest supported version (in this case, 3.13, to avoid syntax errors like you mentioned), but then in the checks themselves (for example, FURB134), we would use the lowest version, in this case, 3.8.

Both have their pros and cons. With option 1 you get a nice warning if you try to use Python 3.9+ features when you're expecting to support 3.8+, but with option 2 you don't need to scan the codebase multiple times (you could probably do this in parallel, but that's more of an implementation detail).

I think option 1 is the easiest, and probably covers your use case the best. What do you think?

kasium commented 5 days ago

Sounds like a plan. However, I think setting the python_version to the latest and just ignoring invalid findings is also a valid option

dosisod / refurb