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.

Sign up for GitHub

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

Jump to bottom

[ruff-0.7] Stabilise the expansion of open-file-with-context-handler to work with other standard-library IO modules (SIM115) #13680

Merged
merged 1 commit into from
Oct 8, 2024
Merged

[ruff-0.7] Stabilise the expansion of open-file-with-context-handler to work with other standard-library IO modules (SIM115) #13680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion crates/ruff_linter/src/rules/flake8_simplify/mod.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading