[pylint] (Re-)Implement import-private-name (C2701)

crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/__init__.py

@@ -0,0 +1 @@
+_top_level_secret = 0

crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/sibling_module/__init__.py

@@ -0,0 +1,3 @@
+_sibling_submodule_secret = 1
+_another_sibling_submodule_secret = 2
+some_value = 3

crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__init__.py

@@ -0,0 +1 @@
+_submodule_secret = 1

crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py

@@ -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._ddd as eee  # Panicked in https://github.com/astral-sh/ruff/pull/5920
+
+# 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

crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/subsubmodule/__init__.py

@@ -0,0 +1 @@
+_subsubmodule_secret = 42

crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs

@@ -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() {

crates/ruff_linter/src/codes.rs

@@ -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),

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -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"))]

crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs

@@ -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()),
+            _ => continue,
+        };
+
+        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.call_path[..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,
+        }
+    }
+}

crates/ruff_linter/src/rules/pylint/rules/mod.rs

@@ -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;

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name__submodule____main__.py.snap

@@ -0,0 +1,72 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+__main__.py:2:16: PLC2701 Private name import `_a`
+  |
+1 | # Errors.
+2 | from _a import b
+  |                ^ PLC2701
+3 | from c._d import e
+4 | from _f.g import h
+  |
+
+__main__.py:3:18: PLC2701 Private name import `_d` from external module `c`
+  |
+1 | # Errors.
+2 | from _a import b
+3 | from c._d import e
+  |                  ^ PLC2701
+4 | from _f.g import h
+5 | from i import _j
+  |
+
+__main__.py:4:18: PLC2701 Private name import `_f`
+  |
+2 | from _a import b
+3 | from c._d import e
+4 | from _f.g import h
+  |                  ^ PLC2701
+5 | from i import _j
+6 | from k import _l as m
+  |
+
+__main__.py:5:15: PLC2701 Private name import `_j` from external module `i`
+  |
+3 | from c._d import e
+4 | from _f.g import h
+5 | from i import _j
+  |               ^^ PLC2701
+6 | from k import _l as m
+7 | import _aaa
+  |
+
+__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._ddd as eee  # Panicked in https://github.com/astral-sh/ruff/pull/5920
+  |
+
+__main__.py:7:8: PLC2701 Private name import `_aaa`
+  |
+5 | from i import _j
+6 | from k import _l as m
+7 | import _aaa
+  |        ^^^^ PLC2701
+8 | import bbb.ccc._ddd as eee  # Panicked in https://github.com/astral-sh/ruff/pull/5920
+  |
+
+__main__.py:8:24: PLC2701 Private name import `_ddd` from external module `bbb.ccc`
+   |
+ 6 | from k import _l as m
+ 7 | import _aaa
+ 8 | import bbb.ccc._ddd as eee  # Panicked in https://github.com/astral-sh/ruff/pull/5920
+   |                        ^^^ PLC2701
+ 9 | 
+10 | # Non-errors.
+   |
+
+