From c98ef43a94dd1c1575467a9a83e5f629df4136fb Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Fri, 29 Dec 2023 10:09:22 +0000 Subject: [PATCH] Check non-from imports --- .../import_private_name/submodule/__main__.py | 5 ++ .../rules/pylint/rules/import_private_name.rs | 67 ++++++++++++++++--- ..._private_name__submodule____main__.py.snap | 42 +----------- 3 files changed, 63 insertions(+), 51 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py b/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py index f49abff080d52..be095577fa996 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/import_private_name/submodule/__main__.py @@ -4,6 +4,8 @@ from _f.g import h from i import _j from k import _l as m +import _aaa +import bbb._ccc # Non-errors. import n @@ -30,6 +32,7 @@ from _rr import ss from tt._uu import vv from _ww.xx import yy as zz +import _ddd as ddd some_variable: _nn = None @@ -37,5 +40,7 @@ def func(arg: qq) -> ss: pass class Class: + lst: list[ddd] + def __init__(self, arg: vv) -> "zz": pass 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 index 58574cca511d3..26b04804d0437 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs @@ -1,8 +1,9 @@ use itertools::join; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use std::borrow::Cow; -use ruff_python_semantic::{Imported, ResolvedReference, Scope}; +use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scope}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -76,25 +77,27 @@ pub(crate) fn import_private_name( let Some(import) = binding.as_any_import() else { continue; }; - let Some(import) = import.from_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 module = import.module_name(); - let Some(root_module) = module.first() else { + let Some(root_module) = import_info.module_name.first() else { continue; }; // Relative imports are not a public API. // Ex) `from . import foo` - if module.starts_with(&["."]) { + 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.member_name().starts_with("__") { + if root_module.starts_with("__") || import_info.member_name.starts_with("__") { continue; } @@ -107,8 +110,11 @@ pub(crate) fn import_private_name( continue; } - let call_path = import.call_path(); - if call_path.iter().any(|name| name.starts_with('_')) { + if import_info + .call_path + .iter() + .any(|name| name.starts_with('_')) + { // Ignore private imports used for typing. if binding.context.is_runtime() && binding @@ -119,9 +125,16 @@ pub(crate) fn import_private_name( continue; } - let private_name = call_path.iter().find(|name| name.starts_with('_')).unwrap(); + let private_name = import_info + .call_path + .iter() + .find(|name| name.starts_with('_')) + .unwrap(); let external_module = Some(join( - call_path.iter().take_while(|name| name != &private_name), + import_info + .call_path + .iter() + .take_while(|name| name != &private_name), ".", )) .filter(|module| !module.is_empty()); @@ -144,3 +157,35 @@ fn is_typing(reference: &ResolvedReference) -> bool { || 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, + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name__submodule____main__.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name__submodule____main__.py.snap index cb6584200c692..6aff971da3018 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name__submodule____main__.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2701_import_private_name__submodule____main__.py.snap @@ -1,52 +1,14 @@ --- 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 - | - __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 | -8 | # Non-errors. +7 | import _aaa +8 | import bbb._ccc |