Skip to content

Commit

Permalink
[refurb] Implement read-whole-file [FURB101]
Browse files Browse the repository at this point in the history
  • Loading branch information
SavchenkoValeriy committed Sep 27, 2023
1 parent c8360a1 commit f848029
Show file tree
Hide file tree
Showing 8 changed files with 521 additions and 0 deletions.
105 changes: 105 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB101.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
def foo():
...


def bar(x):
...


# Errors.

# FURB101
with open("file.txt") as f:
x = f.read()

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

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

# FURB101
with open("file.txt", encoding="utf8") as f:
x = f.read()

# FURB101
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()

# FURB101
with open(foo(), "rb") as f:
# The body of `with` is non-trivial, but the recommendation holds.
bar("pre")
bar(f.read())
bar("post")
print("Done")

# FURB101
with open("a.txt") as a, open("b.txt", "rb") as b:
x = a.read()
y = b.read()

# FURB101
with foo() as a, open("file.txt") as b, foo() as c:
# We have other things in here, multiple with items, but
# the user reads the whole file and that bit they can replace.
bar(a)
bar(bar(a + b.read()))
bar(c)


# Non-errors.

f2 = open("file2.txt")
with open("file.txt") as f:
x = f2.read()

with open("file.txt") as f:
# Path.read_text() does not support size, so ignore this
x = f.read(100)

# mode is dynamic
with open("file.txt", foo()) as f:
x = f.read()

# keyword mode is incorrect
with open("file.txt", mode="a+") as f:
x = f.read()

# enables line buffering, not supported in read_text()
with open("file.txt", buffering=1) as f:
x = f.read()

# force CRLF, not supported in read_text()
with open("file.txt", newline="\r\n") as f:
x = f.read()

# dont mistake "newline" for "mode"
with open("file.txt", newline="b") as f:
x = f.read()

# I guess we can possibly also report this case, but the question
# is why the user would put "r+" here in the first place.
with open("file.txt", "r+") as f:
x = f.read()

# Even though we read the whole file, we do other things.
with open("file.txt") as f:
x = f.read()
f.seek(0)
x += f.read(100)

open = bar
# Not a real `open`.
with open("file.txt") as f:
x = f.read()
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1166,6 +1166,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::RedefinedLoopName) {
pylint::rules::redefined_loop_name(checker, stmt);
}
if checker.enabled(Rule::ReadWholeFile) {
refurb::rules::read_whole_file(checker, with_stmt);
}
}
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
if checker.enabled(Rule::FunctionUsesLoopVariable) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Slots, "002") => (RuleGroup::Unspecified, rules::flake8_slots::rules::NoSlotsInNamedtupleSubclass),

// refurb
(Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile),
(Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString),
#[allow(deprecated)]
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod tests {
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))]
#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
#[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
#[test_case(Rule::CheckAndRemoveFromSet, Path::new("FURB132.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub(crate) use check_and_remove_from_set::*;
pub(crate) use delete_full_slice::*;
pub(crate) use print_empty_string::*;
pub(crate) use read_whole_file::*;
pub(crate) use reimplemented_starmap::*;
pub(crate) use repeated_append::*;
pub(crate) use slice_copy::*;
Expand All @@ -9,6 +10,7 @@ pub(crate) use unnecessary_enumerate::*;
mod check_and_remove_from_set;
mod delete_full_slice;
mod print_empty_string;
mod read_whole_file;
mod reimplemented_starmap;
mod repeated_append;
mod slice_copy;
Expand Down
Loading

0 comments on commit f848029

Please sign in to comment.