Skip to content

Commit fbbed61

Browse files
committed
Add auto-import support for notebook cells
Enable auto-import functionality for notebook cells with proper scoping to ensure import edits are restricted to the current cell rather than spanning across cells. Changes: - Update Insertion::start_of_file to accept optional within_range parameter - When provided, restricts import placement to statements within that range - Allows per-cell import handling in notebooks - Update importer logic to handle notebook cell boundaries - Find cell range for the position where import is needed - Pass cell range to restrict import insertion to current cell - Skip imports from other cells when searching for existing imports - Ensure import insertions respect cell document URIs - Add tests verifying auto-import works correctly in notebooks - Verify imports aren't added to different cells This ensures that when auto-completing in a notebook cell, suggested imports are added to the current cell only, not to import statements in other cells.
1 parent 249643e commit fbbed61

File tree

10 files changed

+617
-23
lines changed

10 files changed

+617
-23
lines changed

.config/nextest.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ serial = { max-threads = 1 }
77
filter = 'binary(file_watching)'
88
test-group = 'serial'
99

10+
[[profile.default.overrides]]
11+
filter = 'binary(e2e)'
12+
test-group = 'serial'
13+
1014
[profile.ci]
1115
# Print out output for failing tests as soon as they fail, and also at the end
1216
# of the run (for easy scrollability).

crates/ruff_linter/src/importer/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ impl<'a> Importer<'a> {
8383
.into_edit(&required_import)
8484
} else {
8585
// Insert at the start of the file.
86-
Insertion::start_of_file(self.python_ast, self.source, self.stylist)
86+
Insertion::start_of_file(self.python_ast, self.source, self.stylist, None)
8787
.into_edit(&required_import)
8888
}
8989
}
@@ -113,7 +113,7 @@ impl<'a> Importer<'a> {
113113
Insertion::end_of_statement(stmt, self.source, self.stylist)
114114
} else {
115115
// Insert at the start of the file.
116-
Insertion::start_of_file(self.python_ast, self.source, self.stylist)
116+
Insertion::start_of_file(self.python_ast, self.source, self.stylist, None)
117117
};
118118
let add_import_edit = insertion.into_edit(&content);
119119

@@ -498,7 +498,7 @@ impl<'a> Importer<'a> {
498498
Insertion::end_of_statement(stmt, self.source, self.stylist)
499499
} else {
500500
// Insert at the start of the file.
501-
Insertion::start_of_file(self.python_ast, self.source, self.stylist)
501+
Insertion::start_of_file(self.python_ast, self.source, self.stylist, None)
502502
};
503503
if insertion.is_inline() {
504504
Err(anyhow::anyhow!(

crates/ruff_notebook/src/cell.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -294,19 +294,17 @@ impl CellOffsets {
294294
}
295295

296296
/// Returns `true` if the given range contains a cell boundary.
297+
///
298+
/// A range starting at the cell boundary isn't considered to contain a cell boundary
299+
/// as it starts right after it. A range starting before a cell boundary
300+
/// and ending exactly at the boundary is considered to contain a cell boundary.
297301
pub fn has_cell_boundary(&self, range: TextRange) -> bool {
298-
self.binary_search_by(|offset| {
299-
if range.start() <= *offset {
300-
if range.end() < *offset {
301-
std::cmp::Ordering::Greater
302-
} else {
303-
std::cmp::Ordering::Equal
304-
}
305-
} else {
306-
std::cmp::Ordering::Less
307-
}
308-
})
309-
.is_ok()
302+
let after_range_start = self.partition_point(|offset| *offset <= range.start());
303+
let Some(boundary) = self.get(after_range_start).copied() else {
304+
return false;
305+
};
306+
307+
range.contains_inclusive(boundary)
310308
}
311309

312310
/// Returns an iterator over [`TextRange`]s covered by each cell.

crates/ruff_python_importer/src/insertion.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use ruff_python_parser::{TokenKind, Tokens};
1010
use ruff_python_trivia::is_python_whitespace;
1111
use ruff_python_trivia::{PythonWhitespace, textwrap::indent};
1212
use ruff_source_file::{LineRanges, UniversalNewlineIterator};
13-
use ruff_text_size::{Ranged, TextSize};
13+
use ruff_text_size::{Ranged, TextRange, TextSize};
1414

1515
#[derive(Debug, Clone, PartialEq, Eq)]
1616
pub(super) enum Placement<'a> {
@@ -37,7 +37,7 @@ pub struct Insertion<'a> {
3737

3838
impl<'a> Insertion<'a> {
3939
/// Create an [`Insertion`] to insert (e.g.) an import statement at the start of a given
40-
/// file, along with a prefix and suffix to use for the insertion.
40+
/// file or cell, along with a prefix and suffix to use for the insertion.
4141
///
4242
/// For example, given the following code:
4343
///
@@ -49,7 +49,21 @@ impl<'a> Insertion<'a> {
4949
///
5050
/// The insertion returned will begin at the start of the `import os` statement, and will
5151
/// include a trailing newline.
52-
pub fn start_of_file(body: &[Stmt], contents: &str, stylist: &Stylist) -> Insertion<'static> {
52+
pub fn start_of_file(
53+
body: &[Stmt],
54+
contents: &str,
55+
stylist: &Stylist,
56+
within_range: Option<TextRange>,
57+
) -> Insertion<'static> {
58+
let body = within_range
59+
.map(|range| {
60+
let start = body.partition_point(|stmt| stmt.start() < range.start());
61+
let end = body.partition_point(|stmt| stmt.end() <= range.end());
62+
63+
&body[start..end]
64+
})
65+
.unwrap_or(body);
66+
5367
// Skip over any docstrings.
5468
let mut location = if let Some(mut location) = match_docstring_end(body) {
5569
// If the first token after the docstring is a semicolon, insert after the semicolon as
@@ -66,6 +80,10 @@ impl<'a> Insertion<'a> {
6680

6781
// Otherwise, advance to the next row.
6882
contents.full_line_end(location)
83+
} else if let Some(range) = within_range
84+
&& range.start() != TextSize::ZERO
85+
{
86+
range.start()
6987
} else {
7088
contents.bom_start_offset()
7189
};
@@ -374,7 +392,12 @@ mod tests {
374392
fn insert(contents: &str) -> Result<Insertion<'_>> {
375393
let parsed = parse_module(contents)?;
376394
let stylist = Stylist::from_tokens(parsed.tokens(), contents);
377-
Ok(Insertion::start_of_file(parsed.suite(), contents, &stylist))
395+
Ok(Insertion::start_of_file(
396+
parsed.suite(),
397+
contents,
398+
&stylist,
399+
None,
400+
))
378401
}
379402

380403
let contents = "";

crates/ty_ide/src/importer.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use rustc_hash::FxHashMap;
2020

2121
use ruff_db::files::File;
2222
use ruff_db::parsed::ParsedModuleRef;
23+
use ruff_db::source::source_text;
2324
use ruff_diagnostics::Edit;
2425
use ruff_python_ast as ast;
2526
use ruff_python_ast::name::Name;
@@ -76,6 +77,7 @@ impl<'a> Importer<'a> {
7677
parsed: &'a ParsedModuleRef,
7778
) -> Self {
7879
let imports = TopLevelImports::find(parsed);
80+
7981
Self {
8082
db,
8183
file,
@@ -145,10 +147,15 @@ impl<'a> Importer<'a> {
145147
let request = request.avoid_conflicts(self.db, self.file, members);
146148
let mut symbol_text: Box<str> = request.member.into();
147149
let Some(response) = self.find(&request, members.at) else {
148-
let insertion = if let Some(future) = self.find_last_future_import() {
150+
let insertion = if let Some(future) = self.find_last_future_import(members.at) {
149151
Insertion::end_of_statement(future.stmt, self.source, self.stylist)
150152
} else {
151-
Insertion::start_of_file(self.parsed.suite(), self.source, self.stylist)
153+
// What we want here: Given an offset, return the range of that cell. I guess that's what containing range is
154+
let range = source_text(self.db, self.file)
155+
.as_notebook()
156+
.and_then(|notebook| notebook.cell_offsets().containing_range(members.at));
157+
158+
Insertion::start_of_file(self.parsed.suite(), self.source, self.stylist, range)
152159
};
153160
let import = insertion.into_edit(&request.to_string());
154161
if matches!(request.style, ImportStyle::Import) {
@@ -209,6 +216,9 @@ impl<'a> Importer<'a> {
209216
available_at: TextSize,
210217
) -> Option<ImportResponse<'importer, 'a>> {
211218
let mut choice = None;
219+
let source = source_text(self.db, self.file);
220+
let notebook = source.as_notebook();
221+
212222
for import in &self.imports {
213223
// If the import statement comes after the spot where we
214224
// need the symbol, then we conservatively assume that
@@ -226,7 +236,22 @@ impl<'a> Importer<'a> {
226236
if import.stmt.start() >= available_at {
227237
return choice;
228238
}
239+
229240
if let Some(response) = import.satisfies(self.db, self.file, request) {
241+
let partial = matches!(response.kind, ImportResponseKind::Partial { .. });
242+
243+
// The LSP doesn't support edits across cell boundaries.
244+
// Skip over imports that only partially satisfy the import
245+
// because they would require changes to the import (across cell boundaries).
246+
if partial
247+
&& let Some(notebook) = notebook
248+
&& notebook
249+
.cell_offsets()
250+
.has_cell_boundary(TextRange::new(import.stmt.start(), available_at))
251+
{
252+
continue;
253+
}
254+
230255
if choice
231256
.as_ref()
232257
.is_none_or(|c| !c.kind.is_prioritized_over(&response.kind))
@@ -247,9 +272,21 @@ impl<'a> Importer<'a> {
247272
}
248273

249274
/// Find the last `from __future__` import statement in the AST.
250-
fn find_last_future_import(&self) -> Option<&'a AstImport> {
275+
fn find_last_future_import(&self, at: TextSize) -> Option<&'a AstImport> {
276+
let source = source_text(self.db, self.file);
277+
let notebook = source.as_notebook();
278+
251279
self.imports
252280
.iter()
281+
.take_while(|import| import.stmt.start() <= at)
282+
// Skip over imports from other cells.
283+
.skip_while(|import| {
284+
notebook.is_some_and(|notebook| {
285+
notebook
286+
.cell_offsets()
287+
.has_cell_boundary(TextRange::new(import.stmt.start(), at))
288+
})
289+
})
253290
.take_while(|import| {
254291
import
255292
.stmt

0 commit comments

Comments
 (0)