mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-02 14:52:01 +00:00
Revert "[pylint
] Implement import-private-name
(C2701
)" (#9547)
This commit is contained in:
parent
21f2d0c90b
commit
f9191b07c5
16 changed files with 0 additions and 268 deletions
|
@ -1 +0,0 @@
|
||||||
_top_level_secret = 0
|
|
|
@ -1,3 +0,0 @@
|
||||||
_sibling_submodule_secret = 1
|
|
||||||
_another_sibling_submodule_secret = 2
|
|
||||||
some_value = 3
|
|
|
@ -1 +0,0 @@
|
||||||
_submodule_secret = 1
|
|
|
@ -1,46 +0,0 @@
|
||||||
# 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
|
|
|
@ -1 +0,0 @@
|
||||||
_subsubmodule_secret = 42
|
|
|
@ -16,7 +16,6 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
||||||
if !checker.any_enabled(&[
|
if !checker.any_enabled(&[
|
||||||
Rule::AsyncioDanglingTask,
|
Rule::AsyncioDanglingTask,
|
||||||
Rule::GlobalVariableNotAssigned,
|
Rule::GlobalVariableNotAssigned,
|
||||||
Rule::ImportPrivateName,
|
|
||||||
Rule::ImportShadowedByLoopVar,
|
Rule::ImportShadowedByLoopVar,
|
||||||
Rule::NoSelfUse,
|
Rule::NoSelfUse,
|
||||||
Rule::RedefinedArgumentFromLocal,
|
Rule::RedefinedArgumentFromLocal,
|
||||||
|
@ -373,10 +372,6 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::UnusedImport) {
|
if checker.enabled(Rule::UnusedImport) {
|
||||||
pyflakes::rules::unused_import(checker, scope, &mut diagnostics);
|
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() {
|
if scope.kind.is_function() {
|
||||||
|
|
|
@ -217,7 +217,6 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(Pylint, "C2801") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDunderCall),
|
(Pylint, "C2801") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDunderCall),
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
(Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString),
|
(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, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall),
|
||||||
(Pylint, "E0100") => (RuleGroup::Stable, rules::pylint::rules::YieldInInit),
|
(Pylint, "E0100") => (RuleGroup::Stable, rules::pylint::rules::YieldInInit),
|
||||||
(Pylint, "E0101") => (RuleGroup::Stable, rules::pylint::rules::ReturnInInit),
|
(Pylint, "E0101") => (RuleGroup::Stable, rules::pylint::rules::ReturnInInit),
|
||||||
|
|
|
@ -63,10 +63,6 @@ mod tests {
|
||||||
Path::new("global_variable_not_assigned.py")
|
Path::new("global_variable_not_assigned.py")
|
||||||
)]
|
)]
|
||||||
#[test_case(Rule::ImportOutsideTopLevel, Path::new("import_outside_top_level.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::ImportSelf, Path::new("import_self/module.py"))]
|
||||||
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
|
#[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))]
|
||||||
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
|
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
|
||||||
|
|
|
@ -1,187 +0,0 @@
|
||||||
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,7 +20,6 @@ pub(crate) use global_at_module_level::*;
|
||||||
pub(crate) use global_statement::*;
|
pub(crate) use global_statement::*;
|
||||||
pub(crate) use global_variable_not_assigned::*;
|
pub(crate) use global_variable_not_assigned::*;
|
||||||
pub(crate) use import_outside_top_level::*;
|
pub(crate) use import_outside_top_level::*;
|
||||||
pub(crate) use import_private_name::*;
|
|
||||||
pub(crate) use import_self::*;
|
pub(crate) use import_self::*;
|
||||||
pub(crate) use invalid_all_format::*;
|
pub(crate) use invalid_all_format::*;
|
||||||
pub(crate) use invalid_all_object::*;
|
pub(crate) use invalid_all_object::*;
|
||||||
|
@ -102,7 +101,6 @@ mod global_at_module_level;
|
||||||
mod global_statement;
|
mod global_statement;
|
||||||
mod global_variable_not_assigned;
|
mod global_variable_not_assigned;
|
||||||
mod import_outside_top_level;
|
mod import_outside_top_level;
|
||||||
mod import_private_name;
|
|
||||||
mod import_self;
|
mod import_self;
|
||||||
mod invalid_all_format;
|
mod invalid_all_format;
|
||||||
mod invalid_all_object;
|
mod invalid_all_object;
|
||||||
|
|
|
@ -1,14 +0,0 @@
|
||||||
---
|
|
||||||
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,9 +3098,6 @@
|
||||||
"PLC240",
|
"PLC240",
|
||||||
"PLC2401",
|
"PLC2401",
|
||||||
"PLC2403",
|
"PLC2403",
|
||||||
"PLC27",
|
|
||||||
"PLC270",
|
|
||||||
"PLC2701",
|
|
||||||
"PLC28",
|
"PLC28",
|
||||||
"PLC280",
|
"PLC280",
|
||||||
"PLC2801",
|
"PLC2801",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue