Skip to content

Commit

Permalink
[ruff-0.7] Stabilise the expansion of `open-file-with-context-handler…
Browse files Browse the repository at this point in the history 
…` to work with other standard-library IO modules (`SIM115`) (#13680)

Closes #7313.
  • Loading branch information
@AlexWaygood
AlexWaygood authored Oct 8, 2024
1 parent 91161b4 commit e38914d
Show file tree
Hide file tree
Showing 4 changed files with 284 additions and 383 deletions.
1 change: 0 additions & 1 deletion crates/ruff_linter/src/rules/flake8_simplify/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ mod tests {
}

#[test_case(Rule::IfElseBlockInsteadOfIfExp, Path::new("SIM108.py"))]
#[test_case(Rule::OpenFileWithContextHandler, Path::new("SIM115.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
59 changes: 11 additions & 48 deletions crates/ruff_linter/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,9 @@ use crate::checkers::ast::Checker;
/// ## Why is this bad?
/// If a file is opened without a context manager, it is not guaranteed that
/// the file will be closed (e.g., if an exception is raised), which can cause
/// resource leaks.
///
/// ## Preview-mode behavior
/// If [preview] mode is enabled, this rule will detect a wide array of IO calls where
/// context managers could be used, such as `tempfile.TemporaryFile()` or
/// `tarfile.TarFile(...).gzopen()`. If preview mode is not enabled, only `open()`,
/// `builtins.open()` and `pathlib.Path(...).open()` are detected.
/// resource leaks. The rule detects a wide array of IO calls where context managers
/// could be used, such as `open`, `pathlib.Path(...).open()`, `tempfile.TemporaryFile()`
/// or`tarfile.TarFile(...).gzopen()`.
///
/// ## Example
/// ```python
Expand Down Expand Up @@ -118,36 +114,9 @@ fn match_exit_stack(semantic: &SemanticModel) -> bool {
false
}

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

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

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

let Expr::Call(ast::ExprCall {
func: value_func, ..
}) = &**value
else {
return false;
};

semantic
.resolve_qualified_name(value_func)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"]))
}

/// Return `true` if the expression is an `open` call or temporary file constructor.
fn is_open_preview(semantic: &SemanticModel, call: &ast::ExprCall) -> bool {
/// Return `true` if the expression is a call to `open()`,
/// or a call to some other standard-library function that opens a file.
fn is_open_call(semantic: &SemanticModel, call: &ast::ExprCall) -> bool {
let func = &*call.func;

// Ex) `open(...)`
Expand Down Expand Up @@ -203,8 +172,8 @@ fn is_open_preview(semantic: &SemanticModel, call: &ast::ExprCall) -> bool {
)
}

/// Return `true` if the current expression is followed by a `close` call.
fn is_closed(semantic: &SemanticModel) -> bool {
/// Return `true` if the current expression is immediately followed by a `.close()` call.
fn is_immediately_closed(semantic: &SemanticModel) -> bool {
let Some(expr) = semantic.current_expression_grandparent() else {
return false;
};
Expand All @@ -231,18 +200,12 @@ fn is_closed(semantic: &SemanticModel) -> bool {
pub(crate) fn open_file_with_context_handler(checker: &mut Checker, call: &ast::ExprCall) {
let semantic = checker.semantic();

if checker.settings.preview.is_disabled() {
if !is_open(semantic, call) {
return;
}
} else {
if !is_open_preview(semantic, call) {
return;
}
if !is_open_call(semantic, call) {
return;
}

// Ex) `open("foo.txt").close()`
if is_closed(semantic) {
if is_immediately_closed(semantic) {
return;
}

Expand Down
273 changes: 273 additions & 0 deletions ...ake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM115_SIM115.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,276 @@ SIM115.py:39:9: SIM115 Use a context manager for opening files
40 |
41 | # OK
|

SIM115.py:80:5: SIM115 Use a context manager for opening files
|
78 | import fileinput
79 |
80 | f = tempfile.NamedTemporaryFile()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
81 | f = tempfile.TemporaryFile()
82 | f = tempfile.SpooledTemporaryFile()
|

SIM115.py:81:5: SIM115 Use a context manager for opening files
|
80 | f = tempfile.NamedTemporaryFile()
81 | f = tempfile.TemporaryFile()
| ^^^^^^^^^^^^^^^^^^^^^^ SIM115
82 | f = tempfile.SpooledTemporaryFile()
83 | f = tarfile.open("foo.tar")
|

SIM115.py:82:5: SIM115 Use a context manager for opening files
|
80 | f = tempfile.NamedTemporaryFile()
81 | f = tempfile.TemporaryFile()
82 | f = tempfile.SpooledTemporaryFile()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
83 | f = tarfile.open("foo.tar")
84 | f = TarFile("foo.tar").open()
|

SIM115.py:83:5: SIM115 Use a context manager for opening files
|
81 | f = tempfile.TemporaryFile()
82 | f = tempfile.SpooledTemporaryFile()
83 | f = tarfile.open("foo.tar")
| ^^^^^^^^^^^^ SIM115
84 | f = TarFile("foo.tar").open()
85 | f = tarfile.TarFile("foo.tar").open()
|

SIM115.py:84:5: SIM115 Use a context manager for opening files
|
82 | f = tempfile.SpooledTemporaryFile()
83 | f = tarfile.open("foo.tar")
84 | f = TarFile("foo.tar").open()
| ^^^^^^^^^^^^^^^^^^^^^^^ SIM115
85 | f = tarfile.TarFile("foo.tar").open()
86 | f = tarfile.TarFile().open()
|

SIM115.py:85:5: SIM115 Use a context manager for opening files
|
83 | f = tarfile.open("foo.tar")
84 | f = TarFile("foo.tar").open()
85 | f = tarfile.TarFile("foo.tar").open()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
86 | f = tarfile.TarFile().open()
87 | f = zipfile.ZipFile("foo.zip").open("foo.txt")
|

SIM115.py:86:5: SIM115 Use a context manager for opening files
|
84 | f = TarFile("foo.tar").open()
85 | f = tarfile.TarFile("foo.tar").open()
86 | f = tarfile.TarFile().open()
| ^^^^^^^^^^^^^^^^^^^^^^ SIM115
87 | f = zipfile.ZipFile("foo.zip").open("foo.txt")
88 | f = io.open("foo.txt")
|

SIM115.py:87:5: SIM115 Use a context manager for opening files
|
85 | f = tarfile.TarFile("foo.tar").open()
86 | f = tarfile.TarFile().open()
87 | f = zipfile.ZipFile("foo.zip").open("foo.txt")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM115
88 | f = io.open("foo.txt")
89 | f = io.open_code("foo.txt")
|

SIM115.py:88:5: SIM115 Use a context manager for opening files
|
86 | f = tarfile.TarFile().open()
87 | f = zipfile.ZipFile("foo.zip").open("foo.txt")
88 | f = io.open("foo.txt")
| ^^^^^^^ SIM115
89 | f = io.open_code("foo.txt")
90 | f = codecs.open("foo.txt")
|

SIM115.py:89:5: SIM115 Use a context manager for opening files
|
87 | f = zipfile.ZipFile("foo.zip").open("foo.txt")
88 | f = io.open("foo.txt")
89 | f = io.open_code("foo.txt")
| ^^^^^^^^^^^^ SIM115
90 | f = codecs.open("foo.txt")
91 | f = bz2.open("foo.txt")
|

SIM115.py:90:5: SIM115 Use a context manager for opening files
|
88 | f = io.open("foo.txt")
89 | f = io.open_code("foo.txt")
90 | f = codecs.open("foo.txt")
| ^^^^^^^^^^^ SIM115
91 | f = bz2.open("foo.txt")
92 | f = gzip.open("foo.txt")
|

SIM115.py:91:5: SIM115 Use a context manager for opening files
|
89 | f = io.open_code("foo.txt")
90 | f = codecs.open("foo.txt")
91 | f = bz2.open("foo.txt")
| ^^^^^^^^ SIM115
92 | f = gzip.open("foo.txt")
93 | f = dbm.open("foo.db")
|

SIM115.py:92:5: SIM115 Use a context manager for opening files
|
90 | f = codecs.open("foo.txt")
91 | f = bz2.open("foo.txt")
92 | f = gzip.open("foo.txt")
| ^^^^^^^^^ SIM115
93 | f = dbm.open("foo.db")
94 | f = dbm.gnu.open("foo.db")
|

SIM115.py:93:5: SIM115 Use a context manager for opening files
|
91 | f = bz2.open("foo.txt")
92 | f = gzip.open("foo.txt")
93 | f = dbm.open("foo.db")
| ^^^^^^^^ SIM115
94 | f = dbm.gnu.open("foo.db")
95 | f = dbm.ndbm.open("foo.db")
|

SIM115.py:94:5: SIM115 Use a context manager for opening files
|
92 | f = gzip.open("foo.txt")
93 | f = dbm.open("foo.db")
94 | f = dbm.gnu.open("foo.db")
| ^^^^^^^^^^^^ SIM115
95 | f = dbm.ndbm.open("foo.db")
96 | f = dbm.dumb.open("foo.db")
|

SIM115.py:95:5: SIM115 Use a context manager for opening files
|
93 | f = dbm.open("foo.db")
94 | f = dbm.gnu.open("foo.db")
95 | f = dbm.ndbm.open("foo.db")
| ^^^^^^^^^^^^^ SIM115
96 | f = dbm.dumb.open("foo.db")
97 | f = lzma.open("foo.xz")
|

SIM115.py:96:5: SIM115 Use a context manager for opening files
|
94 | f = dbm.gnu.open("foo.db")
95 | f = dbm.ndbm.open("foo.db")
96 | f = dbm.dumb.open("foo.db")
| ^^^^^^^^^^^^^ SIM115
97 | f = lzma.open("foo.xz")
98 | f = lzma.LZMAFile("foo.xz")
|

SIM115.py:97:5: SIM115 Use a context manager for opening files
|
95 | f = dbm.ndbm.open("foo.db")
96 | f = dbm.dumb.open("foo.db")
97 | f = lzma.open("foo.xz")
| ^^^^^^^^^ SIM115
98 | f = lzma.LZMAFile("foo.xz")
99 | f = shelve.open("foo.db")
|

SIM115.py:98:5: SIM115 Use a context manager for opening files
|
96 | f = dbm.dumb.open("foo.db")
97 | f = lzma.open("foo.xz")
98 | f = lzma.LZMAFile("foo.xz")
| ^^^^^^^^^^^^^ SIM115
99 | f = shelve.open("foo.db")
100 | f = tokenize.open("foo.py")
|

SIM115.py:99:5: SIM115 Use a context manager for opening files
|
97 | f = lzma.open("foo.xz")
98 | f = lzma.LZMAFile("foo.xz")
99 | f = shelve.open("foo.db")
| ^^^^^^^^^^^ SIM115
100 | f = tokenize.open("foo.py")
101 | f = wave.open("foo.wav")
|

SIM115.py:100:5: SIM115 Use a context manager for opening files
|
98 | f = lzma.LZMAFile("foo.xz")
99 | f = shelve.open("foo.db")
100 | f = tokenize.open("foo.py")
| ^^^^^^^^^^^^^ SIM115
101 | f = wave.open("foo.wav")
102 | f = tarfile.TarFile.taropen("foo.tar")
|

SIM115.py:101:5: SIM115 Use a context manager for opening files
|
99 | f = shelve.open("foo.db")
100 | f = tokenize.open("foo.py")
101 | f = wave.open("foo.wav")
| ^^^^^^^^^ SIM115
102 | f = tarfile.TarFile.taropen("foo.tar")
103 | f = fileinput.input("foo.txt")
|

SIM115.py:102:5: SIM115 Use a context manager for opening files
|
100 | f = tokenize.open("foo.py")
101 | f = wave.open("foo.wav")
102 | f = tarfile.TarFile.taropen("foo.tar")
| ^^^^^^^^^^^^^^^^^^^^^^^ SIM115
103 | f = fileinput.input("foo.txt")
104 | f = fileinput.FileInput("foo.txt")
|

SIM115.py:103:5: SIM115 Use a context manager for opening files
|
101 | f = wave.open("foo.wav")
102 | f = tarfile.TarFile.taropen("foo.tar")
103 | f = fileinput.input("foo.txt")
| ^^^^^^^^^^^^^^^ SIM115
104 | f = fileinput.FileInput("foo.txt")
|

SIM115.py:104:5: SIM115 Use a context manager for opening files
|
102 | f = tarfile.TarFile.taropen("foo.tar")
103 | f = fileinput.input("foo.txt")
104 | f = fileinput.FileInput("foo.txt")
| ^^^^^^^^^^^^^^^^^^^ SIM115
105 |
106 | with contextlib.suppress(Exception):
|

SIM115.py:240:9: SIM115 Use a context manager for opening files
|
238 | def aliased():
239 | from shelve import open as open_shelf
240 | x = open_shelf("foo.dbm")
| ^^^^^^^^^^ SIM115
241 | x.close()
|

SIM115.py:244:9: SIM115 Use a context manager for opening files
|
243 | from tarfile import TarFile as TF
244 | f = TF("foo").open()
| ^^^^^^^^^^^^^^ SIM115
245 | f.close()
|

SIM115.py:257:5: SIM115 Use a context manager for opening files
|
256 | # SIM115
257 | f = dbm.sqlite3.open("foo.db")
| ^^^^^^^^^^^^^^^^ SIM115
258 | f.close()
|
Loading

0 comments on commit e38914d

Please sign in to comment.