feat: pylint `PLE0604` and `PLE0605` (#2241)

README.md

@@ -1248,6 +1248,8 @@ For more, see [Pylint](https://pypi.org/project/pylint/) on PyPI.
 | ---- | ---- | ------- | --- |
 | PLE0117 | nonlocal-without-binding | Nonlocal name `{name}` found without binding |  |
 | PLE0118 | used-prior-global-declaration | Name `{name}` is used prior to global declaration on line {line} |  |
+| PLE0604 | invalid-all-object | Invalid object in `__all__`, must contain only strings |  |
+| PLE0605 | invalid-all-format | Invalid format for `__all__`, must be `tuple` or `list` |  |
 | PLE1142 | await-outside-async | `await` should be used within an async function |  |
 
 #### Refactor (PLR)

resources/test/fixtures/pylint/PLE0604.py

@@ -0,0 +1,11 @@
+__all__ = (
+    None,  # [invalid-all-object]
+    Fruit,
+    Worm,
+)
+
+class Fruit:
+    pass
+
+class Worm:
+    pass

resources/test/fixtures/pylint/PLE0605.py

@@ -0,0 +1,7 @@
+__all__ = ("CONST")  # [invalid-all-format]
+
+__all__ = ["Hello"] + {"world"}  # [invalid-all-format]
+
+__all__ += {"world"}  # [invalid-all-format]
+
+__all__ = {"world"} + ["Hello"]  # [invalid-all-format]

ruff.schema.json

@@ -1615,6 +1615,10 @@
         "PLE011",
         "PLE0117",
         "PLE0118",
+        "PLE06",
+        "PLE060",
+        "PLE0604",
+        "PLE0605",
         "PLE1",
         "PLE11",
         "PLE114",

src/ast/operations.rs

@@ -1,3 +1,4 @@
+use bitflags::bitflags;
 use rustc_hash::FxHashMap;
 use rustpython_ast::{Cmpop, Located};
 use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
@@ -9,9 +10,21 @@ use crate::ast::types::{Binding, BindingKind, Scope};
 use crate::ast::visitor;
 use crate::ast::visitor::Visitor;
 
+bitflags! {
+    #[derive(Default)]
+    pub struct AllNamesFlags: u32 {
+        const INVALID_FORMAT = 0b0000_0001;
+        const INVALID_OBJECT = 0b0000_0010;
+    }
+}
+
 /// Extract the names bound to a given __all__ assignment.
-pub fn extract_all_names(stmt: &Stmt, scope: &Scope, bindings: &[Binding]) -> Vec<String> {
-    fn add_to_names(names: &mut Vec<String>, elts: &[Expr]) {
+pub fn extract_all_names(
+    stmt: &Stmt,
+    scope: &Scope,
+    bindings: &[Binding],
+) -> (Vec<String>, AllNamesFlags) {
+    fn add_to_names(names: &mut Vec<String>, elts: &[Expr], flags: &mut AllNamesFlags) {
         for elt in elts {
             if let ExprKind::Constant {
                 value: Constant::Str(value),
@@ -19,11 +32,14 @@ pub fn extract_all_names(stmt: &Stmt, scope: &Scope, bindings: &[Binding]) -> Ve
             } = &elt.node
             {
                 names.push(value.to_string());
+            } else {
+                *flags |= AllNamesFlags::INVALID_OBJECT;
             }
         }
     }
 
     let mut names: Vec<String> = vec![];
+    let mut flags = AllNamesFlags::empty();
 
     // Grab the existing bound __all__ values.
     if let StmtKind::AugAssign { .. } = &stmt.node {
@@ -42,7 +58,7 @@ pub fn extract_all_names(stmt: &Stmt, scope: &Scope, bindings: &[Binding]) -> Ve
     } {
         match &value.node {
             ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } => {
-                add_to_names(&mut names, elts);
+                add_to_names(&mut names, elts, &mut flags);
             }
             ExprKind::BinOp { left, right, .. } => {
                 let mut current_left = left;
@@ -50,27 +66,35 @@ pub fn extract_all_names(stmt: &Stmt, scope: &Scope, bindings: &[Binding]) -> Ve
                 while let Some(elts) = match &current_right.node {
                     ExprKind::List { elts, .. } => Some(elts),
                     ExprKind::Tuple { elts, .. } => Some(elts),
-                    _ => None,
+                    _ => {
+                        flags |= AllNamesFlags::INVALID_FORMAT;
+                        None
+                    }
                 } {
-                    add_to_names(&mut names, elts);
+                    add_to_names(&mut names, elts, &mut flags);
                     match &current_left.node {
                         ExprKind::BinOp { left, right, .. } => {
                             current_left = left;
                             current_right = right;
                         }
                         ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } => {
-                            add_to_names(&mut names, elts);
+                            add_to_names(&mut names, elts, &mut flags);
                             break;
                         }
-                        _ => break,
+                        _ => {
+                            flags |= AllNamesFlags::INVALID_FORMAT;
+                            break;
                         }
                     }
                 }
-            _ => {}
+            }
+            _ => {
+                flags |= AllNamesFlags::INVALID_FORMAT;
+            }
         }
     }
 
-    names
+    (names, flags)
 }
 
 #[derive(Default)]

src/checkers/ast.rs

@@ -17,7 +17,7 @@ use rustpython_parser::parser;
 use smallvec::smallvec;
 
 use crate::ast::helpers::{binding_range, collect_call_path, extract_handler_names};
-use crate::ast::operations::extract_all_names;
+use crate::ast::operations::{extract_all_names, AllNamesFlags};
 use crate::ast::relocate::relocate_expr;
 use crate::ast::types::{
     Binding, BindingKind, CallPath, ClassDef, ExecutionContext, FunctionDef, Lambda, Node, Range,
@@ -4157,14 +4157,25 @@ impl<'a> Checker<'a> {
                 }
                 _ => false,
             } {
+                let (all_names, all_names_flags) =
+                    extract_all_names(parent, current, &self.bindings);
+
+                if self.settings.rules.enabled(&Rule::InvalidAllFormat)
+                    && matches!(all_names_flags, AllNamesFlags::INVALID_FORMAT)
+                {
+                    pylint::rules::invalid_all_format(self, expr);
+                }
+
+                if self.settings.rules.enabled(&Rule::InvalidAllObject)
+                    && matches!(all_names_flags, AllNamesFlags::INVALID_OBJECT)
+                {
+                    pylint::rules::invalid_all_object(self, expr);
+                }
+
                 self.add_binding(
                     id,
                     Binding {
-                        kind: BindingKind::Export(extract_all_names(
-                            parent,
-                            current,
-                            &self.bindings,
-                        )),
+                        kind: BindingKind::Export(all_names),
                         runtime_usage: None,
                         synthetic_usage: None,
                         typing_usage: None,

src/registry.rs

@@ -78,6 +78,8 @@ ruff_macros::define_rule_mapping!(
     F842 => violations::UnusedAnnotation,
     F901 => violations::RaiseNotImplemented,
     // pylint
+    PLE0604 => rules::pylint::rules::InvalidAllObject,
+    PLE0605 => rules::pylint::rules::InvalidAllFormat,
     PLC0414 => violations::UselessImportAlias,
     PLC3002 => violations::UnnecessaryDirectLambdaCall,
     PLE0117 => violations::NonlocalWithoutBinding,

src/rules/pylint/mod.rs

@@ -34,6 +34,8 @@ mod tests {
     #[test_case(Rule::MagicValueComparison, Path::new("magic_value_comparison.py"); "PLR2004")]
     #[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"); "PLW0120")]
     #[test_case(Rule::GlobalVariableNotAssigned, Path::new("global_variable_not_assigned.py"); "PLW0602")]
+    #[test_case(Rule::InvalidAllFormat, Path::new("PLE0605.py"); "PLE0605")]
+    #[test_case(Rule::InvalidAllObject, Path::new("PLE0604.py"); "PLE0604")]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
         let diagnostics = test_path(

src/rules/pylint/rules/invalid_all_format.rs

@@ -0,0 +1,25 @@
+use ruff_macros::derive_message_formats;
+use rustpython_ast::Expr;
+
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::define_violation;
+use crate::registry::Diagnostic;
+use crate::violation::Violation;
+
+define_violation!(
+    pub struct InvalidAllFormat;
+);
+impl Violation for InvalidAllFormat {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Invalid format for `__all__`, must be `tuple` or `list`")
+    }
+}
+
+/// PLE0605
+pub fn invalid_all_format(checker: &mut Checker, expr: &Expr) {
+    checker
+        .diagnostics
+        .push(Diagnostic::new(InvalidAllFormat, Range::from_located(expr)));
+}

src/rules/pylint/rules/invalid_all_object.rs

@@ -0,0 +1,25 @@
+use ruff_macros::derive_message_formats;
+use rustpython_ast::Expr;
+
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::define_violation;
+use crate::registry::Diagnostic;
+use crate::violation::Violation;
+
+define_violation!(
+    pub struct InvalidAllObject;
+);
+impl Violation for InvalidAllObject {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Invalid object in `__all__`, must contain only strings")
+    }
+}
+
+/// PLE0604
+pub fn invalid_all_object(checker: &mut Checker, expr: &Expr) {
+    checker
+        .diagnostics
+        .push(Diagnostic::new(InvalidAllObject, Range::from_located(expr)));
+}

src/rules/pylint/rules/mod.rs

@@ -1,5 +1,7 @@
 pub use await_outside_async::await_outside_async;
 pub use constant_comparison::constant_comparison;
+pub use invalid_all_format::{invalid_all_format, InvalidAllFormat};
+pub use invalid_all_object::{invalid_all_object, InvalidAllObject};
 pub use magic_value_comparison::magic_value_comparison;
 pub use merge_isinstance::merge_isinstance;
 pub use property_with_parameters::property_with_parameters;
@@ -12,6 +14,8 @@ pub use useless_import_alias::useless_import_alias;
 
 mod await_outside_async;
 mod constant_comparison;
+mod invalid_all_format;
+mod invalid_all_object;
 mod magic_value_comparison;
 mod merge_isinstance;
 mod property_with_parameters;

src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE0604_PLE0604.py.snap

@@ -0,0 +1,15 @@
+---
+source: src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    InvalidAllObject: ~
+  location:
+    row: 1
+    column: 0
+  end_location:
+    row: 1
+    column: 7
+  fix: ~
+  parent: ~
+

src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE0605_PLE0605.py.snap

@@ -0,0 +1,45 @@
+---
+source: src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    InvalidAllFormat: ~
+  location:
+    row: 1
+    column: 0
+  end_location:
+    row: 1
+    column: 7
+  fix: ~
+  parent: ~
+- kind:
+    InvalidAllFormat: ~
+  location:
+    row: 3
+    column: 0
+  end_location:
+    row: 3
+    column: 7
+  fix: ~
+  parent: ~
+- kind:
+    InvalidAllFormat: ~
+  location:
+    row: 5
+    column: 0
+  end_location:
+    row: 5
+    column: 7
+  fix: ~
+  parent: ~
+- kind:
+    InvalidAllFormat: ~
+  location:
+    row: 7
+    column: 0
+  end_location:
+    row: 7
+    column: 7
+  fix: ~
+  parent: ~
+