Avoid recommending context manager in __enter__ implementations (#11575)

## Summary

Closes https://github.com/astral-sh/ruff/issues/11567.
This commit is contained in:
Charlie Marsh 2024-05-27 21:44:24 -04:00 committed by GitHub
parent ab107ef1f3
commit a38c05bf13
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 27 additions and 3 deletions

View file

@ -46,3 +46,15 @@ with contextlib.ExitStack() as exit_stack:
# OK (quick one-liner to clear file contents) # OK (quick one-liner to clear file contents)
open("filename", "w").close() open("filename", "w").close()
pathlib.Path("filename").open("w").close() pathlib.Path("filename").open("w").close()
# OK (custom context manager)
class MyFile:
def __init__(self, filename: str):
self.filename = filename
def __enter__(self):
self.file = open(self.filename)
def __exit__(self, exc_type, exc_val, exc_tb):
self.file.close()

View file

@ -2,7 +2,7 @@ use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::SemanticModel; use ruff_python_semantic::{ScopeKind, SemanticModel};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -114,24 +114,27 @@ fn match_exit_stack(semantic: &SemanticModel) -> bool {
/// Return `true` if `func` is the builtin `open` or `pathlib.Path(...).open`. /// 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, func: &Expr) -> bool {
// open(...) // Ex) `open(...)`
if semantic.match_builtin_expr(func, "open") { if semantic.match_builtin_expr(func, "open") {
return true; return true;
} }
// pathlib.Path(...).open() // Ex) `pathlib.Path(...).open()`
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else { let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
return false; return false;
}; };
if attr != "open" { if attr != "open" {
return false; return false;
} }
let Expr::Call(ast::ExprCall { let Expr::Call(ast::ExprCall {
func: value_func, .. func: value_func, ..
}) = &**value }) = &**value
else { else {
return false; return false;
}; };
semantic semantic
.resolve_qualified_name(value_func) .resolve_qualified_name(value_func)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"])) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pathlib", "Path"]))
@ -189,6 +192,15 @@ pub(crate) fn open_file_with_context_handler(checker: &mut Checker, func: &Expr)
return; return;
} }
// Ex) `def __enter__(self): ...`
if let ScopeKind::Function(ast::StmtFunctionDef { name, .. }) =
&checker.semantic().current_scope().kind
{
if name == "__enter__" {
return;
}
}
checker checker
.diagnostics .diagnostics
.push(Diagnostic::new(OpenFileWithContextHandler, func.range())); .push(Diagnostic::new(OpenFileWithContextHandler, func.range()));