diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 980003bfc57e1..f6f6321fd4702 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -98,7 +98,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pylint::rules::invalid_bool_return(checker, name, body); } if checker.enabled(Rule::InvalidBytesReturnType) { - pylint::rules::invalid_bytes_return(checker, name, body); + pylint::rules::invalid_bytes_return(checker, function_def); } if checker.enabled(Rule::InvalidStrReturnType) { pylint::rules::invalid_str_return(checker, name, body); diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs index ae17d50b423ae..e028eb02c2084 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs @@ -1,10 +1,11 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::{is_docstring_stmt, map_callable}; +use ruff_python_ast::helpers::is_docstring_stmt; use ruff_python_ast::name::QualifiedName; -use ruff_python_ast::{self as ast, Expr, Parameter, ParameterWithDefault, Stmt, StmtRaise}; +use ruff_python_ast::{self as ast, Expr, Parameter, ParameterWithDefault}; use ruff_python_codegen::{Generator, Stylist}; use ruff_python_index::Indexer; +use ruff_python_semantic::analyze::function_type::is_stub; use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_expr}; use ruff_python_semantic::SemanticModel; use ruff_python_trivia::{indentation_at_offset, textwrap}; @@ -213,29 +214,3 @@ fn move_initialization( let initialization_edit = Edit::insertion(content, pos); Some(Fix::unsafe_edits(default_edit, [initialization_edit])) } - -/// Returns `true` if a function has an empty body, and is therefore a stub. -/// -/// A function body is considered to be empty if it contains only `pass` statements, `...` literals, -/// `NotImplementedError` raises, or string literal statements (docstrings). -fn is_stub(function_def: &ast::StmtFunctionDef, semantic: &SemanticModel) -> bool { - function_def.body.iter().all(|stmt| match stmt { - Stmt::Pass(_) => true, - Stmt::Expr(ast::StmtExpr { value, range: _ }) => { - matches!( - value.as_ref(), - Expr::StringLiteral(_) | Expr::EllipsisLiteral(_) - ) - } - Stmt::Raise(StmtRaise { - range: _, - exc: exception, - cause: _, - }) => exception.as_ref().is_some_and(|exc| { - semantic - .resolve_builtin_symbol(map_callable(exc)) - .is_some_and(|name| matches!(name, "NotImplementedError" | "NotImplemented")) - }), - _ => false, - }) -} diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_bytes_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_bytes_return.rs index 05992f1770bfa..fdfe78227d89c 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/invalid_bytes_return.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_bytes_return.rs @@ -1,8 +1,10 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::ReturnStatementVisitor; +use ruff_python_ast::identifier::Identifier; use ruff_python_ast::visitor::Visitor; -use ruff_python_ast::Stmt; +use ruff_python_ast::{self as ast}; +use ruff_python_semantic::analyze::function_type::is_stub; use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType}; use ruff_text_size::Ranged; @@ -42,8 +44,8 @@ impl Violation for InvalidBytesReturnType { } /// E0308 -pub(crate) fn invalid_bytes_return(checker: &mut Checker, name: &str, body: &[Stmt]) { - if name != "__bytes__" { +pub(crate) fn invalid_bytes_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { + if function_def.name.as_str() != "__bytes__" { return; } @@ -51,35 +53,20 @@ pub(crate) fn invalid_bytes_return(checker: &mut Checker, name: &str, body: &[St return; } - if body.len() == 1 - && (matches!(&body[0], Stmt::Expr(expr) if expr.value.is_ellipsis_literal_expr()) - || body[0].is_pass_stmt() - || body[0].is_raise_stmt()) - { - return; - } - - let body_without_comments = body - .iter() - .filter(|stmt| !matches!(stmt, Stmt::Expr(expr) if expr.value.is_string_literal_expr())) - .collect::>(); - if body_without_comments.is_empty() { - return; - } - if body_without_comments.len() == 1 && body_without_comments[0].is_raise_stmt() { + if is_stub(function_def, checker.semantic()) { return; } let returns = { let mut visitor = ReturnStatementVisitor::default(); - visitor.visit_body(body); + visitor.visit_body(&function_def.body); visitor.returns }; if returns.is_empty() { checker.diagnostics.push(Diagnostic::new( InvalidBytesReturnType, - body.last().unwrap().range(), + function_def.identifier(), )); } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0308_invalid_return_type_bytes.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0308_invalid_return_type_bytes.py.snap index a9772b0c94240..462121c23098a 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0308_invalid_return_type_bytes.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0308_invalid_return_type_bytes.py.snap @@ -25,18 +25,19 @@ invalid_return_type_bytes.py:16:16: PLE0308 `__bytes__` does not return `bytes` | ^^^^^^^^^^^^ PLE0308 | -invalid_return_type_bytes.py:21:9: PLE0308 `__bytes__` does not return `bytes` +invalid_return_type_bytes.py:20:9: PLE0308 `__bytes__` does not return `bytes` | 19 | class BytesNoReturn: 20 | def __bytes__(self): + | ^^^^^^^^^ PLE0308 21 | print("ruff") # [invalid-bytes-return] - | ^^^^^^^^^^^^^ PLE0308 | -invalid_return_type_bytes.py:27:9: PLE0308 `__bytes__` does not return `bytes` +invalid_return_type_bytes.py:25:9: PLE0308 `__bytes__` does not return `bytes` | +24 | class BytesWrongRaise: 25 | def __bytes__(self): + | ^^^^^^^^^ PLE0308 26 | print("raise some error") 27 | raise NotImplementedError # [invalid-bytes-return] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ PLE0308 | diff --git a/crates/ruff_python_semantic/src/analyze/function_type.rs b/crates/ruff_python_semantic/src/analyze/function_type.rs index e249c45a64956..a13881df15108 100644 --- a/crates/ruff_python_semantic/src/analyze/function_type.rs +++ b/crates/ruff_python_semantic/src/analyze/function_type.rs @@ -1,6 +1,6 @@ use ruff_python_ast::helpers::map_callable; use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; -use ruff_python_ast::Decorator; +use ruff_python_ast::{Decorator, Expr, Stmt, StmtExpr, StmtFunctionDef, StmtRaise}; use crate::model::SemanticModel; use crate::scope::{Scope, ScopeKind}; @@ -128,3 +128,29 @@ fn is_class_method( false } + +/// Returns `true` if a function has an empty body, and is therefore a stub. +/// +/// A function body is considered to be empty if it contains only `pass` statements, `...` literals, +/// `NotImplementedError` raises, or string literal statements (docstrings). +pub fn is_stub(function_def: &StmtFunctionDef, semantic: &SemanticModel) -> bool { + function_def.body.iter().all(|stmt| match stmt { + Stmt::Pass(_) => true, + Stmt::Expr(StmtExpr { value, range: _ }) => { + matches!( + value.as_ref(), + Expr::StringLiteral(_) | Expr::EllipsisLiteral(_) + ) + } + Stmt::Raise(StmtRaise { + range: _, + exc: exception, + cause: _, + }) => exception.as_ref().is_some_and(|exc| { + semantic + .resolve_builtin_symbol(map_callable(exc)) + .is_some_and(|name| matches!(name, "NotImplementedError" | "NotImplemented")) + }), + _ => false, + }) +}