Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-simplify] Extend open-file-with-context-handler to work with other standard-library IO modules (SIM115) #12959

Merged
merged 7 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@
import pathlib as pl
from pathlib import Path
from pathlib import Path as P
import tempfile
diceroll123 marked this conversation as resolved.
Show resolved Hide resolved

# SIM115
f = open("foo.txt")
f = Path("foo.txt").open()
f = pathlib.Path("foo.txt").open()
f = pl.Path("foo.txt").open()
f = P("foo.txt").open()
f = tempfile.NamedTemporaryFile()
f = tempfile.TemporaryFile()
f = tempfile.SpooledTemporaryFile()
data = f.read()
f.close()

Expand Down Expand Up @@ -43,10 +47,14 @@
exit_stack_ = exit_stack
f = exit_stack_.enter_context(open("filename"))

# OK
with tempfile.TemporaryFile() as f:
data = f.read()

# OK (quick one-liner to clear file contents)
open("filename", "w").close()
pathlib.Path("filename").open("w").close()

tempfile.NamedTemporaryFile().close()

# OK (custom context manager)
class MyFile:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,32 +112,21 @@ fn match_exit_stack(semantic: &SemanticModel) -> bool {
false
}

/// Return `true` if `func` is the builtin `open` or `pathlib.Path(...).open`.
/// Return `true` if the expression is an `open` call or temporary file constructor.
fn is_open(semantic: &SemanticModel, func: &Expr) -> bool {
// Ex) `open(...)`
if semantic.match_builtin_expr(func, "open") {
return true;
}

// Ex) `pathlib.Path(...).open()`
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
return false;
};

if attr != "open" {
return false;
}

let Expr::Call(ast::ExprCall {
func: value_func, ..
}) = &**value
else {
let Some(qualified_name) = semantic.resolve_qualified_name(func) else {
return false;
};
diceroll123 marked this conversation as resolved.
Show resolved Hide resolved

semantic
.resolve_qualified_name(value_func)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"]))
matches!(
qualified_name.segments(),
["" | "builtins", "open"]
| ["pathlib", "Path", "open"]
| [
"tempfile",
"TemporaryFile" | "NamedTemporaryFile" | "SpooledTemporaryFile"
]
)
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
}

/// Return `true` if the current expression is followed by a `close` call.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,63 +1,51 @@
---
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
---
SIM115.py:8:5: SIM115 Use context handler for opening files
|
7 | # SIM115
8 | f = open("foo.txt")
| ^^^^ SIM115
9 | f = Path("foo.txt").open()
10 | f = pathlib.Path("foo.txt").open()
|

SIM115.py:9:5: SIM115 Use context handler for opening files
|
7 | # SIM115
8 | f = open("foo.txt")
9 | f = Path("foo.txt").open()
| ^^^^^^^^^^^^^^^^^^^^ SIM115
10 | f = pathlib.Path("foo.txt").open()
11 | f = pl.Path("foo.txt").open()
8 | # SIM115
9 | f = open("foo.txt")
| ^^^^ SIM115
10 | f = Path("foo.txt").open()
11 | f = pathlib.Path("foo.txt").open()
|

SIM115.py:10:5: SIM115 Use context handler for opening files
SIM115.py:14:5: SIM115 Use context handler for opening files
|
8 | f = open("foo.txt")
9 | f = Path("foo.txt").open()
10 | f = pathlib.Path("foo.txt").open()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
11 | f = pl.Path("foo.txt").open()
12 | f = P("foo.txt").open()
12 | f = pl.Path("foo.txt").open()
13 | f = P("foo.txt").open()
14 | f = tempfile.NamedTemporaryFile()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
15 | f = tempfile.TemporaryFile()
16 | f = tempfile.SpooledTemporaryFile()
|

SIM115.py:11:5: SIM115 Use context handler for opening files
SIM115.py:15:5: SIM115 Use context handler for opening files
|
9 | f = Path("foo.txt").open()
10 | f = pathlib.Path("foo.txt").open()
11 | f = pl.Path("foo.txt").open()
| ^^^^^^^^^^^^^^^^^^^^^^^ SIM115
12 | f = P("foo.txt").open()
13 | data = f.read()
13 | f = P("foo.txt").open()
14 | f = tempfile.NamedTemporaryFile()
15 | f = tempfile.TemporaryFile()
| ^^^^^^^^^^^^^^^^^^^^^^ SIM115
16 | f = tempfile.SpooledTemporaryFile()
17 | data = f.read()
|

SIM115.py:12:5: SIM115 Use context handler for opening files
SIM115.py:16:5: SIM115 Use context handler for opening files
|
10 | f = pathlib.Path("foo.txt").open()
11 | f = pl.Path("foo.txt").open()
12 | f = P("foo.txt").open()
| ^^^^^^^^^^^^^^^^^ SIM115
13 | data = f.read()
14 | f.close()
14 | f = tempfile.NamedTemporaryFile()
15 | f = tempfile.TemporaryFile()
16 | f = tempfile.SpooledTemporaryFile()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
17 | data = f.read()
18 | f.close()
|

SIM115.py:39:9: SIM115 Use context handler for opening files
SIM115.py:43:9: SIM115 Use context handler for opening files
|
37 | # SIM115
38 | with contextlib.ExitStack():
39 | f = open("filename")
41 | # SIM115
42 | with contextlib.ExitStack():
43 | f = open("filename")
| ^^^^ SIM115
40 |
41 | # OK
44 |
45 | # OK
|


Loading