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",