Skip to content

Commit 2cec39a

Browse files
fix
1 parent 14b0eed commit 2cec39a

File tree

9 files changed

+287
-206
lines changed

9 files changed

+287
-206
lines changed

crates/pyrefly_types/src/display.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ pub struct TypeDisplayContext<'a> {
8989
/// Should we display for IDE Hover? This makes type names more readable but less precise.
9090
hover: bool,
9191
always_display_module_name: bool,
92-
display_modules: RefCell<SmallSet<ModuleName>>,
92+
/// Modules encountered while formatting, used downstream (e.g. to decide which imports are required).
93+
modules: RefCell<SmallSet<ModuleName>>,
9394
}
9495

9596
impl<'a> TypeDisplayContext<'a> {
@@ -235,7 +236,7 @@ impl<'a> TypeDisplayContext<'a> {
235236
name: &str,
236237
f: &mut fmt::Formatter<'_>,
237238
) -> fmt::Result {
238-
self.display_modules
239+
self.modules
239240
.borrow_mut()
240241
.insert(ModuleName::from_str(module));
241242
if self.always_display_module_name {
@@ -246,7 +247,7 @@ impl<'a> TypeDisplayContext<'a> {
246247
}
247248

248249
pub fn referenced_modules(&self) -> SmallSet<ModuleName> {
249-
let mut modules = self.display_modules.borrow().clone();
250+
let mut modules = self.modules.borrow().clone();
250251
for info in self.qnames.values() {
251252
for module in info.info.keys() {
252253
modules.insert(module.dupe());

pyrefly/lib/lsp/non_wasm/server.rs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2260,12 +2260,11 @@ impl Server {
22602260
// Group consecutive implementations by module, preserving the sorted order
22612261
let mut grouped: Vec<(ModuleInfo, Vec<TextRange>)> = Vec::new();
22622262
for impl_with_module in implementations {
2263-
if let Some((last_module, ranges)) = grouped.last_mut() {
2264-
if last_module.path() == impl_with_module.module.path() {
2263+
if let Some((last_module, ranges)) = grouped.last_mut()
2264+
&& last_module.path() == impl_with_module.module.path() {
22652265
ranges.push(impl_with_module.range);
22662266
continue;
22672267
}
2268-
}
22692268
grouped.push((impl_with_module.module, vec![impl_with_module.range]));
22702269
}
22712270
Ok(grouped)
@@ -2635,20 +2634,20 @@ impl Server {
26352634
)?;
26362635
let res = t
26372636
.into_iter()
2638-
.filter_map(|(text_size, label_text, _locations)| {
2637+
.filter_map(|hint| {
26392638
// If the url is a notebook cell, filter out inlay hints for other cells
2640-
if info.to_cell_for_lsp(text_size) != maybe_cell_idx {
2639+
if info.to_cell_for_lsp(hint.position) != maybe_cell_idx {
26412640
return None;
26422641
}
2643-
let position = info.to_lsp_position(text_size);
2642+
let position = info.to_lsp_position(hint.position);
26442643
// The range is half-open, so the end position is exclusive according to the spec.
26452644
if position >= range.start && position < range.end {
2646-
let mut text_edits = Vec::with_capacity(1 + x.import_edits.len());
2645+
let mut text_edits = Vec::with_capacity(1 + hint.import_edits.len());
26472646
text_edits.push(TextEdit {
26482647
range: Range::new(position, position),
2649-
new_text: x.label.clone(),
2648+
new_text: hint.label.clone(),
26502649
});
2651-
for (offset, import_text) in x.import_edits {
2650+
for (offset, import_text) in hint.import_edits {
26522651
let insert_position = info.to_lsp_position(offset);
26532652
text_edits.push(TextEdit {
26542653
range: Range::new(insert_position, insert_position),
@@ -2657,12 +2656,9 @@ impl Server {
26572656
}
26582657
Some(InlayHint {
26592658
position,
2660-
label: InlayHintLabel::String(label_text.clone()),
2659+
label: InlayHintLabel::String(hint.label),
26612660
kind: None,
2662-
text_edits: Some(vec![TextEdit {
2663-
range: Range::new(position, position),
2664-
new_text: label_text,
2665-
}]),
2661+
text_edits: Some(text_edits),
26662662
tooltip: None,
26672663
padding_left: None,
26682664
padding_right: None,

pyrefly/lib/playground.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,12 @@ impl Playground {
521521
.get_module_info(handle)
522522
.zip(transaction.inlay_hints(handle, Default::default()))
523523
.map(|(info, hints)| {
524-
hints.into_map(|(position, label, _locations)| {
525-
let position = Position::from_display_pos(info.display_pos(position));
526-
InlayHint { label, position }
524+
hints.into_map(|hint| {
525+
let position = Position::from_display_pos(info.display_pos(hint.position));
526+
InlayHint {
527+
label: hint.label,
528+
position,
529+
}
527530
})
528531
})
529532
.unwrap_or_default()
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
//! Helpers for harvesting imports and formatting type strings for inlay hints.
9+
10+
use std::cmp::Reverse;
11+
12+
use dupe::Dupe;
13+
use pyrefly_python::module_name::ModuleName;
14+
use ruff_python_ast::ModModule;
15+
use ruff_python_ast::Stmt;
16+
use ruff_python_ast::StmtImport;
17+
use starlark_map::small_set::SmallSet;
18+
19+
use crate::types::display::TypeDisplayContext;
20+
use crate::types::types::Type;
21+
22+
/// Tracks imports already present in a module and can determine which modules are still missing
23+
/// for a given set of referenced modules. Also supports alias-aware replacement when displaying
24+
/// type strings.
25+
#[derive(Default)]
26+
pub struct ImportTracker {
27+
canonical_modules: SmallSet<ModuleName>,
28+
alias_modules: Vec<(ModuleName, String)>,
29+
}
30+
31+
impl ImportTracker {
32+
/// Build an import tracker from the top-level `import ...` statements in a module.
33+
pub fn from_ast(ast: &ModModule) -> Self {
34+
let mut tracker = Self::default();
35+
for stmt in &ast.body {
36+
if let Stmt::Import(stmt_import) = stmt {
37+
tracker.record_import(stmt_import);
38+
}
39+
}
40+
tracker
41+
.alias_modules
42+
.sort_by_key(|(module, _)| Reverse(module.as_str().len()));
43+
tracker
44+
}
45+
46+
/// Record an `import ...` statement into the tracker.
47+
pub fn record_import(&mut self, stmt_import: &StmtImport) {
48+
for alias in &stmt_import.names {
49+
let module_name = ModuleName::from_str(alias.name.as_str());
50+
if let Some(asname) = &alias.asname {
51+
self.alias_modules
52+
.push((module_name, asname.id.to_string()));
53+
} else {
54+
self.canonical_modules.insert(module_name);
55+
}
56+
}
57+
}
58+
59+
/// Replace any module prefixes that have been imported under an alias (e.g. `import typing as t`).
60+
pub fn apply_aliases(&self, text: &str) -> String {
61+
if self.alias_modules.is_empty() {
62+
return text.to_owned();
63+
}
64+
let bytes = text.as_bytes();
65+
let mut result = String::with_capacity(text.len());
66+
let mut i = 0;
67+
while i < bytes.len() {
68+
let mut replaced = false;
69+
for (module, alias) in &self.alias_modules {
70+
let module_str = module.as_str();
71+
if module_str.is_empty() {
72+
continue;
73+
}
74+
let module_bytes = module_str.as_bytes();
75+
if i + module_bytes.len() <= bytes.len()
76+
&& &bytes[i..i + module_bytes.len()] == module_bytes
77+
&& Self::is_boundary(bytes, i, i + module_bytes.len())
78+
{
79+
result.push_str(alias);
80+
i += module_bytes.len();
81+
replaced = true;
82+
break;
83+
}
84+
}
85+
if !replaced {
86+
result.push(bytes[i] as char);
87+
i += 1;
88+
}
89+
}
90+
result
91+
}
92+
93+
/// Modules that are referenced in the type string but not yet imported (excluding builtins/current).
94+
pub fn missing_modules(
95+
&self,
96+
modules: &SmallSet<ModuleName>,
97+
current_module: ModuleName,
98+
) -> SmallSet<ModuleName> {
99+
let mut missing = SmallSet::new();
100+
for module in modules.iter() {
101+
let module = module.dupe();
102+
if module.as_str().is_empty()
103+
|| module == current_module
104+
|| module == ModuleName::builtins()
105+
|| module == ModuleName::extra_builtins()
106+
{
107+
continue;
108+
}
109+
if self.module_is_imported(module) {
110+
continue;
111+
}
112+
missing.insert(module);
113+
}
114+
missing
115+
}
116+
117+
fn module_is_imported(&self, module: ModuleName) -> bool {
118+
self.alias_for(module).is_some() || self.has_canonical(module)
119+
}
120+
121+
fn alias_for(&self, module: ModuleName) -> Option<String> {
122+
let target = module.as_str();
123+
for (alias_module, alias_name) in &self.alias_modules {
124+
let alias_module_str = alias_module.as_str();
125+
if alias_module_str.is_empty() {
126+
continue;
127+
}
128+
if target == alias_module_str {
129+
return Some(alias_name.clone());
130+
}
131+
if target.len() > alias_module_str.len()
132+
&& target.starts_with(alias_module_str)
133+
&& target.as_bytes()[alias_module_str.len()] == b'.'
134+
{
135+
let remainder = &target[alias_module_str.len()..];
136+
return Some(format!("{alias_name}{remainder}"));
137+
}
138+
}
139+
None
140+
}
141+
142+
fn has_canonical(&self, module: ModuleName) -> bool {
143+
let target = module.as_str();
144+
self.canonical_modules.iter().any(|imported| {
145+
let imported_str = imported.as_str();
146+
imported_str == target
147+
|| (target.len() > imported_str.len()
148+
&& target.starts_with(imported_str)
149+
&& target.as_bytes()[imported_str.len()] == b'.')
150+
})
151+
}
152+
153+
fn is_boundary(bytes: &[u8], start: usize, end: usize) -> bool {
154+
(start == 0 || !Self::is_ident(bytes[start - 1]))
155+
&& (end == bytes.len() || !Self::is_ident(bytes[end]))
156+
}
157+
158+
fn is_ident(byte: u8) -> bool {
159+
matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_')
160+
}
161+
}
162+
163+
/// Produce a user-facing type string (without module qualifiers) together with all referenced modules
164+
/// (captured with module qualification) so callers can insert the necessary imports.
165+
pub fn format_type_for_annotation(ty: &Type) -> (String, SmallSet<ModuleName>) {
166+
// First pass: force module names so referenced_modules collects everything, but ignore the text.
167+
let mut module_ctx = TypeDisplayContext::new(&[ty]);
168+
module_ctx.always_display_module_name_except_builtins();
169+
let _ = module_ctx.display(ty).to_string();
170+
let modules = module_ctx.referenced_modules();
171+
172+
// Second pass: produce a concise label without module qualifiers.
173+
let display_ctx = TypeDisplayContext::new(&[ty]);
174+
let text = display_ctx.display(ty).to_string();
175+
(text, modules)
176+
}
177+
178+
#[cfg(test)]
179+
mod tests {
180+
use super::*;
181+
182+
#[test]
183+
fn aliases_are_applied_at_boundaries_only() {
184+
let module = ModuleName::from_str("typing");
185+
let mut tracker = ImportTracker::default();
186+
tracker.alias_modules.push((module, "t".to_owned()));
187+
assert_eq!(tracker.apply_aliases("typing.Literal"), "t.Literal");
188+
// Do not replace inside longer identifiers
189+
assert_eq!(tracker.apply_aliases("mytyping"), "mytyping");
190+
}
191+
192+
#[test]
193+
fn missing_modules_skips_builtin_and_current() {
194+
let tracker = ImportTracker::default();
195+
let mut modules = SmallSet::new();
196+
let current = ModuleName::from_str("pkg.mod");
197+
modules.insert(current.dupe());
198+
modules.insert(ModuleName::builtins());
199+
modules.insert(ModuleName::from_str("typing"));
200+
let missing = tracker.missing_modules(&modules, current);
201+
assert!(missing.contains(&ModuleName::from_str("typing")));
202+
assert_eq!(missing.len(), 1);
203+
}
204+
205+
#[test]
206+
fn format_type_collects_modules_but_returns_short_label() {
207+
let ty = Type::LiteralString;
208+
let (text, modules) = format_type_for_annotation(&ty);
209+
assert_eq!(text, "LiteralString");
210+
assert!(modules.contains(&ModuleName::from_str("typing")));
211+
}
212+
}

0 commit comments

Comments
 (0)