Skip to content

Commit 80dbc62

Browse files
Jkhall81ntBre
andauthored
[pylint] Allow dunder submodules and improve diagnostic range (PLC2701) (#22804)
## Summary This PR addresses two improvements for the `PLC2701` (`import-private-name`) rule: 1. **Excludes dunder submodules**: Specifically handles the false positive where `__main__` was being flagged as a private name import. It now ensures that any segment identified as a "dunder" (starting and ending with `__`) is ignored by the rule. 2. **Improves diagnostic ranges**: Refines the error highlight to be more surgical. Instead of pointing to the end of the import binding, the linter now pinpoint's the exact private segment—whether it's in the module path (e.g., `from pkg._private import func`) or the imported member (e.g., `from pkg import _private`). Closes #22187 ## Test Plan - **Manual Verification**: Verified using a `repro.py` script covering three scenarios: - `from pkg import _private` (Surgical highlight on member) - `from pkg._private import func` (Surgical highlight on module path) - `from pkg.__main__ import func` (Correctly ignored) - **Snapshot Testing**: Updated existing test snapshots in `crates/ruff_linter/src/rules/pylint/snapshots/`. The diffs confirm that diagnostic ranges have shifted from the imported name to the actual private name causing the violation. --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent 6a93673 commit 80dbc62

3 files changed

Lines changed: 46 additions & 12 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,5 @@ class Class:
4444

4545
def __init__(self, arg: vv) -> "zz":
4646
pass
47+
48+
from foo. _bar import baz

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ use std::borrow::Cow;
33
use itertools::Itertools;
44

55
use ruff_macros::{ViolationMetadata, derive_message_formats};
6-
use ruff_python_ast::name::QualifiedName;
6+
use ruff_python_ast::{self as ast, helpers::is_dunder, name::QualifiedName};
77
use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scope};
8+
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
89
use ruff_text_size::Ranged;
910

1011
use crate::Violation;
@@ -96,7 +97,7 @@ pub(crate) fn import_private_name(checker: &Checker, scope: &Scope) {
9697
// We can also ignore dunder names.
9798
// Ex) `from __future__ import annotations`
9899
// Ex) `from foo import __version__`
99-
if root_module.starts_with("__") || import_info.member_name.starts_with("__") {
100+
if is_dunder(root_module) || is_dunder(&import_info.member_name) {
100101
continue;
101102
}
102103

@@ -116,7 +117,7 @@ pub(crate) fn import_private_name(checker: &Checker, scope: &Scope) {
116117
.qualified_name
117118
.segments()
118119
.iter()
119-
.find_position(|name| name.starts_with('_'))
120+
.find_position(|name| name.starts_with('_') && !is_dunder(name))
120121
else {
121122
continue;
122123
};
@@ -137,7 +138,29 @@ pub(crate) fn import_private_name(checker: &Checker, scope: &Scope) {
137138
} else {
138139
None
139140
};
140-
checker.report_diagnostic(ImportPrivateName { name, module }, binding.range());
141+
let range = match binding.source.map(|id| checker.semantic().statement(id)) {
142+
Some(ast::Stmt::ImportFrom(ast::StmtImportFrom { module, names, .. })) => {
143+
if index < import_info.module_name.len() {
144+
module.as_ref().map_or(binding.range(), |module| {
145+
SimpleTokenizer::starts_at(module.start(), checker.locator().contents())
146+
.skip_trivia()
147+
.find(|token| {
148+
token.kind == SimpleTokenKind::Name
149+
&& checker.locator().slice(token.range()) == *private_name
150+
})
151+
.map_or(binding.range(), |token| token.range())
152+
})
153+
} else {
154+
names
155+
.iter()
156+
.find(|alias| alias.name.as_str() == import_info.member_name)
157+
.map_or(binding.range(), |alias| alias.name.range())
158+
}
159+
}
160+
_ => binding.range(),
161+
};
162+
163+
checker.report_diagnostic(ImportPrivateName { name, module }, range);
141164
}
142165
}
143166

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,33 @@
22
source: crates/ruff_linter/src/rules/pylint/mod.rs
33
---
44
PLC2701 Private name import `_a`
5-
--> __main__.py:2:16
5+
--> __main__.py:2:6
66
|
77
1 | # Errors.
88
2 | from _a import b
9-
| ^
9+
| ^^
1010
3 | from c._d import e
1111
4 | from _f.g import h
1212
|
1313

1414
PLC2701 Private name import `_d` from external module `c`
15-
--> __main__.py:3:18
15+
--> __main__.py:3:8
1616
|
1717
1 | # Errors.
1818
2 | from _a import b
1919
3 | from c._d import e
20-
| ^
20+
| ^^
2121
4 | from _f.g import h
2222
5 | from i import _j
2323
|
2424

2525
PLC2701 Private name import `_f`
26-
--> __main__.py:4:18
26+
--> __main__.py:4:6
2727
|
2828
2 | from _a import b
2929
3 | from c._d import e
3030
4 | from _f.g import h
31-
| ^
31+
| ^^
3232
5 | from i import _j
3333
6 | from k import _l as m
3434
|
@@ -45,12 +45,12 @@ PLC2701 Private name import `_j` from external module `i`
4545
|
4646

4747
PLC2701 Private name import `_l` from external module `k`
48-
--> __main__.py:6:21
48+
--> __main__.py:6:15
4949
|
5050
4 | from _f.g import h
5151
5 | from i import _j
5252
6 | from k import _l as m
53-
| ^
53+
| ^^
5454
7 | import _aaa
5555
8 | import bbb.ccc._ddd as eee # Panicked in https://github.com/astral-sh/ruff/pull/5920
5656
|
@@ -75,3 +75,12 @@ PLC2701 Private name import `_ddd` from external module `bbb.ccc`
7575
9 |
7676
10 | # Non-errors.
7777
|
78+
79+
PLC2701 Private name import `_bar` from external module `foo`
80+
--> __main__.py:48:14
81+
|
82+
46 | pass
83+
47 |
84+
48 | from foo. _bar import baz
85+
| ^^^^
86+
|

0 commit comments

Comments
 (0)