From e3ae9871b5faf9e3e21259888a2703b7be548cae Mon Sep 17 00:00:00 2001 From: bitloi Date: Fri, 20 Mar 2026 20:10:24 +0100 Subject: [PATCH 1/2] [ruff] Allow dunder-named assignments in RUF067 (non-strict) (#22822) --- .../fixtures/ruff/RUF067/modules/__init__.py | 11 +- .../rules/ruff/rules/non_empty_init_module.rs | 123 +++--------------- ...__RUF067_RUF067__modules____init__.py.snap | 22 +++- ...s__strictly_empty_init_modules_ruf067.snap | 110 +++++++++++++--- 4 files changed, 141 insertions(+), 125 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/__init__.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/__init__.py index 7a1ae57943312..87a9479e37bf5 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/__init__.py +++ b/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 diff --git a/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs b/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs index fc28332708042..7ecac2ccaefbc 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs +++ b/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,28 @@ 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 { - 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.is_empty() + && self.targets.iter().all(|target| { + target + .as_name_expr() + .is_some_and(|name| is_dunder(name.id.as_str())) + }) } } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules____init__.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules____init__.py.snap index f901677b9b94a..e0318db25129e 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules____init__.py.snap +++ b/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__` | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__strictly_empty_init_modules_ruf067.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__strictly_empty_init_modules_ruf067.snap index dbc538d34b7db..682497466e3b8 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__strictly_empty_init_modules_ruf067.snap +++ b/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 | ^^^^^^^^^^^^^^^^^^^^^^^^^ | From 379f50bd687f4bb45dc1397adaf07e27a6b44cfb Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 30 Mar 2026 11:17:27 +0200 Subject: [PATCH 2/2] Remove unnecessary `is_empty` check --- .../src/rules/ruff/rules/non_empty_init_module.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs b/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs index 7ecac2ccaefbc..ff51b046438fa 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs @@ -168,11 +168,10 @@ impl<'a> Assignment<'a> { /// 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.is_empty() - && self.targets.iter().all(|target| { - target - .as_name_expr() - .is_some_and(|name| is_dunder(name.id.as_str())) - }) + self.targets.iter().all(|target| { + target + .as_name_expr() + .is_some_and(|name| is_dunder(name.id.as_str())) + }) } }