Skip to content

Commit

Permalink
[refurb] Do not allow any keyword arguments for read-whole-file i…
Browse files Browse the repository at this point in the history
…n `rb` mode (`FURB101`) (#10803)

## Summary

`Path.read_bytes()` does not support any keyword arguments, so `FURB101`
should not be triggered if the file is opened in `rb` mode with any
keyword arguments.

## Test Plan

Move erroneous test to "Non-error" section of fixture.
  • Loading branch information
augustelalande committed Apr 6, 2024
1 parent 44459f9 commit 7fb012d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 36 deletions.
9 changes: 5 additions & 4 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ def bar(x):
with open("file.txt", errors="ignore") as f:
x = f.read()

# FURB101
with open("file.txt", errors="ignore", mode="rb") as f:
x = f.read()

# FURB101
with open("file.txt", mode="r") as f: # noqa: FURB120
x = f.read()
Expand Down Expand Up @@ -60,6 +56,11 @@ def bar(x):

# Non-errors.

# Path.read_bytes does not support any kwargs
with open("file.txt", errors="ignore", mode="rb") as f:
x = f.read()


f2 = open("file2.txt")
with open("file.txt") as f:
x = f2.read()
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/read_whole_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ fn find_file_open<'a>(
// keyword mode should override that.
let mode = kw_mode.unwrap_or(pos_mode);

if matches!(mode, ReadMode::Bytes) && !keywords.is_empty() {
return None;
}

// Now we need to find what is this variable bound to...
let scope = semantic.current_scope();
let bindings: Vec<BindingId> = scope.get_all(var.id.as_str()).collect();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,56 +41,46 @@ FURB101.py:28:6: FURB101 `open` and `read` should be replaced by `Path("file.txt
29 | x = f.read()
|

FURB101.py:32:6: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_bytes(errors="ignore")`
FURB101.py:32:6: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()`
|
31 | # FURB101
32 | with open("file.txt", errors="ignore", mode="rb") as f:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB101
32 | with open("file.txt", mode="r") as f: # noqa: FURB120
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB101
33 | x = f.read()
|

FURB101.py:36:6: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()`
FURB101.py:36:6: FURB101 `open` and `read` should be replaced by `Path(foo()).read_bytes()`
|
35 | # FURB101
36 | with open("file.txt", mode="r") as f: # noqa: FURB120
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB101
37 | x = f.read()
|

FURB101.py:40:6: FURB101 `open` and `read` should be replaced by `Path(foo()).read_bytes()`
|
39 | # FURB101
40 | with open(foo(), "rb") as f:
36 | with open(foo(), "rb") as f:
| ^^^^^^^^^^^^^^^^^^^^^^ FURB101
41 | # The body of `with` is non-trivial, but the recommendation holds.
42 | bar("pre")
37 | # The body of `with` is non-trivial, but the recommendation holds.
38 | bar("pre")
|

FURB101.py:48:6: FURB101 `open` and `read` should be replaced by `Path("a.txt").read_text()`
FURB101.py:44:6: FURB101 `open` and `read` should be replaced by `Path("a.txt").read_text()`
|
47 | # FURB101
48 | with open("a.txt") as a, open("b.txt", "rb") as b:
43 | # FURB101
44 | with open("a.txt") as a, open("b.txt", "rb") as b:
| ^^^^^^^^^^^^^^^^^^ FURB101
49 | x = a.read()
50 | y = b.read()
45 | x = a.read()
46 | y = b.read()
|

FURB101.py:48:26: FURB101 `open` and `read` should be replaced by `Path("b.txt").read_bytes()`
FURB101.py:44:26: FURB101 `open` and `read` should be replaced by `Path("b.txt").read_bytes()`
|
47 | # FURB101
48 | with open("a.txt") as a, open("b.txt", "rb") as b:
43 | # FURB101
44 | with open("a.txt") as a, open("b.txt", "rb") as b:
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB101
49 | x = a.read()
50 | y = b.read()
45 | x = a.read()
46 | y = b.read()
|

FURB101.py:53:18: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()`
FURB101.py:49:18: FURB101 `open` and `read` should be replaced by `Path("file.txt").read_text()`
|
52 | # FURB101
53 | with foo() as a, open("file.txt") as b, foo() as c:
48 | # FURB101
49 | with foo() as a, open("file.txt") as b, foo() as c:
| ^^^^^^^^^^^^^^^^^^^^^ FURB101
54 | # We have other things in here, multiple with items, but
55 | # the user reads the whole file and that bit they can replace.
50 | # We have other things in here, multiple with items, but
51 | # the user reads the whole file and that bit they can replace.
|


0 comments on commit 7fb012d

Please sign in to comment.