Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] - implement C2701 import-private-name #8903

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
103 changes: 103 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs
Original file line number Diff line number Diff line change
@@ -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(),
));
}
}
}
_ => {}
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|


3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading