Add "is file" pathlib check (#147)

dosisod · Dec 9, 2022 · 22994ae · 22994ae
1 parent cc8e98d
commit 22994ae
Show file tree

Hide file tree

Showing 4 changed files with 148 additions and 16 deletions.
diff --git a/docs/check-ideas.md b/docs/check-ideas.md
@@ -19,20 +19,6 @@ Good:
 file = Path("folder") / "filename"
 ```
 
-### Dont use `os.remove`, use `Path.unlink()`
-
-Bad:
-
-```python
-os.remove("file.txt")
-```
-
-Good:
-
-```python
-Path("file.txt").unlink()
-```
-
 ## Dict
 
 ### Replace `{x[0]: x[1] for x in y}` with `dict(y)`
@@ -153,8 +139,6 @@ d[x].append(y)
 
 ## List
 
-* Dont use `x[:]`, use `x.copy()`
-
 ## Built-in methods
 
 See https://docs.python.org/3/library/stdtypes.html

diff --git a/refurb/checks/pathlib/is_file.py b/refurb/checks/pathlib/is_file.py
@@ -0,0 +1,72 @@
+from dataclasses import dataclass
+
+from mypy.nodes import BytesExpr, CallExpr, MemberExpr, NameExpr, StrExpr, Var
+
+from refurb.checks.pathlib.util import is_pathlike
+from refurb.error import Error
+
+
+@dataclass
+class ErrorInfo(Error):
+    """
+    Don't use the `os.path.isfile` (or similar) functions, use the more modern
+    `pathlib` module instead:
+
+    Bad:
+
+    ```
+    if os.path.isfile("file.txt"):
+        pass
+    ```
+
+    Good:
+
+    ```
+    if Path("file.txt").is_file():
+        pass
+    ```
+    """
+
+    code = 146
+    categories = ["pathlib"]
+
+
+PATH_TO_PATHLIB_NAMES = {
+    "posixpath.isabs": "is_absolute",
+    "genericpath.isdir": "is_dir",
+    "genericpath.isfile": "is_file",
+    "posixpath.islink": "is_symlink",
+}
+
+
+def check(node: CallExpr, errors: list[Error]) -> None:
+    match node:
+        case CallExpr(
+            callee=MemberExpr(fullname=fullname, name=name),
+            args=[arg],
+        ) if new_name := PATH_TO_PATHLIB_NAMES.get(fullname or ""):
+            if is_pathlike(arg):
+                replace = f"x.{new_name}()"
+
+            else:
+                match arg:
+                    case BytesExpr() | StrExpr():
+                        pass
+
+                    case NameExpr(node=Var(type=ty)) if (
+                        str(ty) in ("builtins.str", "builtins.bytes")
+                    ):
+                        pass
+
+                    case _:
+                        return
+
+                replace = f"Path(x).{new_name}()"
+
+            errors.append(
+                ErrorInfo(
+                    node.line,
+                    node.column,
+                    f"Replace `os.path.{name}(x)` with `{replace}`",
+                )
+            )
diff --git a/test/data/err_146.py b/test/data/err_146.py
@@ -0,0 +1,56 @@
+import os
+from pathlib import Path
+
+file = Path("filename")
+filename = "filename"
+filename2 = b"filename"
+
+# these should match
+
+os.path.isabs("filename")
+os.path.isdir("filename")
+os.path.isfile("filename")
+os.path.islink("filename")
+
+os.path.isabs(file)
+os.path.isdir(file)
+os.path.isfile(file)
+os.path.islink(file)
+
+os.path.isabs(b"filename")
+os.path.isdir(b"filename")
+os.path.isfile(b"filename")
+os.path.islink(b"filename")
+
+os.path.isabs(filename)
+os.path.isdir(filename)
+os.path.isfile(filename)
+os.path.islink(filename)
+
+os.path.isabs(filename2)
+os.path.isdir(filename2)
+os.path.isfile(filename2)
+os.path.islink(filename2)
+
+
+# these should not
+
+os.path.ismount("somefile")
+
+file.is_absolute()
+file.is_dir()
+file.is_file()
+file.is_symlink()
+file.is_mount()
+
+os.path.isdir(1)
+os.path.isfile(1)
+os.path.islink(1)
+os.path.ismount(1)
+
+fd = 1
+
+os.path.isdir(fd)
+os.path.isfile(fd)
+os.path.islink(fd)
+os.path.ismount(fd)
diff --git a/test/data/err_146.txt b/test/data/err_146.txt
@@ -0,0 +1,20 @@
+test/data/err_146.py:10:1 [FURB146]: Replace `os.path.isabs(x)` with `Path(x).is_absolute()`
+test/data/err_146.py:11:1 [FURB146]: Replace `os.path.isdir(x)` with `Path(x).is_dir()`
+test/data/err_146.py:12:1 [FURB146]: Replace `os.path.isfile(x)` with `Path(x).is_file()`
+test/data/err_146.py:13:1 [FURB146]: Replace `os.path.islink(x)` with `Path(x).is_symlink()`
+test/data/err_146.py:15:1 [FURB146]: Replace `os.path.isabs(x)` with `x.is_absolute()`
+test/data/err_146.py:16:1 [FURB146]: Replace `os.path.isdir(x)` with `x.is_dir()`
+test/data/err_146.py:17:1 [FURB146]: Replace `os.path.isfile(x)` with `x.is_file()`
+test/data/err_146.py:18:1 [FURB146]: Replace `os.path.islink(x)` with `x.is_symlink()`
+test/data/err_146.py:20:1 [FURB146]: Replace `os.path.isabs(x)` with `Path(x).is_absolute()`
+test/data/err_146.py:21:1 [FURB146]: Replace `os.path.isdir(x)` with `Path(x).is_dir()`
+test/data/err_146.py:22:1 [FURB146]: Replace `os.path.isfile(x)` with `Path(x).is_file()`
+test/data/err_146.py:23:1 [FURB146]: Replace `os.path.islink(x)` with `Path(x).is_symlink()`
+test/data/err_146.py:25:1 [FURB146]: Replace `os.path.isabs(x)` with `Path(x).is_absolute()`
+test/data/err_146.py:26:1 [FURB146]: Replace `os.path.isdir(x)` with `Path(x).is_dir()`
+test/data/err_146.py:27:1 [FURB146]: Replace `os.path.isfile(x)` with `Path(x).is_file()`
+test/data/err_146.py:28:1 [FURB146]: Replace `os.path.islink(x)` with `Path(x).is_symlink()`
+test/data/err_146.py:30:1 [FURB146]: Replace `os.path.isabs(x)` with `Path(x).is_absolute()`
+test/data/err_146.py:31:1 [FURB146]: Replace `os.path.isdir(x)` with `Path(x).is_dir()`
+test/data/err_146.py:32:1 [FURB146]: Replace `os.path.isfile(x)` with `Path(x).is_file()`
+test/data/err_146.py:33:1 [FURB146]: Replace `os.path.islink(x)` with `Path(x).is_symlink()`