From c483b59dddb5d481beb5dbecd481ca0f695378c4 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Tue, 30 Dec 2025 11:32:10 -0500 Subject: [PATCH] [`ruff`] Add `non-empty-init-module` (`RUF067`) (#22143) Summary -- This PR adds a new rule, `non-empty-init-module`, which restricts the kind of code that can be included in an `__init__.py` file. By default, docstrings, imports, and assignments to `__all__` are allowed. When the new configuration option `lint.ruff.strictly-empty-init-modules` is enabled, no code at all is allowed. This closes #9848, where these two variants correspond to different rules in the [`flake8-empty-init-modules`](https://github.com/samueljsb/flake8-empty-init-modules/) linter. The upstream rules are EIM001, which bans all code, and EIM002, which bans non-import/docstring/`__all__` code. Since we discussed folding these into one rule on [Discord], I just added the rule to the `RUF` group instead of adding a new `EIM` plugin. I'm not really sure we need to flag docstrings even when the strict setting is enabled, but I just followed upstream for now. Similarly, as I noted in a TODO comment, we could also allow more statements involving `__all__`, such as `__all__.append(...)` or `__all__.extend(...)`. The current version only allows assignments, like upstream, as well as annotated and augmented assignments, unlike upstream. I think when we discussed this previously, we considered flagging the module itself as containing code, but for now I followed the upstream implementation of flagging each statement in the module that breaks the rule (actually the upstream linter flags each _line_, including comments). This will obviously be a bit noisier, emitting many diagnostics for the same module. But this also seems preferable because it flags every statement that needs to be fixed up front instead of only emitting one diagnostic for the whole file that persists as you keep removing more lines. It was also easy to implement in `analyze::statement` without a separate visitor. The first commit adds the rule and baseline tests, the second commit adds the option and a diff test showing the additional diagnostics when the setting is enabled. I noticed a small (~2%) performance regression on our largest benchmark, so I also added a cached `Checker::in_init_module` field and method instead of the `Checker::path` method. This was almost the only reason for the `Checker::path` field at all, but there's one remaining reference in a `warn_user!` [call](https://github.com/astral-sh/ruff/blob/main/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs#L188). Test Plan -- New tests adapted from the upstream linter ## Ecosystem Report I've spot-checked the ecosystem report, and the results look "correct." This is obviously a very noisy rule if you do include code in `__init__.py` files. We could make it less noisy by adding more exceptions (e.g. allowing `if TYPE_CHECKING` blocks, allowing `__getattr__` functions, allowing imports from `importlib` assignments), but I'm sort of inclined just to start simple and see what users need. [Discord]: https://discord.com/channels/1039017663004942429/1082324250112823306/1440086001035771985 --------- Co-authored-by: Micha Reiser --- ...ires_python_extend_from_shared_config.snap | 1 + .../cli__lint__requires_python_no_tool.snap | 1 + ...quires_python_no_tool_preview_enabled.snap | 1 + ...ython_no_tool_target_version_override.snap | 1 + ..._requires_python_pyproject_toml_above.snap | 1 + ...python_pyproject_toml_above_with_tool.snap | 1 + ...nt__requires_python_ruff_toml_above-2.snap | 1 + ...lint__requires_python_ruff_toml_above.snap | 1 + ...s_python_ruff_toml_no_target_fallback.snap | 1 + ...ow_settings__display_default_settings.snap | 1 + .../fixtures/ruff/RUF067/modules/__init__.py | 51 +++ .../test/fixtures/ruff/RUF067/modules/okay.py | 54 +++ .../src/checkers/ast/analyze/statement.rs | 3 + .../ast/analyze/unresolved_references.rs | 2 +- crates/ruff_linter/src/checkers/ast/mod.rs | 15 +- crates/ruff_linter/src/codes.rs | 1 + .../src/rules/pyflakes/rules/unused_import.rs | 2 +- .../pylint/rules/useless_import_alias.rs | 4 +- crates/ruff_linter/src/rules/ruff/mod.rs | 26 ++ .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../rules/ruff/rules/non_empty_init_module.rs | 259 +++++++++++++ crates/ruff_linter/src/rules/ruff/settings.rs | 2 + ...__RUF067_RUF067__modules____init__.py.snap | 53 +++ ...ests__RUF067_RUF067__modules__okay.py.snap | 4 + ...s__strictly_empty_init_modules_ruf067.snap | 344 ++++++++++++++++++ crates/ruff_workspace/src/options.rs | 12 + ruff.schema.json | 8 + 27 files changed, 843 insertions(+), 9 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/__init__.py create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/okay.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules____init__.py.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules__okay.py.snap create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__strictly_empty_init_modules_ruf067.snap diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_extend_from_shared_config.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_extend_from_shared_config.snap index 18e47274f8..d937287741 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_extend_from_shared_config.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_extend_from_shared_config.snap @@ -261,6 +261,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool.snap index c4c3502fb7..d417cb1a07 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool.snap @@ -263,6 +263,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_preview_enabled.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_preview_enabled.snap index cb18b61d4d..dbb8144c8f 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_preview_enabled.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_preview_enabled.snap @@ -265,6 +265,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_target_version_override.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_target_version_override.snap index fa5f26f762..6dfab2a36f 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_target_version_override.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_no_tool_target_version_override.snap @@ -265,6 +265,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above.snap index 8db9b7a64e..e372f3b7e6 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above.snap @@ -262,6 +262,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above_with_tool.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above_with_tool.snap index e5e110fab2..0e920cc3bc 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above_with_tool.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_pyproject_toml_above_with_tool.snap @@ -262,6 +262,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above-2.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above-2.snap index 4c3acae10d..9af331105e 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above-2.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above-2.snap @@ -261,6 +261,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above.snap index 0cfa48e35c..d15fba3b5d 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_above.snap @@ -261,6 +261,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_no_target_fallback.snap b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_no_target_fallback.snap index 1d4b6171a7..a8d9819854 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_no_target_fallback.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__requires_python_ruff_toml_no_target_fallback.snap @@ -261,6 +261,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] diff --git a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap index 2a7779d137..ec36549bcd 100644 --- a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap +++ b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap @@ -374,6 +374,7 @@ linter.pylint.max_locals = 15 linter.pylint.max_nested_blocks = 5 linter.pyupgrade.keep_runtime_typing = false linter.ruff.parenthesize_tuple_in_subscript = false +linter.ruff.strictly_empty_init_modules = false # Formatter Settings formatter.exclude = [] 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 new file mode 100644 index 0000000000..7a1ae57943 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/__init__.py @@ -0,0 +1,51 @@ +"""This is the module docstring.""" + +# convenience imports: +import os +from pathlib import Path + +__all__ = ["MY_CONSTANT"] +__all__ += ["foo"] +__all__: list[str] = __all__ +__all__ = __all__ = __all__ + +MY_CONSTANT = 5 +"""This is an important constant.""" + +os.environ["FOO"] = 1 + + +def foo(): + return Path("foo.py") + +def __getattr__(name): # ok + return name + +__path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok + +if os.environ["FOO"] != "1": # RUF067 + MY_CONSTANT = 4 # ok, don't flag nested statements + +if TYPE_CHECKING: # ok + MY_CONSTANT = 3 + +import typing + +if typing.TYPE_CHECKING: # ok + MY_CONSTANT = 2 + +__version__ = "1.2.3" # ok + +def __dir__(): # ok + return ["foo"] + +import pkgutil + +__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 + +# also allow `__author__` +__author__ = "The Author" # ok diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/okay.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/okay.py new file mode 100644 index 0000000000..f744b50aa5 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF067/modules/okay.py @@ -0,0 +1,54 @@ +""" +The code here is not in an `__init__.py` file and should not trigger the +lint. +""" + +# convenience imports: +import os +from pathlib import Path + +__all__ = ["MY_CONSTANT"] +__all__ += ["foo"] +__all__: list[str] = __all__ +__all__ = __all__ = __all__ + +MY_CONSTANT = 5 +"""This is an important constant.""" + +os.environ["FOO"] = 1 + + +def foo(): + return Path("foo.py") + +def __getattr__(name): # ok + return name + +__path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok + +if os.environ["FOO"] != "1": # RUF067 + MY_CONSTANT = 4 # ok, don't flag nested statements + +if TYPE_CHECKING: # ok + MY_CONSTANT = 3 + +import typing + +if typing.TYPE_CHECKING: # ok + MY_CONSTANT = 2 + +__version__ = "1.2.3" # ok + +def __dir__(): # ok + return ["foo"] + +import pkgutil + +__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 + +# also allow `__author__` +__author__ = "The Author" # ok diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index e1b4da60f2..4b0e39bab8 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1630,4 +1630,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } _ => {} } + if checker.is_rule_enabled(Rule::NonEmptyInitModule) { + ruff::rules::non_empty_init_module(checker, stmt); + } } diff --git a/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs b/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs index 4a6764a119..fd781b8452 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/unresolved_references.rs @@ -33,7 +33,7 @@ pub(crate) fn unresolved_references(checker: &Checker) { } // Allow __path__. - if checker.path.ends_with("__init__.py") { + if checker.in_init_module() { if reference.name(checker.source()) == "__path__" { continue; } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 37422cbf18..be8bc4b275 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -21,7 +21,7 @@ //! represents the lint-rule analysis phase. In the future, these steps may be separated into //! distinct passes over the AST. -use std::cell::RefCell; +use std::cell::{OnceCell, RefCell}; use std::path::Path; use itertools::Itertools; @@ -198,6 +198,8 @@ pub(crate) struct Checker<'a> { parsed_type_annotation: Option<&'a ParsedAnnotation>, /// The [`Path`] to the file under analysis. path: &'a Path, + /// Whether `path` points to an `__init__.py` file. + in_init_module: OnceCell, /// The [`Path`] to the package containing the current file. package: Option>, /// The module representation of the current file (e.g., `foo.bar`). @@ -274,6 +276,7 @@ impl<'a> Checker<'a> { noqa_line_for, noqa, path, + in_init_module: OnceCell::new(), package, module, source_type, @@ -482,9 +485,11 @@ impl<'a> Checker<'a> { self.context.settings } - /// The [`Path`] to the file under analysis. - pub(crate) const fn path(&self) -> &'a Path { - self.path + /// Returns whether the file under analysis is an `__init__.py` file. + pub(crate) fn in_init_module(&self) -> bool { + *self + .in_init_module + .get_or_init(|| self.path.ends_with("__init__.py")) } /// The [`Path`] to the package containing the current file. @@ -3171,7 +3176,7 @@ impl<'a> Checker<'a> { // F822 if self.is_rule_enabled(Rule::UndefinedExport) { if is_undefined_export_in_dunder_init_enabled(self.settings()) - || !self.path.ends_with("__init__.py") + || !self.in_init_module() { self.report_diagnostic( pyflakes::rules::UndefinedExport { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 74cb38bddb..b33ab5b521 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1060,6 +1060,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "064") => rules::ruff::rules::NonOctalPermissions, (Ruff, "065") => rules::ruff::rules::LoggingEagerConversion, (Ruff, "066") => rules::ruff::rules::PropertyWithoutReturn, + (Ruff, "067") => rules::ruff::rules::NonEmptyInitModule, (Ruff, "100") => rules::ruff::rules::UnusedNOQA, (Ruff, "101") => rules::ruff::rules::RedirectedNOQA, diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs index 88936e22ab..af1ae56a0d 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -389,7 +389,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope) { } } - let in_init = checker.path().ends_with("__init__.py"); + let in_init = checker.in_init_module(); let fix_init = !checker.settings().ignore_init_module_imports; let preview_mode = is_dunder_init_fix_unused_import_enabled(checker.settings()); let dunder_all_exprs = find_dunder_all_exprs(checker.semantic()); diff --git a/crates/ruff_linter/src/rules/pylint/rules/useless_import_alias.rs b/crates/ruff_linter/src/rules/pylint/rules/useless_import_alias.rs index 118dbb880e..6b45d9cc64 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/useless_import_alias.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/useless_import_alias.rs @@ -77,7 +77,7 @@ pub(crate) fn useless_import_alias(checker: &Checker, alias: &Alias) { } // A re-export in __init__.py is probably intentional. - if checker.path().ends_with("__init__.py") { + if checker.in_init_module() { return; } @@ -116,7 +116,7 @@ pub(crate) fn useless_import_from_alias( } // A re-export in __init__.py is probably intentional. - if checker.path().ends_with("__init__.py") { + if checker.in_init_module() { return; } diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 9bc6363731..48c6f7c80c 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -119,6 +119,8 @@ mod tests { #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))] #[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))] + #[test_case(Rule::NonEmptyInitModule, Path::new("RUF067/modules/__init__.py"))] + #[test_case(Rule::NonEmptyInitModule, Path::new("RUF067/modules/okay.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( @@ -136,6 +138,7 @@ mod tests { &LinterSettings { ruff: super::settings::Settings { parenthesize_tuple_in_subscript: true, + ..super::settings::Settings::default() }, ..LinterSettings::for_rule(Rule::IncorrectlyParenthesizedTupleInSubscript) }, @@ -151,6 +154,7 @@ mod tests { &LinterSettings { ruff: super::settings::Settings { parenthesize_tuple_in_subscript: false, + ..super::settings::Settings::default() }, unresolved_target_version: PythonVersion::PY310.into(), ..LinterSettings::for_rule(Rule::IncorrectlyParenthesizedTupleInSubscript) @@ -714,4 +718,26 @@ mod tests { assert_diagnostics!(snapshot, diagnostics); Ok(()) } + + #[test] + fn strictly_empty_init_modules_ruf067() -> Result<()> { + assert_diagnostics_diff!( + Path::new("ruff/RUF067/modules/__init__.py"), + &LinterSettings { + ruff: super::settings::Settings { + strictly_empty_init_modules: false, + ..super::settings::Settings::default() + }, + ..LinterSettings::for_rule(Rule::NonEmptyInitModule) + }, + &LinterSettings { + ruff: super::settings::Settings { + strictly_empty_init_modules: true, + ..super::settings::Settings::default() + }, + ..LinterSettings::for_rule(Rule::NonEmptyInitModule) + }, + ); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 70363d3f8b..61c43b5af2 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -32,6 +32,7 @@ pub(crate) use mutable_dataclass_default::*; pub(crate) use mutable_fromkeys_value::*; pub(crate) use needless_else::*; pub(crate) use never_union::*; +pub(crate) use non_empty_init_module::*; pub(crate) use non_octal_permissions::*; pub(crate) use none_not_at_end_of_union::*; pub(crate) use parenthesize_chained_operators::*; @@ -99,6 +100,7 @@ mod mutable_dataclass_default; mod mutable_fromkeys_value; mod needless_else; mod never_union; +mod non_empty_init_module; mod non_octal_permissions; mod none_not_at_end_of_union; mod parenthesize_chained_operators; 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 new file mode 100644 index 0000000000..fc28332708 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/non_empty_init_module.rs @@ -0,0 +1,259 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; +use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_python_semantic::analyze::typing::is_type_checking_block; +use ruff_text_size::Ranged; + +use crate::{Violation, checkers::ast::Checker}; + +/// ## What it does +/// +/// Detects the presence of code in `__init__.py` files. +/// +/// ## Why is this bad? +/// +/// `__init__.py` files are often empty or only contain simple code to modify a module's API. As +/// such, it's easy to overlook them and their possible side effects when debugging. +/// +/// ## Example +/// +/// Instead of defining `MyClass` directly in `__init__.py`: +/// +/// ```python +/// """My module docstring.""" +/// +/// +/// class MyClass: +/// def my_method(self): ... +/// ``` +/// +/// move the definition to another file, import it, and include it in `__all__`: +/// +/// ```python +/// """My module docstring.""" +/// +/// from submodule import MyClass +/// +/// __all__ = ["MyClass"] +/// ``` +/// +/// Code in `__init__.py` files is also run at import time and can cause surprising slowdowns. To +/// disallow any code in `__init__.py` files, you can enable the +/// [`lint.ruff.strictly-empty-init-modules`] setting. In this case: +/// +/// ```python +/// from submodule import MyClass +/// +/// __all__ = ["MyClass"] +/// ``` +/// +/// the only fix is entirely emptying the file: +/// +/// ```python +/// ``` +/// +/// ## Details +/// +/// In non-strict mode, this rule allows several common patterns in `__init__.py` files: +/// +/// - Imports +/// - Assignments to `__all__`, `__path__`, `__version__`, and `__author__` +/// - Module-level and attribute docstrings +/// - `if TYPE_CHECKING` blocks +/// - [PEP-562] module-level `__getattr__` and `__dir__` functions +/// +/// ## Options +/// +/// - [`lint.ruff.strictly-empty-init-modules`] +/// +/// ## References +/// +/// - [`flake8-empty-init-modules`](https://github.com/samueljsb/flake8-empty-init-modules/) +/// +/// [PEP-562]: https://peps.python.org/pep-0562/ +#[derive(ViolationMetadata)] +#[violation_metadata(preview_since = "0.14.11")] +pub(crate) struct NonEmptyInitModule { + strictly_empty_init_modules: bool, +} + +impl Violation for NonEmptyInitModule { + #[derive_message_formats] + fn message(&self) -> String { + if self.strictly_empty_init_modules { + "`__init__` module should not contain any code".to_string() + } else { + "`__init__` module should only contain docstrings and re-exports".to_string() + } + } +} + +/// RUF067 +pub(crate) fn non_empty_init_module(checker: &Checker, stmt: &Stmt) { + if !checker.in_init_module() { + return; + } + + let semantic = checker.semantic(); + + // Only flag top-level statements + if !semantic.at_top_level() { + return; + } + + let strictly_empty_init_modules = checker.settings().ruff.strictly_empty_init_modules; + + if !strictly_empty_init_modules { + // Even though module-level attributes are disallowed, we still allow attribute docstrings + // to avoid needing two `noqa` comments in a case like: + // + // ```py + // MY_CONSTANT = 1 # noqa: RUF067 + // "A very important constant" + // ``` + if semantic.in_pep_257_docstring() || semantic.in_attribute_docstring() { + return; + } + + match stmt { + // Allow imports + Stmt::Import(_) | Stmt::ImportFrom(_) => return, + + // Allow PEP-562 module `__getattr__` and `__dir__` + Stmt::FunctionDef(func) if matches!(&*func.name, "__getattr__" | "__dir__") => return, + + // Allow `TYPE_CHECKING` blocks + Stmt::If(stmt_if) if is_type_checking_block(stmt_if, semantic) => return, + + _ => {} + } + + 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__") { + return; + } + } + } + + checker.report_diagnostic( + NonEmptyInitModule { + strictly_empty_init_modules, + }, + stmt.range(), + ); +} + +/// Any assignment statement, including plain assignment, annotated assignments, and augmented +/// 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)) + } + _ => return None, + }; + + Some(Self { targets, value }) + } + + /// 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__" + } +} diff --git a/crates/ruff_linter/src/rules/ruff/settings.rs b/crates/ruff_linter/src/rules/ruff/settings.rs index c6768121f0..f20daa2001 100644 --- a/crates/ruff_linter/src/rules/ruff/settings.rs +++ b/crates/ruff_linter/src/rules/ruff/settings.rs @@ -7,6 +7,7 @@ use std::fmt; #[derive(Debug, Clone, CacheKey, Default)] pub struct Settings { pub parenthesize_tuple_in_subscript: bool, + pub strictly_empty_init_modules: bool, } impl fmt::Display for Settings { @@ -16,6 +17,7 @@ impl fmt::Display for Settings { namespace = "linter.ruff", fields = [ self.parenthesize_tuple_in_subscript, + self.strictly_empty_init_modules, ] } Ok(()) 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 new file mode 100644 index 0000000000..f901677b9b --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules____init__.py.snap @@ -0,0 +1,53 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:12:1 + | +10 | __all__ = __all__ = __all__ +11 | +12 | MY_CONSTANT = 5 + | ^^^^^^^^^^^^^^^ +13 | """This is an important constant.""" + | + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:15:1 + | +13 | """This is an important constant.""" +14 | +15 | os.environ["FOO"] = 1 + | ^^^^^^^^^^^^^^^^^^^^^ + | + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:18:1 + | +18 | / def foo(): +19 | | return Path("foo.py") + | |_________________________^ +20 | +21 | def __getattr__(name): # ok + | + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:26:1 + | +24 | __path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok +25 | +26 | / if os.environ["FOO"] != "1": # RUF067 +27 | | MY_CONSTANT = 4 # ok, don't flag nested statements + | |___________________^ +28 | +29 | if TYPE_CHECKING: # ok + | + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:48:1 + | +47 | # non-`extend_path` assignments are not allowed +48 | __path__ = 5 # RUF067 + | ^^^^^^^^^^^^ +49 | +50 | # also allow `__author__` + | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules__okay.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules__okay.py.snap new file mode 100644 index 0000000000..7f58cfd724 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF067_RUF067__modules__okay.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- + 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 new file mode 100644 index 0000000000..dbc538d34b --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__strictly_empty_init_modules_ruf067.snap @@ -0,0 +1,344 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +--- Linter settings --- +-linter.ruff.strictly_empty_init_modules = false ++linter.ruff.strictly_empty_init_modules = true + +--- Summary --- +Removed: 5 +Added: 24 + +--- Removed --- +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:12:1 + | +10 | __all__ = __all__ = __all__ +11 | +12 | MY_CONSTANT = 5 + | ^^^^^^^^^^^^^^^ +13 | """This is an important constant.""" + | + + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:15:1 + | +13 | """This is an important constant.""" +14 | +15 | os.environ["FOO"] = 1 + | ^^^^^^^^^^^^^^^^^^^^^ + | + + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:18:1 + | +18 | / def foo(): +19 | | return Path("foo.py") + | |_________________________^ +20 | +21 | def __getattr__(name): # ok + | + + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:26:1 + | +24 | __path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok +25 | +26 | / if os.environ["FOO"] != "1": # RUF067 +27 | | MY_CONSTANT = 4 # ok, don't flag nested statements + | |___________________^ +28 | +29 | if TYPE_CHECKING: # ok + | + + +RUF067 `__init__` module should only contain docstrings and re-exports + --> __init__.py:48:1 + | +47 | # non-`extend_path` assignments are not allowed +48 | __path__ = 5 # RUF067 + | ^^^^^^^^^^^^ +49 | +50 | # also allow `__author__` + | + + + +--- Added --- +RUF067 `__init__` module should not contain any code + --> __init__.py:1:1 + | +1 | """This is the module docstring.""" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +2 | +3 | # convenience imports: + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:4:1 + | +3 | # convenience imports: +4 | import os + | ^^^^^^^^^ +5 | from pathlib import Path + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:5:1 + | +3 | # convenience imports: +4 | import os +5 | from pathlib import Path + | ^^^^^^^^^^^^^^^^^^^^^^^^ +6 | +7 | __all__ = ["MY_CONSTANT"] + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:7:1 + | +5 | from pathlib import Path +6 | +7 | __all__ = ["MY_CONSTANT"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ +8 | __all__ += ["foo"] +9 | __all__: list[str] = __all__ + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:8:1 + | + 7 | __all__ = ["MY_CONSTANT"] + 8 | __all__ += ["foo"] + | ^^^^^^^^^^^^^^^^^^ + 9 | __all__: list[str] = __all__ +10 | __all__ = __all__ = __all__ + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:9:1 + | + 7 | __all__ = ["MY_CONSTANT"] + 8 | __all__ += ["foo"] + 9 | __all__: list[str] = __all__ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +10 | __all__ = __all__ = __all__ + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:10:1 + | + 8 | __all__ += ["foo"] + 9 | __all__: list[str] = __all__ +10 | __all__ = __all__ = __all__ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +11 | +12 | MY_CONSTANT = 5 + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:12:1 + | +10 | __all__ = __all__ = __all__ +11 | +12 | MY_CONSTANT = 5 + | ^^^^^^^^^^^^^^^ +13 | """This is an important constant.""" + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:13:1 + | +12 | MY_CONSTANT = 5 +13 | """This is an important constant.""" + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +14 | +15 | os.environ["FOO"] = 1 + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:15:1 + | +13 | """This is an important constant.""" +14 | +15 | os.environ["FOO"] = 1 + | ^^^^^^^^^^^^^^^^^^^^^ + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:18:1 + | +18 | / def foo(): +19 | | return Path("foo.py") + | |_________________________^ +20 | +21 | def __getattr__(name): # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:21:1 + | +19 | return Path("foo.py") +20 | +21 | / def __getattr__(name): # ok +22 | | return name + | |_______________^ +23 | +24 | __path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:24:1 + | +22 | return name +23 | +24 | __path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +25 | +26 | if os.environ["FOO"] != "1": # RUF067 + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:26:1 + | +24 | __path__ = __import__('pkgutil').extend_path(__path__, __name__) # ok +25 | +26 | / if os.environ["FOO"] != "1": # RUF067 +27 | | MY_CONSTANT = 4 # ok, don't flag nested statements + | |___________________^ +28 | +29 | if TYPE_CHECKING: # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:29:1 + | +27 | MY_CONSTANT = 4 # ok, don't flag nested statements +28 | +29 | / if TYPE_CHECKING: # ok +30 | | MY_CONSTANT = 3 + | |___________________^ +31 | +32 | import typing + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:32:1 + | +30 | MY_CONSTANT = 3 +31 | +32 | import typing + | ^^^^^^^^^^^^^ +33 | +34 | if typing.TYPE_CHECKING: # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:34:1 + | +32 | import typing +33 | +34 | / if typing.TYPE_CHECKING: # ok +35 | | MY_CONSTANT = 2 + | |___________________^ +36 | +37 | __version__ = "1.2.3" # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:37:1 + | +35 | MY_CONSTANT = 2 +36 | +37 | __version__ = "1.2.3" # ok + | ^^^^^^^^^^^^^^^^^^^^^ +38 | +39 | def __dir__(): # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:39:1 + | +37 | __version__ = "1.2.3" # ok +38 | +39 | / def __dir__(): # ok +40 | | return ["foo"] + | |__________________^ +41 | +42 | import pkgutil + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:42:1 + | +40 | return ["foo"] +41 | +42 | import pkgutil + | ^^^^^^^^^^^^^^ +43 | +44 | __path__ = pkgutil.extend_path(__path__, __name__) # ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:44:1 + | +42 | import pkgutil +43 | +44 | __path__ = pkgutil.extend_path(__path__, __name__) # ok + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +45 | __path__ = unknown.extend_path(__path__, __name__) # also ok + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:45:1 + | +44 | __path__ = pkgutil.extend_path(__path__, __name__) # ok +45 | __path__ = unknown.extend_path(__path__, __name__) # also ok + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +46 | +47 | # non-`extend_path` assignments are not allowed + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:48:1 + | +47 | # non-`extend_path` assignments are not allowed +48 | __path__ = 5 # RUF067 + | ^^^^^^^^^^^^ +49 | +50 | # also allow `__author__` + | + + +RUF067 `__init__` module should not contain any code + --> __init__.py:51:1 + | +50 | # also allow `__author__` +51 | __author__ = "The Author" # ok + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index eaf557469e..087914d8f8 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -3497,6 +3497,17 @@ pub struct RuffOptions { note = "The `allowed-markup-names` option has been moved to the `flake8-bandit` section of the configuration." )] pub allowed_markup_calls: Option>, + /// Whether to require `__init__.py` files to contain no code at all, including imports and + /// docstrings (see `RUF067`). + #[option( + default = r#"false"#, + value_type = "bool", + example = r#" + # Make it a violation to include any code, including imports and docstrings in `__init__.py` + strictly-empty-init-modules = true + "# + )] + pub strictly_empty_init_modules: Option, } impl RuffOptions { @@ -3505,6 +3516,7 @@ impl RuffOptions { parenthesize_tuple_in_subscript: self .parenthesize_tuple_in_subscript .unwrap_or_default(), + strictly_empty_init_modules: self.strictly_empty_init_modules.unwrap_or_default(), } } } diff --git a/ruff.schema.json b/ruff.schema.json index 5af8837779..79e96ca138 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2926,6 +2926,13 @@ "boolean", "null" ] + }, + "strictly-empty-init-modules": { + "description": "Whether to require `__init__.py` files to contain no code at all, including imports and\ndocstrings (see `RUF067`).", + "type": [ + "boolean", + "null" + ] } }, "additionalProperties": false @@ -4046,6 +4053,7 @@ "RUF064", "RUF065", "RUF066", + "RUF067", "RUF1", "RUF10", "RUF100",