mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 18:58:04 +00:00
[pylint
] Implement import-private-name
(C2701
) (#5920)
## Summary Implements [`import-private-name` (`C2701`)](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/import-private-name.html) as `import-private-name` (`PLC2701`). Includes documentation. Related to #970. Closes https://github.com/astral-sh/ruff/issues/9138. ### PEP 420 namespace package limitation `checker.module_path` doesn't seem to support automatic detection of namespace packages (PEP 420). This leads to 'false' positives (Pylint allows both). Currently, for this to work like Pylint, users would have to [manually input known namespace packages](https://beta.ruff.rs/docs/settings/#namespace-packages). ## Test Plan `cargo test`
This commit is contained in:
parent
7ef7e0ddb6
commit
2b605527bd
16 changed files with 268 additions and 0 deletions
0
crates/ruff_linter/__init__.py
Normal file
0
crates/ruff_linter/__init__.py
Normal file
0
crates/ruff_linter/resources/__init__.py
Normal file
0
crates/ruff_linter/resources/__init__.py
Normal file
0
crates/ruff_linter/resources/test/__init__.py
Normal file
0
crates/ruff_linter/resources/test/__init__.py
Normal file
0
crates/ruff_linter/resources/test/fixtures/__init__.py
vendored
Normal file
0
crates/ruff_linter/resources/test/fixtures/__init__.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/__init__.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/__init__.py
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
_top_level_secret = 0
|
|
@ -0,0 +1,3 @@
|
|||
_sibling_submodule_secret = 1
|
||||
_another_sibling_submodule_secret = 2
|
||||
some_value = 3
|
1
crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__init__.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__init__.py
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
_submodule_secret = 1
|
46
crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py
vendored
Normal file
46
crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py
vendored
Normal file
|
@ -0,0 +1,46 @@
|
|||
# Errors.
|
||||
from _a import b
|
||||
from c._d import e
|
||||
from _f.g import h
|
||||
from i import _j
|
||||
from k import _l as m
|
||||
import _aaa
|
||||
import bbb._ccc
|
||||
|
||||
# Non-errors.
|
||||
import n
|
||||
import o as _p
|
||||
from q import r
|
||||
from s import t as _v
|
||||
from w.x import y
|
||||
from z.aa import bb as _cc
|
||||
from .dd import _ee # Relative import.
|
||||
from .ff._gg import hh # Relative import.
|
||||
from ._ii.jj import kk # Relative import.
|
||||
from __future__ import annotations # __future__ is a special case.
|
||||
from __main__ import main # __main__ is a special case.
|
||||
from ll import __version__ # __version__ is a special case.
|
||||
from import_private_name import _top_level_secret # Can import from self.
|
||||
from import_private_name.submodule import _submodule_secret # Can import from self.
|
||||
from import_private_name.submodule.subsubmodule import (
|
||||
_subsubmodule_secret,
|
||||
) # Can import from self.
|
||||
|
||||
# Non-errors (used for type annotations).
|
||||
from mm import _nn
|
||||
from oo import _pp as qq
|
||||
from _rr import ss
|
||||
from tt._uu import vv
|
||||
from _ww.xx import yy as zz
|
||||
import _ddd as ddd
|
||||
|
||||
some_variable: _nn = None
|
||||
|
||||
def func(arg: qq) -> ss:
|
||||
pass
|
||||
|
||||
class Class:
|
||||
lst: list[ddd]
|
||||
|
||||
def __init__(self, arg: vv) -> "zz":
|
||||
pass
|
|
@ -0,0 +1 @@
|
|||
_subsubmodule_secret = 42
|
|
@ -16,6 +16,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
|||
if !checker.any_enabled(&[
|
||||
Rule::AsyncioDanglingTask,
|
||||
Rule::GlobalVariableNotAssigned,
|
||||
Rule::ImportPrivateName,
|
||||
Rule::ImportShadowedByLoopVar,
|
||||
Rule::NoSelfUse,
|
||||
Rule::RedefinedArgumentFromLocal,
|
||||
|
@ -372,6 +373,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
|||
if checker.enabled(Rule::UnusedImport) {
|
||||
pyflakes::rules::unused_import(checker, scope, &mut diagnostics);
|
||||
}
|
||||
|
||||
if checker.enabled(Rule::ImportPrivateName) {
|
||||
pylint::rules::import_private_name(checker, scope, &mut diagnostics);
|
||||
}
|
||||
}
|
||||
|
||||
if scope.kind.is_function() {
|
||||
|
|
|
@ -217,6 +217,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Pylint, "C2801") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDunderCall),
|
||||
#[allow(deprecated)]
|
||||
(Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString),
|
||||
(Pylint, "C2701") => (RuleGroup::Preview, rules::pylint::rules::ImportPrivateName),
|
||||
(Pylint, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall),
|
||||
(Pylint, "E0100") => (RuleGroup::Stable, rules::pylint::rules::YieldInInit),
|
||||
(Pylint, "E0101") => (RuleGroup::Stable, rules::pylint::rules::ReturnInInit),
|
||||
|
|
|
@ -63,6 +63,10 @@ mod tests {
|
|||
Path::new("global_variable_not_assigned.py")
|
||||
)]
|
||||
#[test_case(Rule::ImportOutsideTopLevel, Path::new("import_outside_top_level.py"))]
|
||||
#[test_case(
|
||||
Rule::ImportPrivateName,
|
||||
Path::new("import_private_name/submodule/__main__.py")
|
||||
)]
|
||||
#[test_case(Rule::ImportSelf, Path::new("import_self/module.py"))]
|
||||
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
|
||||
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
|
||||
|
|
187
crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs
Normal file
187
crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs
Normal file
|
@ -0,0 +1,187 @@
|
|||
use std::borrow::Cow;
|
||||
|
||||
use itertools::Itertools;
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scope};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for import statements that import a private name (a name starting
|
||||
/// with an underscore `_`) from another module.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// [PEP 8] states that names starting with an underscore are private. Thus,
|
||||
/// they are not intended to be used outside of the module in which they are
|
||||
/// defined.
|
||||
///
|
||||
/// Further, as private imports are not considered part of the public API, they
|
||||
/// are prone to unexpected changes, especially outside of semantic versioning.
|
||||
///
|
||||
/// Instead, consider using the public API of the module.
|
||||
///
|
||||
/// This rule ignores private name imports that are exclusively used in type
|
||||
/// annotations. Ideally, types would be public; however, this is not always
|
||||
/// possible when using third-party libraries.
|
||||
///
|
||||
/// ## Known problems
|
||||
/// Does not ignore private name imports from within the module that defines
|
||||
/// the private name if the module is defined with [PEP 420] namespace packages
|
||||
/// (i.e., directories that omit the `__init__.py` file). Namespace packages
|
||||
/// must be configured via the [`namespace-packages`] setting.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// from foo import _bar
|
||||
/// ```
|
||||
///
|
||||
/// ## Options
|
||||
/// - [`namespace-packages`]: List of packages that are defined as namespace
|
||||
/// packages.
|
||||
///
|
||||
/// ## References
|
||||
/// - [PEP 8: Naming Conventions](https://peps.python.org/pep-0008/#naming-conventions)
|
||||
/// - [Semantic Versioning](https://semver.org/)
|
||||
///
|
||||
/// [PEP 8]: https://www.python.org/dev/peps/pep-0008/
|
||||
/// [PEP 420]: https://www.python.org/dev/peps/pep-0420/
|
||||
/// [`namespace-packages`]: https://beta.ruff.rs/docs/settings/#namespace-packages
|
||||
#[violation]
|
||||
pub struct ImportPrivateName {
|
||||
name: String,
|
||||
module: Option<String>,
|
||||
}
|
||||
|
||||
impl Violation for ImportPrivateName {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
let ImportPrivateName { name, module } = self;
|
||||
match module {
|
||||
Some(module) => {
|
||||
format!("Private name import `{name}` from external module `{module}`")
|
||||
}
|
||||
None => format!("Private name import `{name}`"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// PLC2701
|
||||
pub(crate) fn import_private_name(
|
||||
checker: &Checker,
|
||||
scope: &Scope,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) {
|
||||
for binding_id in scope.binding_ids() {
|
||||
let binding = checker.semantic().binding(binding_id);
|
||||
let Some(import) = binding.as_any_import() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let import_info = match import {
|
||||
import if import.is_import() => ImportInfo::from(import.import().unwrap()),
|
||||
import if import.is_from_import() => ImportInfo::from(import.from_import().unwrap()),
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let Some(root_module) = import_info.module_name.first() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
// Relative imports are not a public API.
|
||||
// Ex) `from . import foo`
|
||||
if import_info.module_name.starts_with(&["."]) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// We can also ignore dunder names.
|
||||
// Ex) `from __future__ import annotations`
|
||||
// Ex) `from foo import __version__`
|
||||
if root_module.starts_with("__") || import_info.member_name.starts_with("__") {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Ignore private imports from the same module.
|
||||
// Ex) `from foo import _bar` within `foo/baz.py`
|
||||
if checker
|
||||
.package()
|
||||
.is_some_and(|path| path.ends_with(root_module))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
// Ignore public imports; require at least one private name.
|
||||
// Ex) `from foo import bar`
|
||||
let Some((index, private_name)) = import_info
|
||||
.call_path
|
||||
.iter()
|
||||
.find_position(|name| name.starts_with('_'))
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
// Ignore private imports used exclusively for typing.
|
||||
if !binding.references.is_empty()
|
||||
&& binding
|
||||
.references()
|
||||
.map(|reference_id| checker.semantic().reference(reference_id))
|
||||
.all(is_typing)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
let name = (*private_name).to_string();
|
||||
let module = if index > 0 {
|
||||
Some(import_info.module_name[..index].join("."))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
diagnostics.push(Diagnostic::new(
|
||||
ImportPrivateName { name, module },
|
||||
binding.range(),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns `true` if the [`ResolvedReference`] is in a typing context.
|
||||
fn is_typing(reference: &ResolvedReference) -> bool {
|
||||
reference.in_type_checking_block()
|
||||
|| reference.in_typing_only_annotation()
|
||||
|| reference.in_complex_string_type_definition()
|
||||
|| reference.in_simple_string_type_definition()
|
||||
|| reference.in_runtime_evaluated_annotation()
|
||||
}
|
||||
|
||||
struct ImportInfo<'a> {
|
||||
module_name: &'a [&'a str],
|
||||
member_name: Cow<'a, str>,
|
||||
call_path: &'a [&'a str],
|
||||
}
|
||||
|
||||
impl<'a> From<&'a FromImport<'_>> for ImportInfo<'a> {
|
||||
fn from(import: &'a FromImport) -> Self {
|
||||
let module_name = import.module_name();
|
||||
let member_name = import.member_name();
|
||||
let call_path = import.call_path();
|
||||
Self {
|
||||
module_name,
|
||||
member_name,
|
||||
call_path,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> From<&'a Import<'_>> for ImportInfo<'a> {
|
||||
fn from(import: &'a Import) -> Self {
|
||||
let module_name = import.module_name();
|
||||
let member_name = import.member_name();
|
||||
let call_path = import.call_path();
|
||||
Self {
|
||||
module_name,
|
||||
member_name,
|
||||
call_path,
|
||||
}
|
||||
}
|
||||
}
|
|
@ -20,6 +20,7 @@ pub(crate) use global_at_module_level::*;
|
|||
pub(crate) use global_statement::*;
|
||||
pub(crate) use global_variable_not_assigned::*;
|
||||
pub(crate) use import_outside_top_level::*;
|
||||
pub(crate) use import_private_name::*;
|
||||
pub(crate) use import_self::*;
|
||||
pub(crate) use invalid_all_format::*;
|
||||
pub(crate) use invalid_all_object::*;
|
||||
|
@ -101,6 +102,7 @@ mod global_at_module_level;
|
|||
mod global_statement;
|
||||
mod global_variable_not_assigned;
|
||||
mod import_outside_top_level;
|
||||
mod import_private_name;
|
||||
mod import_self;
|
||||
mod invalid_all_format;
|
||||
mod invalid_all_object;
|
||||
|
|
|
@ -0,0 +1,14 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
||||
---
|
||||
__main__.py:6:21: PLC2701 Private name import `_l` from external module `k`
|
||||
|
|
||||
4 | from _f.g import h
|
||||
5 | from i import _j
|
||||
6 | from k import _l as m
|
||||
| ^ PLC2701
|
||||
7 | import _aaa
|
||||
8 | import bbb._ccc
|
||||
|
|
||||
|
||||
|
3
ruff.schema.json
generated
3
ruff.schema.json
generated
|
@ -3098,6 +3098,9 @@
|
|||
"PLC240",
|
||||
"PLC2401",
|
||||
"PLC2403",
|
||||
"PLC27",
|
||||
"PLC270",
|
||||
"PLC2701",
|
||||
"PLC28",
|
||||
"PLC280",
|
||||
"PLC2801",
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue