mirror of
https://github.com/astral-sh/ruff.git
synced 2025-12-23 09:19:58 +00:00
[ruff] Add non-empty-init-module (RUF070)
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.
Test Plan
--
New tests adapted from the upstream linter
[Discord]: 1440086001
This commit is contained in:
parent
816b19c4a6
commit
af5faaacf4
9 changed files with 178 additions and 0 deletions
19
crates/ruff_linter/resources/test/fixtures/ruff/RUF070/modules/__init__.py
vendored
Normal file
19
crates/ruff_linter/resources/test/fixtures/ruff/RUF070/modules/__init__.py
vendored
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
"""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")
|
||||
22
crates/ruff_linter/resources/test/fixtures/ruff/RUF070/modules/okay.py
vendored
Normal file
22
crates/ruff_linter/resources/test/fixtures/ruff/RUF070/modules/okay.py
vendored
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
"""
|
||||
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")
|
||||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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, "070") => rules::ruff::rules::NonEmptyInitModule,
|
||||
|
||||
(Ruff, "100") => rules::ruff::rules::UnusedNOQA,
|
||||
(Ruff, "101") => rules::ruff::rules::RedirectedNOQA,
|
||||
|
|
|
|||
|
|
@ -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("RUF070/modules/__init__.py"))]
|
||||
#[test_case(Rule::NonEmptyInitModule, Path::new("RUF070/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(
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,96 @@
|
|||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast as ast;
|
||||
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?
|
||||
///
|
||||
/// Including code in `__init__.py` files can cause surprising slowdowns because the code is run at
|
||||
/// import time. `__init__.py` files are also often empty, so it's easy to overlook them when
|
||||
/// debugging these issues.
|
||||
///
|
||||
/// ## Example
|
||||
///
|
||||
/// ```python
|
||||
/// """My module docstring."""
|
||||
///
|
||||
/// class MyClass:
|
||||
/// def my_method(self):
|
||||
/// ...
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
///
|
||||
/// ```python
|
||||
/// """My module docstring."""
|
||||
///
|
||||
/// from submodule import MyClass
|
||||
///
|
||||
/// __all__ = ["MyClass"]
|
||||
/// ```
|
||||
///
|
||||
/// ## References
|
||||
/// - [`flake8-empty-init-modules`](https://github.com/samueljsb/flake8-empty-init-modules/)
|
||||
#[derive(ViolationMetadata)]
|
||||
#[violation_metadata(preview_since = "0.14.11")]
|
||||
pub(crate) struct NonEmptyInitModule;
|
||||
|
||||
impl Violation for NonEmptyInitModule {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
"Code detected".to_string()
|
||||
}
|
||||
}
|
||||
|
||||
/// RUF070
|
||||
pub(crate) fn non_empty_init_module(checker: &Checker, stmt: &ast::Stmt) {
|
||||
if !checker.path().ends_with("__init__.py") {
|
||||
return;
|
||||
}
|
||||
|
||||
// Only flag top-level statements
|
||||
if !checker.semantic().current_scope().kind.is_module() {
|
||||
return;
|
||||
}
|
||||
|
||||
if checker.semantic().in_pep_257_docstring() || checker.semantic().in_attribute_docstring() {
|
||||
return;
|
||||
}
|
||||
|
||||
if matches!(stmt, ast::Stmt::Import(_) | ast::Stmt::ImportFrom(_)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// 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`.
|
||||
let targets = match stmt {
|
||||
ast::Stmt::Assign(ast::StmtAssign { targets, .. }) => targets.as_slice(),
|
||||
ast::Stmt::AnnAssign(ast::StmtAnnAssign { target, .. })
|
||||
| ast::Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => {
|
||||
std::slice::from_ref(&**target)
|
||||
}
|
||||
_ => &[],
|
||||
};
|
||||
|
||||
if !targets.is_empty()
|
||||
&& targets.iter().all(|target| {
|
||||
target
|
||||
.as_name_expr()
|
||||
.is_some_and(|name| name.id == "__all__")
|
||||
})
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
checker.report_diagnostic(NonEmptyInitModule, stmt.range());
|
||||
}
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/ruff/mod.rs
|
||||
---
|
||||
RUF070 Code detected
|
||||
--> __init__.py:12:1
|
||||
|
|
||||
10 | __all__ = __all__ = __all__
|
||||
11 |
|
||||
12 | MY_CONSTANT = 5
|
||||
| ^^^^^^^^^^^^^^^
|
||||
13 | """This is an important constant."""
|
||||
|
|
||||
|
||||
RUF070 Code detected
|
||||
--> __init__.py:15:1
|
||||
|
|
||||
13 | """This is an important constant."""
|
||||
14 |
|
||||
15 | os.environ["FOO"] = 1
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
|
||||
RUF070 Code detected
|
||||
--> __init__.py:18:1
|
||||
|
|
||||
18 | / def foo():
|
||||
19 | | return Path("foo.py")
|
||||
| |_________________________^
|
||||
|
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/ruff/mod.rs
|
||||
---
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue