Code review

astral-sh · Apr 16, 2024 · 2b50cd0 · 2b50cd0
1 parent 8117173
commit 2b50cd0
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 15 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_length.py b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_length.py
@@ -12,6 +12,10 @@ class Str:
     def __len__(self):
         return "ruff"  # [invalid-length-return]
 
+class LengthNoReturn:
+    def __len__(self):
+        print("ruff")  # [invalid-length-return]
+
 class LengthNegative:
     def __len__(self):
         return -42  # [invalid-length-return]

diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_length_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_length_return.rs
@@ -60,13 +60,21 @@ pub(crate) fn invalid_length_return(checker: &mut Checker, name: &str, body: &[S
         visitor.returns
     };
 
+    if returns.is_empty() {
+        checker.diagnostics.push(Diagnostic::new(
+            InvalidLengthReturnType,
+            body.last().unwrap().range(),
+        ));
+    }
+
     for stmt in returns {
         if let Some(value) = stmt.value.as_deref() {
-            if !matches!(
-                ResolvedPythonType::from(value),
-                ResolvedPythonType::Unknown
-                    | ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
-            ) || is_negative_integer(value)
+            if is_negative_integer(value)
+                || !matches!(
+                    ResolvedPythonType::from(value),
+                    ResolvedPythonType::Unknown
+                        | ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
+                )
             {
                 checker
                     .diagnostics
@@ -82,8 +90,11 @@ pub(crate) fn invalid_length_return(checker: &mut Checker, name: &str, body: &[S
 }
 
 fn is_negative_integer(value: &Expr) -> bool {
-    let Expr::UnaryOp(ast::ExprUnaryOp { op, .. }) = value else {
-        return false;
-    };
-    matches!(op, ast::UnaryOp::USub)
+    matches!(
+        value,
+        Expr::UnaryOp(ast::ExprUnaryOp {
+            op: ast::UnaryOp::USub,
+            ..
+        })
+    )
 }
diff --git a/...t/snapshots/ruff_linter__rules__pylint__tests__PLE0303_invalid_return_type_length.py.snap b/...t/snapshots/ruff_linter__rules__pylint__tests__PLE0303_invalid_return_type_length.py.snap
@@ -28,15 +28,25 @@ invalid_return_type_length.py:13:16: PLE0303 `__len__` does not return a non-neg
 13 |         return "ruff"  # [invalid-length-return]
    |                ^^^^^^ PLE0303
 14 | 
-15 | class LengthNegative:
+15 | class LengthNoReturn:
    |
 
-invalid_return_type_length.py:17:16: PLE0303 `__len__` does not return a non-negative `integer`
+invalid_return_type_length.py:17:9: PLE0303 `__len__` does not return a non-negative `integer`
    |
-15 | class LengthNegative:
+15 | class LengthNoReturn:
 16 |     def __len__(self):
-17 |         return -42  # [invalid-length-return]
-   |                ^^^ PLE0303
+17 |         print("ruff")  # [invalid-length-return]
+   |         ^^^^^^^^^^^^^ PLE0303
 18 | 
-19 | # TODO: Once Ruff has better type checking
+19 | class LengthNegative:
+   |
+
+invalid_return_type_length.py:21:16: PLE0303 `__len__` does not return a non-negative `integer`
+   |
+19 | class LengthNegative:
+20 |     def __len__(self):
+21 |         return -42  # [invalid-length-return]
+   |                ^^^ PLE0303
+22 | 
+23 | # TODO: Once Ruff has better type checking
    |