diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name.py new file mode 100644 index 0000000000000..1049de2eb271c --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name.py @@ -0,0 +1,13 @@ +from __future__ import annotations # Ok + +from .. import __version__ # Ok +from .internal import _private # Ok + +import _private # PLC2701 +import _private as _p # PLC2701 +from _private import _private # PLC2701 +from lib import _private # PLC2701 +from lib import _private as _p # PLC2701 +from lib._private import _private # PLC2701 +from lib._private import _private as _p # PLC2701 +from lib._private import _private, _private as _p # PLC2701 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 448ce1d2bac06..fce51cb18ce35 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -536,6 +536,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::ImportOutsideTopLevel) { pylint::rules::import_outside_top_level(checker, stmt); } + if checker.enabled(Rule::ImportPrivateName) { + pylint::rules::import_private_name(checker, stmt); + } if checker.enabled(Rule::GlobalStatement) { for name in names { if let Some(asname) = name.asname.as_ref() { @@ -715,6 +718,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::ImportOutsideTopLevel) { pylint::rules::import_outside_top_level(checker, stmt); } + if checker.enabled(Rule::ImportPrivateName) { + pylint::rules::import_private_name(checker, stmt); + } if checker.enabled(Rule::GlobalStatement) { for name in names { if let Some(asname) = name.asname.as_ref() { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 4dc1f13c55e05..5069494a83762 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -214,6 +214,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "C0415") => (RuleGroup::Preview, rules::pylint::rules::ImportOutsideTopLevel), (Pylint, "C2401") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiName), (Pylint, "C2403") => (RuleGroup::Preview, rules::pylint::rules::NonAsciiImportName), + (Pylint, "C2701") => (RuleGroup::Preview, rules::pylint::rules::ImportPrivateName), #[allow(deprecated)] (Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString), (Pylint, "C3002") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryDirectLambdaCall), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index e2f272619b2ce..8a07047a10224 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -62,6 +62,7 @@ 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.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"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs new file mode 100644 index 0000000000000..4d90489841ff4 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs @@ -0,0 +1,103 @@ +use ruff_diagnostics::{Diagnostic, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Stmt}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Check for imported symbols that have a leading underscore, known as "private" symbols. +/// +/// ## Why is this bad? +/// According to [PEP 8], the underscore prefix is used to indicate that a symbol is private. +/// Private symbols are not meant to be used outside of the module they are defined in. +/// +/// [PEP 8]: https://peps.python.org/pep-0008/#descriptive-naming-styles +#[violation] +pub struct ImportPrivateName { + symbol_type: SymbolType, +} + +impl Violation for ImportPrivateName { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::None; + + #[derive_message_formats] + fn message(&self) -> String { + let ImportPrivateName { symbol_type } = self; + match symbol_type { + SymbolType::Module => format!("Imported private module"), + SymbolType::FromModule => format!("Imported from private module"), + SymbolType::Object => format!("Imported private object"), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SymbolType { + Module, + FromModule, + Object, +} + +/// C2701 +pub(crate) fn import_private_name(checker: &mut Checker, stmt: &Stmt) { + match stmt { + Stmt::Import(ast::StmtImport { names, .. }) => { + for alias in names { + if alias.name.as_str().starts_with('_') { + checker.diagnostics.push(Diagnostic::new( + ImportPrivateName { + symbol_type: SymbolType::Module, + }, + alias.name.range(), + )); + } + } + } + Stmt::ImportFrom(ast::StmtImportFrom { + names, + module, + level, + .. + }) => { + if level.is_some_and(|level| level > 0) { + // allow relative private imports, common in libs + return; + } + + if let Some(identifier) = module { + if identifier == "__future__" { + return; + } + + if identifier.starts_with('_') { + checker.diagnostics.push(Diagnostic::new( + ImportPrivateName { + symbol_type: SymbolType::Module, + }, + identifier.range(), + )); + } else if identifier.contains("._") { + checker.diagnostics.push(Diagnostic::new( + ImportPrivateName { + symbol_type: SymbolType::FromModule, + }, + identifier.range(), + )); + } + } + + for alias in names { + if alias.name.as_str().starts_with('_') { + checker.diagnostics.push(Diagnostic::new( + ImportPrivateName { + symbol_type: SymbolType::Object, + }, + alias.name.range(), + )); + } + } + } + _ => {} + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index b130d36260182..20c7207ce7559 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -19,6 +19,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::*; @@ -92,6 +93,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; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name.py.snap new file mode 100644 index 0000000000000..ff620e8ea0e42 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name.py.snap @@ -0,0 +1,125 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +import_private_name.py:6:8: PLC2701 Imported private module + | +4 | from .internal import _private # Ok +5 | +6 | import _private # PLC2701 + | ^^^^^^^^ PLC2701 +7 | import _private as _p # PLC2701 +8 | from _private import _private # PLC2701 + | + +import_private_name.py:7:8: PLC2701 Imported private module + | +6 | import _private # PLC2701 +7 | import _private as _p # PLC2701 + | ^^^^^^^^ PLC2701 +8 | from _private import _private # PLC2701 +9 | from lib import _private # PLC2701 + | + +import_private_name.py:8:6: PLC2701 Imported private module + | + 6 | import _private # PLC2701 + 7 | import _private as _p # PLC2701 + 8 | from _private import _private # PLC2701 + | ^^^^^^^^ PLC2701 + 9 | from lib import _private # PLC2701 +10 | from lib import _private as _p # PLC2701 + | + +import_private_name.py:8:22: PLC2701 Imported private object + | + 6 | import _private # PLC2701 + 7 | import _private as _p # PLC2701 + 8 | from _private import _private # PLC2701 + | ^^^^^^^^ PLC2701 + 9 | from lib import _private # PLC2701 +10 | from lib import _private as _p # PLC2701 + | + +import_private_name.py:9:17: PLC2701 Imported private object + | + 7 | import _private as _p # PLC2701 + 8 | from _private import _private # PLC2701 + 9 | from lib import _private # PLC2701 + | ^^^^^^^^ PLC2701 +10 | from lib import _private as _p # PLC2701 +11 | from lib._private import _private # PLC2701 + | + +import_private_name.py:10:17: PLC2701 Imported private object + | + 8 | from _private import _private # PLC2701 + 9 | from lib import _private # PLC2701 +10 | from lib import _private as _p # PLC2701 + | ^^^^^^^^ PLC2701 +11 | from lib._private import _private # PLC2701 +12 | from lib._private import _private as _p # PLC2701 + | + +import_private_name.py:11:6: PLC2701 Imported from private module + | + 9 | from lib import _private # PLC2701 +10 | from lib import _private as _p # PLC2701 +11 | from lib._private import _private # PLC2701 + | ^^^^^^^^^^^^ PLC2701 +12 | from lib._private import _private as _p # PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | + +import_private_name.py:11:26: PLC2701 Imported private object + | + 9 | from lib import _private # PLC2701 +10 | from lib import _private as _p # PLC2701 +11 | from lib._private import _private # PLC2701 + | ^^^^^^^^ PLC2701 +12 | from lib._private import _private as _p # PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | + +import_private_name.py:12:6: PLC2701 Imported from private module + | +10 | from lib import _private as _p # PLC2701 +11 | from lib._private import _private # PLC2701 +12 | from lib._private import _private as _p # PLC2701 + | ^^^^^^^^^^^^ PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | + +import_private_name.py:12:26: PLC2701 Imported private object + | +10 | from lib import _private as _p # PLC2701 +11 | from lib._private import _private # PLC2701 +12 | from lib._private import _private as _p # PLC2701 + | ^^^^^^^^ PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | + +import_private_name.py:13:6: PLC2701 Imported from private module + | +11 | from lib._private import _private # PLC2701 +12 | from lib._private import _private as _p # PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | ^^^^^^^^^^^^ PLC2701 + | + +import_private_name.py:13:26: PLC2701 Imported private object + | +11 | from lib._private import _private # PLC2701 +12 | from lib._private import _private as _p # PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | ^^^^^^^^ PLC2701 + | + +import_private_name.py:13:36: PLC2701 Imported private object + | +11 | from lib._private import _private # PLC2701 +12 | from lib._private import _private as _p # PLC2701 +13 | from lib._private import _private, _private as _p # PLC2701 + | ^^^^^^^^ PLC2701 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index cb37f44161c40..48d9f56eba36c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3033,6 +3033,9 @@ "PLC240", "PLC2401", "PLC2403", + "PLC27", + "PLC270", + "PLC2701", "PLC3", "PLC30", "PLC300",