dosisod / refurb

A tool for refurbishing and modernizing Python codebases
GNU General Public License v3.0
2.48k stars 54 forks source link

[Bug]: Missing FURB151 (`Path(x).touch()`) when separating open() and close() #326

Open opk12 opened 8 months ago

opk12 commented 8 months ago

Has your issue already been fixed?

The Bug

The following code:

#! /usr/bin/env python3
import os.path

path = "/tmp/tmpfile1"

if not os.path.exists(path):
    # Missing FURB151: No warning
    f = open(path, "w")
    f.close()

    # Warns as expected
    open(path, "w").close()

Does not emit the Path.touch() warning for lines 8-9. It only warns for line 12.

$ refurb --enable-all a.py 
a.py:6:8 [FURB141]: Replace `os.path.exists(x)` with `Path(x).exists()`
a.py:12:5 [FURB151]: Replace `open(x, "w").close()` with `Path(x).touch()`

Version Info

Refurb: v1.27.0
Mypy: v1.8.0

Python Version

Python 3.11.7

Config File

# N/A

Extra Info

None

dosisod commented 8 months ago

Thank you @opk12 for opening this! This would be a good feature to add.

Out of curiosity, is this example from code you've seen/wrote? I'm trying to find examples of it in the wild using grep.app but it doesn't seem like a very common idiom.

opk12 commented 8 months ago

(edited) Thank you for looking into it. I just wanted to throw an idea in the air, but I don't know if this is a common idiom in real-world code. I saw it in a student homework, which touches the file and then reads it.

def apri_file(nome):
    if not os.path.exists(nome):
        f = open(nome,"w")
        f.close()
    with open(nome, 'r') as route:
        for row in csv.reader(route, delimiter="\t"):
            ...

This example could be refactored to not touch:

def apri_file(nome):
    if os.path.exists(nome):
        with open(nome, 'r') as route:
            for row in csv.reader(route, delimiter="\t"):
                ...
    else:
        special case...

but as the warning is already implemented, I thought it just needs to be extended.