From 4d8890eef5ae21d951be948864e8b61b549c77d0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 17 Apr 2024 21:57:20 -0400 Subject: [PATCH] [`pylint`] Omit stubs from `invalid-bool` and `invalid-str-return-type` (#11008) ## Summary Reflecting some improvements that were made in https://github.com/astral-sh/ruff/pull/10959. --- .../pylint/invalid_return_type_str.py | 13 ++++++ .../src/checkers/ast/analyze/statement.rs | 4 +- .../rules/pylint/rules/invalid_bool_return.rs | 21 ++++++++-- .../rules/pylint/rules/invalid_str_return.rs | 21 ++++++++-- ...s__PLE0307_invalid_return_type_str.py.snap | 40 ++++++++----------- 5 files changed, 65 insertions(+), 34 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_str.py b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_str.py index a1eed0ccfa094..bf6a9b52a10d0 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_str.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_str.py @@ -1,36 +1,49 @@ # These testcases should raise errors + class Float: def __str__(self): return 3.05 + class Int: def __str__(self): return 1 + class Int2: def __str__(self): return 0 + class Bool: def __str__(self): return False + # TODO: Once Ruff has better type checking def return_int(): return 3 + class ComplexReturn: def __str__(self): return return_int() + # These testcases should NOT raise errors + class Str: def __str__(self): return "ruff" + class Str2: def __str__(self): x = "ruff" return x + + +class Str3: + def __str__(self): ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 6f314ec5b1437..d7d189e849afd 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -95,7 +95,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } if checker.enabled(Rule::InvalidBoolReturnType) { - pylint::rules::invalid_bool_return(checker, name, body); + pylint::rules::invalid_bool_return(checker, function_def); } if checker.enabled(Rule::InvalidLengthReturnType) { pylint::rules::invalid_length_return(checker, function_def); @@ -104,7 +104,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pylint::rules::invalid_bytes_return(checker, function_def); } if checker.enabled(Rule::InvalidStrReturnType) { - pylint::rules::invalid_str_return(checker, name, body); + pylint::rules::invalid_str_return(checker, function_def); } if checker.enabled(Rule::InvalidFunctionName) { if let Some(diagnostic) = pep8_naming::rules::invalid_function_name( diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs index d677b86e51832..3fe6dad55b3f5 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_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::{NumberLike, PythonType, ResolvedPythonType}; use ruff_text_size::Ranged; @@ -42,8 +44,8 @@ impl Violation for InvalidBoolReturnType { } /// E0307 -pub(crate) fn invalid_bool_return(checker: &mut Checker, name: &str, body: &[Stmt]) { - if name != "__bool__" { +pub(crate) fn invalid_bool_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { + if function_def.name.as_str() != "__bool__" { return; } @@ -51,12 +53,23 @@ pub(crate) fn invalid_bool_return(checker: &mut Checker, name: &str, body: &[Stm return; } + 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( + InvalidBoolReturnType, + function_def.identifier(), + )); + } + for stmt in returns { if let Some(value) = stmt.value.as_deref() { if !matches!( diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_str_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_str_return.rs index cf7856ecf4606..ef492562de4d3 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/invalid_str_return.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_str_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 InvalidStrReturnType { } /// E0307 -pub(crate) fn invalid_str_return(checker: &mut Checker, name: &str, body: &[Stmt]) { - if name != "__str__" { +pub(crate) fn invalid_str_return(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { + if function_def.name.as_str() != "__str__" { return; } @@ -51,12 +53,23 @@ pub(crate) fn invalid_str_return(checker: &mut Checker, name: &str, body: &[Stmt return; } + 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( + InvalidStrReturnType, + function_def.identifier(), + )); + } + for stmt in returns { if let Some(value) = stmt.value.as_deref() { if !matches!( diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0307_invalid_return_type_str.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0307_invalid_return_type_str.py.snap index f7a44c074eff9..d3ee70b18ddac 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0307_invalid_return_type_str.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0307_invalid_return_type_str.py.snap @@ -1,42 +1,34 @@ --- source: crates/ruff_linter/src/rules/pylint/mod.rs --- -invalid_return_type_str.py:5:16: PLE0307 `__str__` does not return `str` +invalid_return_type_str.py:6:16: PLE0307 `__str__` does not return `str` | -3 | class Float: -4 | def __str__(self): -5 | return 3.05 +4 | class Float: +5 | def __str__(self): +6 | return 3.05 | ^^^^ PLE0307 -6 | -7 | class Int: | -invalid_return_type_str.py:9:16: PLE0307 `__str__` does not return `str` +invalid_return_type_str.py:11:16: PLE0307 `__str__` does not return `str` | - 7 | class Int: - 8 | def __str__(self): - 9 | return 1 + 9 | class Int: +10 | def __str__(self): +11 | return 1 | ^ PLE0307 -10 | -11 | class Int2: | -invalid_return_type_str.py:13:16: PLE0307 `__str__` does not return `str` +invalid_return_type_str.py:16:16: PLE0307 `__str__` does not return `str` | -11 | class Int2: -12 | def __str__(self): -13 | return 0 +14 | class Int2: +15 | def __str__(self): +16 | return 0 | ^ PLE0307 -14 | -15 | class Bool: | -invalid_return_type_str.py:17:16: PLE0307 `__str__` does not return `str` +invalid_return_type_str.py:21:16: PLE0307 `__str__` does not return `str` | -15 | class Bool: -16 | def __str__(self): -17 | return False +19 | class Bool: +20 | def __str__(self): +21 | return False | ^^^^^ PLE0307 -18 | -19 | # TODO: Once Ruff has better type checking |