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

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
This commit is contained in:
Steve C 2024-08-22 09:18:55 -04:00 committed by GitHub
parent d1d067896c
commit d37e2e5d33
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 602 additions and 22 deletions

View file

@ -883,7 +883,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
flake8_simplify::rules::use_capital_environment_variables(checker, expr);
}
if checker.enabled(Rule::OpenFileWithContextHandler) {
flake8_simplify::rules::open_file_with_context_handler(checker, func);
flake8_simplify::rules::open_file_with_context_handler(checker, call);
}
if checker.enabled(Rule::DictGetWithNoneDefault) {
flake8_simplify::rules::dict_get_with_none_default(checker, expr);

View file

@ -58,6 +58,7 @@ 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__{}_{}",

View file

@ -8,14 +8,20 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for uses of the builtin `open()` function without an associated context
/// manager.
/// Checks for cases where files are opened (e.g., using the builtin `open()` function)
/// without using a context manager.
///
/// ## 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.
///
/// ## Example
/// ```python
/// file = open("foo.txt")
@ -37,7 +43,7 @@ pub struct OpenFileWithContextHandler;
impl Violation for OpenFileWithContextHandler {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use context handler for opening files")
format!("Use a context manager for opening files")
}
}
@ -113,14 +119,14 @@ fn match_exit_stack(semantic: &SemanticModel) -> bool {
}
/// Return `true` if `func` is the builtin `open` or `pathlib.Path(...).open`.
fn is_open(semantic: &SemanticModel, func: &Expr) -> bool {
fn is_open(semantic: &SemanticModel, call: &ast::ExprCall) -> bool {
// Ex) `open(...)`
if semantic.match_builtin_expr(func, "open") {
if semantic.match_builtin_expr(&call.func, "open") {
return true;
}
// Ex) `pathlib.Path(...).open()`
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = &*call.func else {
return false;
};
@ -140,6 +146,63 @@ fn is_open(semantic: &SemanticModel, func: &Expr) -> bool {
.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 {
let func = &*call.func;
// Ex) `open(...)`
if let Some(qualified_name) = semantic.resolve_qualified_name(func) {
return matches!(
qualified_name.segments(),
[
"" | "builtins"
| "bz2"
| "codecs"
| "dbm"
| "gzip"
| "tarfile"
| "shelve"
| "tokenize"
| "wave",
"open"
] | ["dbm", "gnu" | "ndbm" | "dumb", "open"]
| ["fileinput", "FileInput" | "input"]
| ["io", "open" | "open_code"]
| ["lzma", "LZMAFile" | "open"]
| ["tarfile", "TarFile", "taropen"]
| [
"tempfile",
"TemporaryFile" | "NamedTemporaryFile" | "SpooledTemporaryFile"
]
);
}
// Ex) `pathlib.Path(...).open()`
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
return false;
};
let Expr::Call(ast::ExprCall { func, .. }) = &**value else {
return false;
};
// E.g. for `pathlib.Path(...).open()`, `qualified_name_of_instance.segments() == ["pathlib", "Path"]`
let Some(qualified_name_of_instance) = semantic.resolve_qualified_name(func) else {
return false;
};
matches!(
(qualified_name_of_instance.segments(), &**attr),
(
["pathlib", "Path"] | ["zipfile", "ZipFile"] | ["lzma", "LZMAFile"],
"open"
) | (
["tarfile", "TarFile"],
"open" | "taropen" | "gzopen" | "bz2open" | "xzopen"
)
)
}
/// Return `true` if the current expression is followed by a `close` call.
fn is_closed(semantic: &SemanticModel) -> bool {
let Some(expr) = semantic.current_expression_grandparent() else {
@ -165,11 +228,17 @@ fn is_closed(semantic: &SemanticModel) -> bool {
}
/// SIM115
pub(crate) fn open_file_with_context_handler(checker: &mut Checker, func: &Expr) {
pub(crate) fn open_file_with_context_handler(checker: &mut Checker, call: &ast::ExprCall) {
let semantic = checker.semantic();
if !is_open(semantic, func) {
return;
if checker.settings.preview.is_disabled() {
if !is_open(semantic, call) {
return;
}
} else {
if !is_open_preview(semantic, call) {
return;
}
}
// Ex) `open("foo.txt").close()`
@ -201,7 +270,8 @@ pub(crate) fn open_file_with_context_handler(checker: &mut Checker, func: &Expr)
}
}
checker
.diagnostics
.push(Diagnostic::new(OpenFileWithContextHandler, func.range()));
checker.diagnostics.push(Diagnostic::new(
OpenFileWithContextHandler,
call.func.range(),
));
}

View file

@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
---
SIM115.py:8:5: SIM115 Use context handler for opening files
SIM115.py:8:5: SIM115 Use a context manager for opening files
|
7 | # SIM115
8 | f = open("foo.txt")
@ -10,7 +10,7 @@ SIM115.py:8:5: SIM115 Use context handler for opening files
10 | f = pathlib.Path("foo.txt").open()
|
SIM115.py:9:5: SIM115 Use context handler for opening files
SIM115.py:9:5: SIM115 Use a context manager for opening files
|
7 | # SIM115
8 | f = open("foo.txt")
@ -20,7 +20,7 @@ SIM115.py:9:5: SIM115 Use context handler for opening files
11 | f = pl.Path("foo.txt").open()
|
SIM115.py:10:5: SIM115 Use context handler for opening files
SIM115.py:10:5: SIM115 Use a context manager for opening files
|
8 | f = open("foo.txt")
9 | f = Path("foo.txt").open()
@ -30,7 +30,7 @@ SIM115.py:10:5: SIM115 Use context handler for opening files
12 | f = P("foo.txt").open()
|
SIM115.py:11:5: SIM115 Use context handler for opening files
SIM115.py:11:5: SIM115 Use a context manager for opening files
|
9 | f = Path("foo.txt").open()
10 | f = pathlib.Path("foo.txt").open()
@ -40,7 +40,7 @@ SIM115.py:11:5: SIM115 Use context handler for opening files
13 | data = f.read()
|
SIM115.py:12:5: SIM115 Use context handler for opening files
SIM115.py:12:5: SIM115 Use a context manager for opening files
|
10 | f = pathlib.Path("foo.txt").open()
11 | f = pl.Path("foo.txt").open()
@ -50,7 +50,7 @@ SIM115.py:12:5: SIM115 Use context handler for opening files
14 | f.close()
|
SIM115.py:39:9: SIM115 Use context handler for opening files
SIM115.py:39:9: SIM115 Use a context manager for opening files
|
37 | # SIM115
38 | with contextlib.ExitStack():
@ -59,5 +59,3 @@ SIM115.py:39:9: SIM115 Use context handler for opening files
40 |
41 | # OK
|

View file

@ -0,0 +1,326 @@
---
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
---
SIM115.py:8:5: SIM115 Use a context manager 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 a context manager 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()
|
SIM115.py:10:5: SIM115 Use a context manager 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()
|
SIM115.py:11:5: SIM115 Use a context manager 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()
|
SIM115.py:12:5: SIM115 Use a context manager 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()
|
SIM115.py:39:9: SIM115 Use a context manager for opening files
|
37 | # SIM115
38 | with contextlib.ExitStack():
39 | f = open("filename")
| ^^^^ SIM115
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()
|