Skip to content

Commit f239758

Browse files
authored
[flake8-pyi] Fix false positive in __all__ (PYI053) (#26872)
Summary -- This PR fixes a false positive on long strings in `__all__`. These strings correspond to existing exported symbols and are out of the stub author's control. I initially hoped that we could use the existing `Semantic::in_dunder_all_definition`, but this is only set in `Checker::visit_exports` and used for adding global references, so the flag is no longer set when the rule runs in `Checker::visit_expr`. I found another similar piece of existing code in `Checker::handle_node_store` and factored that out into a `Checker` helper method. Test Plan -- New mdtest
1 parent 54acbcd commit f239758

3 files changed

Lines changed: 62 additions & 29 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# `string-or-bytes-too-long` (`PYI053`)
2+
3+
```toml
4+
[lint]
5+
select = ["PYI053"]
6+
```
7+
8+
## Long name in `__all__`
9+
10+
Strings in `__all__` correspond to exported names and should be exempt from the rule.
11+
12+
```pyi
13+
__all__ = [
14+
"aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeeef",
15+
]
16+
```

crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2792,34 +2792,7 @@ impl<'a> Checker<'a> {
27922792
_ => {}
27932793
}
27942794

2795-
let scope = self.semantic.current_scope();
2796-
2797-
if scope.kind.is_module()
2798-
&& match parent {
2799-
Stmt::Assign(ast::StmtAssign { targets, .. }) => {
2800-
if let Some(Expr::Name(ast::ExprName { id, .. })) = targets.first() {
2801-
id == "__all__"
2802-
} else {
2803-
false
2804-
}
2805-
}
2806-
Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => {
2807-
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
2808-
id == "__all__"
2809-
} else {
2810-
false
2811-
}
2812-
}
2813-
Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => {
2814-
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
2815-
id == "__all__"
2816-
} else {
2817-
false
2818-
}
2819-
}
2820-
_ => false,
2821-
}
2822-
{
2795+
if self.in_dunder_all_assignment(parent) {
28232796
let (all_names, all_flags) = self.semantic.extract_dunder_all_names(parent);
28242797

28252798
if all_flags.intersects(DunderAllFlags::INVALID_OBJECT) {
@@ -3285,6 +3258,41 @@ impl<'a> Checker<'a> {
32853258

32863259
self.semantic.restore(snapshot);
32873260
}
3261+
3262+
/// Report whether a module-level `__all__` assignment is being visited.
3263+
///
3264+
/// This differs from [`SemanticModel::in_dunder_all_definition`], which is set only while
3265+
/// adding bindings for the entries in `__all__`.
3266+
pub(crate) fn in_dunder_all_assignment(&self, parent: &Stmt) -> bool {
3267+
if !self.semantic.current_scope().kind.is_module() {
3268+
return false;
3269+
}
3270+
3271+
match parent {
3272+
Stmt::Assign(ast::StmtAssign { targets, .. }) => {
3273+
if let Some(Expr::Name(ast::ExprName { id, .. })) = targets.first() {
3274+
id == "__all__"
3275+
} else {
3276+
false
3277+
}
3278+
}
3279+
Stmt::AugAssign(ast::StmtAugAssign { target, .. }) => {
3280+
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
3281+
id == "__all__"
3282+
} else {
3283+
false
3284+
}
3285+
}
3286+
Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => {
3287+
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
3288+
id == "__all__"
3289+
} else {
3290+
false
3291+
}
3292+
}
3293+
_ => false,
3294+
}
3295+
}
32883296
}
32893297

32903298
struct ParsedAnnotationsCache<'a> {

crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
2121
/// checkers, the primary consumers of stub files. Replace very long constants
2222
/// with ellipses (`...`) to simplify the stub.
2323
///
24+
/// The rule does not apply to long entries in `__all__`, which are assumed to
25+
/// be outside the stub author's control.
26+
///
2427
/// ## Example
2528
///
2629
/// ```pyi
@@ -51,8 +54,10 @@ impl AlwaysFixableViolation for StringOrBytesTooLong {
5154
pub(crate) fn string_or_bytes_too_long(checker: &Checker, string: StringLike) {
5255
let semantic = checker.semantic();
5356

57+
let parent = semantic.current_statement();
58+
5459
// Ignore docstrings.
55-
if is_docstring_stmt(semantic.current_statement()) {
60+
if is_docstring_stmt(parent) {
5661
return;
5762
}
5863

@@ -64,6 +69,10 @@ pub(crate) fn string_or_bytes_too_long(checker: &Checker, string: StringLike) {
6469
return;
6570
}
6671

72+
if checker.in_dunder_all_assignment(parent) {
73+
return;
74+
}
75+
6776
let length = match string {
6877
StringLike::String(ast::ExprStringLiteral { value, .. }) => value.chars().count(),
6978
StringLike::Bytes(ast::ExprBytesLiteral { value, .. }) => value.len(),

0 commit comments

Comments
 (0)