Exempt `PLR1711` and `RET501` if non-`None` annotation (#3705)

crates/ruff/resources/test/fixtures/flake8_return/RET501.py

@@ -2,3 +2,13 @@ def x(y):
     if not y:
         return
     return None  # error
+
+
+class BaseCache:
+    def get(self, key: str) -> str | None:
+        print(f"{key} not found")
+        return None
+
+    def get(self, key: str) -> None:
+        print(f"{key} not found")
+        return None

crates/ruff/resources/test/fixtures/pylint/useless_return.py

@@ -48,3 +48,13 @@ def print_python_version():
     """This function returns None."""
     print(sys.version)
     return None  # [useless-return]
+
+
+class BaseCache:
+    def get(self, key: str) -> str | None:
+        print(f"{key} not found")
+        return None
+
+    def get(self, key: str) -> None:
+        print(f"{key} not found")
+        return None

crates/ruff/src/checkers/ast/mod.rs

@@ -434,11 +434,20 @@ where
                     Rule::SuperfluousElseContinue,
                     Rule::SuperfluousElseBreak,
                 ]) {
-                    flake8_return::rules::function(self, body);
+                    flake8_return::rules::function(
+                        self,
+                        body,
+                        returns.as_ref().map(|expr| &**expr),
+                    );
                 }
 
                 if self.settings.rules.enabled(Rule::UselessReturn) {
-                    pylint::rules::useless_return(self, stmt, body);
+                    pylint::rules::useless_return(
+                        self,
+                        stmt,
+                        body,
+                        returns.as_ref().map(|expr| &**expr),
+                    );
                 }
 
                 if self.settings.rules.enabled(Rule::ComplexStructure) {

crates/ruff/src/rules/flake8_annotations/helpers.rs

@@ -6,9 +6,7 @@ use ruff_python_ast::visibility;
 use crate::checkers::ast::Checker;
 use crate::docstrings::definition::{Definition, DefinitionKind};
 
-pub(super) fn match_function_def(
+pub(super) fn match_function_def(stmt: &Stmt) -> (&str, &Arguments, Option<&Expr>, &Vec<Stmt>) {
-    stmt: &Stmt,
-) -> (&str, &Arguments, &Option<Box<Expr>>, &Vec<Stmt>) {
     match &stmt.node {
         StmtKind::FunctionDef {
             name,
@@ -23,7 +21,7 @@ pub(super) fn match_function_def(
             returns,
             body,
             ..
-        } => (name, args, returns, body),
+        } => (name, args, returns.as_ref().map(|expr| &**expr), body),
         _ => panic!("Found non-FunctionDef in match_name"),
     }
 }

crates/ruff/src/rules/flake8_return/rules.rs

@@ -4,6 +4,7 @@ use rustpython_parser::ast::{Constant, Expr, ExprKind, Location, Stmt, StmtKind}
 use ruff_diagnostics::{AlwaysAutofixableViolation, Violation};
 use ruff_diagnostics::{Diagnostic, Fix};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::is_const_none;
 use ruff_python_ast::helpers::{elif_else_range, end_of_statement};
 use ruff_python_ast::types::Range;
 use ruff_python_ast::visitor::Visitor;
@@ -484,7 +485,7 @@ fn superfluous_else(checker: &mut Checker, stack: &Stack) -> bool {
 }
 
 /// Run all checks from the `flake8-return` plugin.
-pub fn function(checker: &mut Checker, body: &[Stmt]) {
+pub fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Expr>) {
     // Skip empty functions.
     if body.is_empty() {
         return;
@@ -535,7 +536,10 @@ pub fn function(checker: &mut Checker, body: &[Stmt]) {
 
     if !result_exists(&stack.returns) {
         if checker.settings.rules.enabled(Rule::UnnecessaryReturnNone) {
-            unnecessary_return_none(checker, &stack);
+            // Skip functions that have a return annotation that is not `None`.
+            if returns.map_or(true, is_const_none) {
+                unnecessary_return_none(checker, &stack);
+            }
         }
         return;
     }

crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET501_RET501.py.snap

@@ -22,4 +22,24 @@ expression: diagnostics
       row: 4
       column: 15
   parent: ~
+- kind:
+    name: UnnecessaryReturnNone
+    body: "Do not explicitly `return None` in function if it is the only possible return value"
+    suggestion: "Remove explicit `return None`"
+    fixable: true
+  location:
+    row: 14
+    column: 8
+  end_location:
+    row: 14
+    column: 19
+  fix:
+    content: return
+    location:
+      row: 14
+      column: 8
+    end_location:
+      row: 14
+      column: 19
+  parent: ~
 

crates/ruff/src/rules/pylint/rules/useless_return.rs

@@ -1,5 +1,5 @@
 use log::error;
-use rustpython_parser::ast::{Constant, ExprKind, Stmt, StmtKind};
+use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
 use ruff_macros::{derive_message_formats, violation};
@@ -46,7 +46,17 @@ impl AlwaysAutofixableViolation for UselessReturn {
 }
 
 /// PLR1711
-pub fn useless_return<'a>(checker: &mut Checker<'a>, stmt: &'a Stmt, body: &'a [Stmt]) {
+pub fn useless_return<'a>(
+    checker: &mut Checker<'a>,
+    stmt: &'a Stmt,
+    body: &'a [Stmt],
+    returns: Option<&'a Expr>,
+) {
+    // Skip functions that have a return annotation that is not `None`.
+    if !returns.map_or(true, is_const_none) {
+        return;
+    }
+
     // Skip empty functions.
     if body.is_empty() {
         return;

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1711_useless_return.py.snap

@@ -102,4 +102,24 @@ expression: diagnostics
       row: 51
       column: 0
   parent: ~
+- kind:
+    name: UselessReturn
+    body: "Useless `return` statement at end of function"
+    suggestion: "Remove useless `return` statement"
+    fixable: true
+  location:
+    row: 60
+    column: 8
+  end_location:
+    row: 60
+    column: 19
+  fix:
+    content: ""
+    location:
+      row: 60
+      column: 0
+    end_location:
+      row: 61
+      column: 0
+  parent: ~