diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs index 6165ffc955..47eaa9053a 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs @@ -26,6 +26,27 @@ impl AlwaysAutofixableViolation for UncapitalizedEnvironmentVariables { } } +/// ## What it does +/// Check for `dict.get()` calls that pass `None` as the default value. +/// +/// ## Why is this bad? +/// `None` is the default value for `dict.get()`, so it is redundant to pass it +/// explicitly. +/// +/// ## Example +/// ```python +/// ages = {"Tom": 23, "Maria": 23, "Dog": 11} +/// age = ages.get("Cat", None) # None +/// ``` +/// +/// Use instead: +/// ```python +/// ages = {"Tom": 23, "Maria": 23, "Dog": 11} +/// age = ages.get("Cat") # None +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#dict.get) #[violation] pub struct DictGetWithNoneDefault { expected: String, diff --git a/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs b/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs index 33466ac876..d33d046008 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/open_file_with_context_handler.rs @@ -5,6 +5,30 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for usages of the builtin `open()` function without an associated 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. +/// +/// ## Example +/// ```python +/// file = open("foo.txt") +/// ... +/// file.close() +/// ``` +/// +/// Use instead: +/// ```python +/// with open("foo.txt") as file: +/// ... +/// ``` +/// +/// # References +/// - [Python documentation](https://docs.python.org/3/library/functions.html#open) #[violation] pub struct OpenFileWithContextHandler; diff --git a/crates/ruff/src/rules/flake8_simplify/rules/return_in_try_except_finally.rs b/crates/ruff/src/rules/flake8_simplify/rules/return_in_try_except_finally.rs index 159c104cd3..b473a071c8 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/return_in_try_except_finally.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/return_in_try_except_finally.rs @@ -5,17 +5,51 @@ use ruff_macros::{derive_message_formats, violation}; use crate::checkers::ast::Checker; +/// ## What it does +/// Checks for `return` statements in `try`-`except` and `finally` blocks. +/// +/// ## Why is this bad? +/// The `return` statement in a `finally` block will always be executed, even if +/// an exception is raised in the `try` or `except` block. This can lead to +/// unexpected behavior. +/// +/// ## Example +/// ```python +/// def squared(n): +/// try: +/// sqr = n**2 +/// return sqr +/// except Exception: +/// return "An exception occurred" +/// finally: +/// return -1 # Always returns -1. +/// ``` +/// +/// Use instead: +/// ```python +/// def squared(n): +/// try: +/// return_value = n**2 +/// except Exception: +/// return_value = "An exception occurred" +/// finally: +/// return_value = -1 +/// return return_value +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions) #[violation] pub struct ReturnInTryExceptFinally; impl Violation for ReturnInTryExceptFinally { #[derive_message_formats] fn message(&self) -> String { - format!("Don't use `return` in `try`/`except` and `finally`") + format!("Don't use `return` in `try`-`except` and `finally`") } } -fn find_return(stmts: &[rustpython_parser::ast::Stmt]) -> Option<&Stmt> { +fn find_return(stmts: &[Stmt]) -> Option<&Stmt> { stmts .iter() .find(|stmt| matches!(stmt.node, StmtKind::Return { .. })) @@ -34,8 +68,8 @@ pub fn return_in_try_except_finally( find_return(body).is_some() }); - if let Some(finally_return) = find_return(finalbody) { - if try_has_return || except_has_return { + if try_has_return || except_has_return { + if let Some(finally_return) = find_return(finalbody) { checker.diagnostics.push(Diagnostic::new( ReturnInTryExceptFinally, finally_return.range(), diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM107_SIM107.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM107_SIM107.py.snap index 8b56f2eda8..2592854591 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM107_SIM107.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM107_SIM107.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/flake8_simplify/mod.rs --- -SIM107.py:9:9: SIM107 Don't use `return` in `try`/`except` and `finally` +SIM107.py:9:9: SIM107 Don't use `return` in `try`-`except` and `finally` | 9 | return "2" 10 | finally: