Skip to content

Commit

Permalink
Check non-from imports
Browse files Browse the repository at this point in the history
  • Loading branch information
tjkuson committed Dec 29, 2023
1 parent 44cc626 commit c98ef43
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,12 +32,15 @@
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
67 changes: 56 additions & 11 deletions crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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
Expand All @@ -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());
Expand All @@ -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,
}
}
}
Original file line number Diff line number Diff line change
@@ -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
|


0 comments on commit c98ef43

Please sign in to comment.