Skip to content

Commit

Permalink
implement write-whole-file
Browse files Browse the repository at this point in the history
  • Loading branch information
augustelalande committed Apr 6, 2024
1 parent 1b31d4e commit d0cd4ae
Show file tree
Hide file tree
Showing 9 changed files with 583 additions and 4 deletions.
123 changes: 123 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB103.py
@@ -0,0 +1,123 @@
def foo():
...


def bar(x):
...


# Errors.

# FURB103
with open("file.txt", "w") as f:
f.write("test")

# FURB103
with open("file.txt", "wb") as f:
f.write(foobar)

# FURB103
with open("file.txt", mode="wb") as f:
f.write(b"abc")

# FURB103
with open("file.txt", "w", encoding="utf8") as f:
f.write(foobar)

# FURB103
with open("file.txt", "w", errors="ignore") as f:
f.write(foobar)

# FURB103
with open("file.txt", mode="w") as f:
f.write(foobar)

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

# FURB103
with open("a.txt", "w") as a, open("b.txt", "wb") as b:
a.write(x)
b.write(y)

# FURB103
with foo() as a, open("file.txt", "w") as b, foo() as c:
# We have other things in here, multiple with items, but the user
# writes a single time to file and that bit they can replace.
bar(a)
b.write(bar(bar(a + x)))
bar(c)


# FURB103
with open("file.txt", "w", newline="\r\n") as f:
f.write(foobar)


# Non-errors.

with open("file.txt", errors="ignore", mode="wb") as f:
# Path.write_bytes() does not support errors
f.write(foobar)

f2 = open("file2.txt", "w")
with open("file.txt", "w") as f:
f2.write(x)

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

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

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

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

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

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

# This shouldn't error, since it could contain unsupported arguments, like `buffering`.
with open(*filename, mode="w") as f:
f.write(x)

# This shouldn't error, since it could contain unsupported arguments, like `buffering`.
with open(**kwargs) as f:
f.write(x)

# This shouldn't error, since it could contain unsupported arguments, like `buffering`.
with open("file.txt", **kwargs) as f:
f.write(x)

# This shouldn't error, since it could contain unsupported arguments, like `buffering`.
with open("file.txt", mode="w", **kwargs) as f:
f.write(x)

# This could error (but doesn't), since it can't contain unsupported arguments, like
# `buffering`.
with open(*filename, mode="w") as f:
f.write(x)

# This could error (but doesn't), since it can't contain unsupported arguments, like
# `buffering`.
with open(*filename, file="file.txt", mode="w") as f:
f.write(x)
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Expand Up @@ -1217,6 +1217,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ReadWholeFile) {
refurb::rules::read_whole_file(checker, with_stmt);
}
if checker.enabled(Rule::WriteWholeFile) {
refurb::rules::write_whole_file(checker, with_stmt);
}
if checker.enabled(Rule::UselessWithLock) {
pylint::rules::useless_with_lock(checker, with_stmt);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Expand Up @@ -1035,6 +1035,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {

// refurb
(Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile),
(Refurb, "103") => (RuleGroup::Preview, rules::refurb::rules::WriteWholeFile),
(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
Expand Up @@ -40,6 +40,7 @@ mod tests {
#[test_case(Rule::MetaClassABCMeta, Path::new("FURB180.py"))]
#[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))]
#[test_case(Rule::ListReverseCopy, Path::new("FURB187.py"))]
#[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Expand Up @@ -24,6 +24,7 @@ pub(crate) use type_none_comparison::*;
pub(crate) use unnecessary_enumerate::*;
pub(crate) use unnecessary_from_float::*;
pub(crate) use verbose_decimal_constructor::*;
pub(crate) use write_whole_file::*;

mod bit_count;
mod check_and_remove_from_set;
Expand Down Expand Up @@ -51,3 +52,4 @@ mod type_none_comparison;
mod unnecessary_enumerate;
mod unnecessary_from_float;
mod verbose_decimal_constructor;
mod write_whole_file;

0 comments on commit d0cd4ae

Please sign in to comment.