Skip to content

Commit 431d4c2

Browse files
maldoincmeta-codesync[bot]
authored andcommitted
Fix wrong unused-import when referenced in __all__ (#2125)
Summary: Currently if a file contains an import that is only referenced in `__all__` it is incorrectly flagged as unused. This addresses that. Fixes #2128 Pull Request resolved: #2125 Test Plan: Added `test_diagnostic_import_used_in_all`. Reviewed By: kinto0 Differential Revision: D91080237 Pulled By: stroxler fbshipit-source-id: 79b9a6d114cc94d4e5519fcbee34ac6bb5eb65e3
1 parent 5a47e90 commit 431d4c2

File tree

6 files changed

+86
-0
lines changed

6 files changed

+86
-0
lines changed

pyrefly/lib/binding/bindings.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,10 @@ impl Bindings {
454454
);
455455
}
456456

457+
if let Some(exported_names) = exports.get_explicit_dunder_all_names_iter() {
458+
builder.record_used_imports_from_dunder_all_names(exported_names);
459+
}
460+
457461
let unused_imports = builder.scopes.collect_module_unused_imports();
458462
builder.record_unused_imports(unused_imports);
459463
let scope_trace = builder.scopes.finish();
@@ -708,6 +712,17 @@ impl<'a> BindingsBuilder<'a> {
708712
self.unused_variables.extend(unused);
709713
}
710714

715+
pub fn record_used_imports_from_dunder_all_names<T>(&mut self, dunder_all_names: T)
716+
where
717+
T: Iterator<Item = &'a Name>,
718+
{
719+
for name in dunder_all_names {
720+
if self.scopes.has_import_name(name) {
721+
self.scopes.mark_import_used(name);
722+
}
723+
}
724+
}
725+
711726
pub(crate) fn with_await_context<R>(
712727
&mut self,
713728
ctx: AwaitContext,

pyrefly/lib/binding/scope.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,6 +1296,15 @@ impl Scopes {
12961296
ScopeTrace(b)
12971297
}
12981298

1299+
pub fn has_import_name(&self, name: &Name) -> bool {
1300+
let module_scope = self.scopes.first();
1301+
1302+
match module_scope.scope.kind {
1303+
ScopeKind::Module => module_scope.scope.imports.contains_key(name),
1304+
_ => false,
1305+
}
1306+
}
1307+
12991308
pub fn collect_module_unused_imports(&self) -> Vec<UnusedImport> {
13001309
let module_scope = self.scopes.first();
13011310
if !matches!(module_scope.scope.kind, ScopeKind::Module) {

pyrefly/lib/export/exports.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,24 @@ impl Exports {
168168
.contains(name)
169169
}
170170

171+
/// Return an iterator with entries in `__all__` that are user-defined or None if `__all__` was not present.
172+
pub fn get_explicit_dunder_all_names_iter(&self) -> Option<impl Iterator<Item = &Name>> {
173+
match self.0.definitions.dunder_all.kind {
174+
DunderAllKind::Specified => Some(
175+
self.0
176+
.definitions
177+
.dunder_all
178+
.entries
179+
.iter()
180+
.filter_map(|entry| match entry {
181+
DunderAllEntry::Name(_, name) => Some(name),
182+
_ => None,
183+
}),
184+
),
185+
_ => None,
186+
}
187+
}
188+
171189
/// Returns entries in `__all__` that don't exist in the module's definitions.
172190
/// Only validates explicitly user-defined `__all__` entries, not synthesized ones.
173191
/// Returns a vector of (range, name) tuples for invalid entries.

pyrefly/lib/test/lsp/lsp_interaction/diagnostic.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,32 @@ fn test_unused_from_import_diagnostic() {
407407
interaction.shutdown().unwrap();
408408
}
409409

410+
#[test]
411+
fn test_diagnostic_import_used_in_all() {
412+
let test_files_root = get_test_files_root();
413+
let mut interaction = LspInteraction::new();
414+
interaction.set_root(test_files_root.path().to_path_buf());
415+
interaction
416+
.initialize(InitializeSettings {
417+
configuration: Some(Some(json!([
418+
{"pyrefly": {"displayTypeErrors": "force-on"}}
419+
]))),
420+
..Default::default()
421+
})
422+
.unwrap();
423+
424+
interaction.client.did_open("unused_import_all/__init__.py");
425+
interaction
426+
.client
427+
.diagnostic("unused_import_all/__init__.py")
428+
.expect_response(json!({
429+
"items": [],
430+
"kind": "full"
431+
}))
432+
.unwrap();
433+
interaction.shutdown().unwrap();
434+
}
435+
410436
#[test]
411437
fn test_unused_variable_diagnostic() {
412438
let test_files_root = get_test_files_root();
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
from foo import Bar as Baz, Foo
7+
8+
__all__ = ["Foo", "Baz"]
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
7+
class Foo: ...
8+
9+
10+
class Bar: ...

0 commit comments

Comments
 (0)