RUF067: Allow dunder-named assignments in non-strict mode

crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/__init__.py

@@ -44,8 +44,15 @@ def __dir__():  # ok
 __path__ = pkgutil.extend_path(__path__, __name__)  # ok
 __path__ = unknown.extend_path(__path__, __name__)  # also ok
 
-# non-`extend_path` assignments are not allowed
-__path__ = 5  # RUF067
+# any dunder-named assignment is allowed in non-strict mode
+__path__ = 5  # ok
+__submodules__ = []  # ok (e.g. mkinit)
+__protected__ = []  # ok
+__custom__: list[str] = []  # ok
+__submodules__ += ["extra"]  # ok
+
+foo = __submodules__ = []  # RUF067: not every target is a dunder
+__all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
 
 # also allow `__author__`
 __author__ = "The Author"  # ok

crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs

@@ -1,4 +1,5 @@
 use ruff_macros::{ViolationMetadata, derive_message_formats};
+use ruff_python_ast::helpers::is_dunder;
 use ruff_python_ast::{self as ast, Expr, Stmt};
 use ruff_python_semantic::analyze::typing::is_type_checking_block;
 use ruff_text_size::Ranged;
@@ -56,7 +57,8 @@ use crate::{Violation, checkers::ast::Checker};
 /// In non-strict mode, this rule allows several common patterns in `__init__.py` files:
 ///
 /// - Imports
-/// - Assignments to `__all__`, `__path__`, `__version__`, and `__author__`
+/// - Assignments to dunder names (identifiers starting and ending with `__`, such as `__all__` or
+///   `__submodules__`)
 /// - Module-level and attribute docstrings
 /// - `if TYPE_CHECKING` blocks
 /// - [PEP-562] module-level `__getattr__` and `__dir__` functions
@@ -128,33 +130,10 @@ pub(crate) fn non_empty_init_module(checker: &Checker, stmt: &Stmt) {
         }
 
         if let Some(assignment) = Assignment::from_stmt(stmt) {
-            // Allow assignments to `__all__`.
-            //
-            // TODO(brent) should we allow additional cases here? Beyond simple assignments, you could
-            // also append or extend `__all__`.
-            //
-            // This is actually going slightly beyond the upstream rule already, which only checks for
-            // `Stmt::Assign`.
-            if assignment.is_assignment_to("__all__") {
-                return;
-            }
-
-            // Allow legacy namespace packages with assignments like:
-            //
-            // ```py
-            // __path__ = __import__('pkgutil').extend_path(__path__, __name__)
-            // ```
-            if assignment.is_assignment_to("__path__") && assignment.is_pkgutil_extend_path() {
-                return;
-            }
-
-            // Allow assignments to `__version__`.
-            if assignment.is_assignment_to("__version__") {
-                return;
-            }
-
-            // Allow assignments to `__author__`.
-            if assignment.is_assignment_to("__author__") {
+            // Allow assignments to any dunder-named target (e.g. `__all__`, `__path__`, or
+            // tool-specific names like `__submodules__`). Chained assignments require every target
+            // to be a dunder.
+            if assignment.all_targets_are_dunder() {
                 return;
             }
         }
@@ -172,88 +151,27 @@ pub(crate) fn non_empty_init_module(checker: &Checker, stmt: &Stmt) {
 /// assignments.
 struct Assignment<'a> {
     targets: &'a [Expr],
-    value: Option<&'a Expr>,
 }
 
 impl<'a> Assignment<'a> {
     fn from_stmt(stmt: &'a Stmt) -> Option<Self> {
-        let (targets, value) = match stmt {
-            Stmt::Assign(ast::StmtAssign { targets, value, .. }) => {
-                (targets.as_slice(), Some(&**value))
-            }
-            Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => {
-                (std::slice::from_ref(&**target), value.as_deref())
-            }
-            Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => {
-                (std::slice::from_ref(&**target), Some(&**value))
-            }
+        let targets = match stmt {
+            Stmt::Assign(ast::StmtAssign { targets, .. }) => targets.as_slice(),
+            Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => std::slice::from_ref(&**target),
+            Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => std::slice::from_ref(&**target),
             _ => return None,
         };
 
-        Some(Self { targets, value })
+        Some(Self { targets })
     }
 
-    /// Returns whether all of the assignment targets match `name`.
-    ///
-    /// For example, both of the following would be allowed for a `name` of `__all__`:
-    ///
-    /// ```py
-    /// __all__ = ["foo"]
-    /// __all__ = __all__ = ["foo"]
-    /// ```
-    ///
-    /// but not:
-    ///
-    /// ```py
-    /// __all__ = another_list = ["foo"]
-    /// ```
-    fn is_assignment_to(&self, name: &str) -> bool {
-        self.targets
-            .iter()
-            .all(|target| target.as_name_expr().is_some_and(|expr| expr.id == name))
-    }
-
-    /// Returns `true` if the value being assigned is a call to `pkgutil.extend_path`.
-    ///
-    /// For example, both of the following would return true:
-    ///
-    /// ```py
-    /// __path__ = __import__('pkgutil').extend_path(__path__, __name__)
-    /// __path__ = other.extend_path(__path__, __name__)
-    /// ```
-    ///
-    /// We're intentionally a bit less strict here, not requiring that the receiver of the
-    /// `extend_path` call is the typical `__import__('pkgutil')` or `pkgutil`.
-    fn is_pkgutil_extend_path(&self) -> bool {
-        let Some(Expr::Call(ast::ExprCall {
-            func: extend_func,
-            arguments: extend_arguments,
-            ..
-        })) = self.value
-        else {
-            return false;
-        };
-
-        let Expr::Attribute(ast::ExprAttribute {
-            attr: maybe_extend_path,
-            ..
-        }) = &**extend_func
-        else {
-            return false;
-        };
-
-        // Test that this is an `extend_path(__path__, __name__)` call
-        if maybe_extend_path != "extend_path" {
-            return false;
-        }
-
-        let Some(Expr::Name(path)) = extend_arguments.find_argument_value("path", 0) else {
-            return false;
-        };
-        let Some(Expr::Name(name)) = extend_arguments.find_argument_value("name", 1) else {
-            return false;
-        };
-
-        path.id() == "__path__" && name.id() == "__name__"
+    /// Returns `true` when every assignment target is a simple name and each name is a dunder
+    /// (`__` prefix and suffix), matching [`is_dunder`].
+    fn all_targets_are_dunder(&self) -> bool {
+        self.targets.iter().all(|target| {
+            target
+                .as_name_expr()
+                .is_some_and(|name| is_dunder(name.id.as_str()))
+        })
     }
 }

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules____init__.py.snap

@@ -43,11 +43,21 @@ RUF067 `__init__` module should only contain docstrings and re-exports
    |
 
 RUF067 `__init__` module should only contain docstrings and re-exports
-  --> __init__.py:48:1
+  --> __init__.py:54:1
    |
-47 | # non-`extend_path` assignments are not allowed
-48 | __path__ = 5  # RUF067
-   | ^^^^^^^^^^^^
-49 |
-50 | # also allow `__author__`
+52 | __submodules__ += ["extra"]  # ok
+53 |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+55 | __all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
+   |
+
+RUF067 `__init__` module should only contain docstrings and re-exports
+  --> __init__.py:55:1
+   |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+55 | __all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+56 |
+57 | # also allow `__author__`
    |

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__strictly_empty_init_modules_ruf067.snap

@@ -6,8 +6,8 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs
 +linter.ruff.strictly_empty_init_modules = true
 
 --- Summary ---
-Removed: 5
-Added: 24
+Removed: 6
+Added: 30
 
 --- Removed ---
 RUF067 `__init__` module should only contain docstrings and re-exports
@@ -56,13 +56,24 @@ RUF067 `__init__` module should only contain docstrings and re-exports
 
 
 RUF067 `__init__` module should only contain docstrings and re-exports
-  --> __init__.py:48:1
+  --> __init__.py:54:1
    |
-47 | # non-`extend_path` assignments are not allowed
-48 | __path__ = 5  # RUF067
-   | ^^^^^^^^^^^^
-49 |
-50 | # also allow `__author__`
+52 | __submodules__ += ["extra"]  # ok
+53 |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+55 | __all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
+   |
+
+
+RUF067 `__init__` module should only contain docstrings and re-exports
+  --> __init__.py:55:1
+   |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+55 | __all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+56 |
+57 | # also allow `__author__`
    |
 
 
@@ -320,25 +331,94 @@ RUF067 `__init__` module should not contain any code
 45 | __path__ = unknown.extend_path(__path__, __name__)  # also ok
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 46 |
-47 | # non-`extend_path` assignments are not allowed
+47 | # any dunder-named assignment is allowed in non-strict mode
    |
 
 
 RUF067 `__init__` module should not contain any code
   --> __init__.py:48:1
    |
-47 | # non-`extend_path` assignments are not allowed
-48 | __path__ = 5  # RUF067
+47 | # any dunder-named assignment is allowed in non-strict mode
+48 | __path__ = 5  # ok
    | ^^^^^^^^^^^^
-49 |
-50 | # also allow `__author__`
+49 | __submodules__ = []  # ok (e.g. mkinit)
+50 | __protected__ = []  # ok
+   |
+
+
+RUF067 `__init__` module should not contain any code
+  --> __init__.py:49:1
+   |
+47 | # any dunder-named assignment is allowed in non-strict mode
+48 | __path__ = 5  # ok
+49 | __submodules__ = []  # ok (e.g. mkinit)
+   | ^^^^^^^^^^^^^^^^^^^
+50 | __protected__ = []  # ok
+51 | __custom__: list[str] = []  # ok
+   |
+
+
+RUF067 `__init__` module should not contain any code
+  --> __init__.py:50:1
+   |
+48 | __path__ = 5  # ok
+49 | __submodules__ = []  # ok (e.g. mkinit)
+50 | __protected__ = []  # ok
+   | ^^^^^^^^^^^^^^^^^^
+51 | __custom__: list[str] = []  # ok
+52 | __submodules__ += ["extra"]  # ok
    |
 
 
 RUF067 `__init__` module should not contain any code
   --> __init__.py:51:1
    |
-50 | # also allow `__author__`
-51 | __author__ = "The Author"  # ok
+49 | __submodules__ = []  # ok (e.g. mkinit)
+50 | __protected__ = []  # ok
+51 | __custom__: list[str] = []  # ok
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+52 | __submodules__ += ["extra"]  # ok
+   |
+
+
+RUF067 `__init__` module should not contain any code
+  --> __init__.py:52:1
+   |
+50 | __protected__ = []  # ok
+51 | __custom__: list[str] = []  # ok
+52 | __submodules__ += ["extra"]  # ok
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+53 |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+   |
+
+
+RUF067 `__init__` module should not contain any code
+  --> __init__.py:54:1
+   |
+52 | __submodules__ += ["extra"]  # ok
+53 |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+55 | __all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
+   |
+
+
+RUF067 `__init__` module should not contain any code
+  --> __init__.py:55:1
+   |
+54 | foo = __submodules__ = []  # RUF067: not every target is a dunder
+55 | __all__[0] = __version__ = "1"  # RUF067: subscript target is not a simple name
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+56 |
+57 | # also allow `__author__`
+   |
+
+
+RUF067 `__init__` module should not contain any code
+  --> __init__.py:58:1
+   |
+57 | # also allow `__author__`
+58 | __author__ = "The Author"  # ok
    | ^^^^^^^^^^^^^^^^^^^^^^^^^
    |