Skip to content

Commit b0dd9d7

Browse files
committed
Fall back to absolute import when a move breaks a relative import
1 parent e3f536e commit b0dd9d7

2 files changed

Lines changed: 77 additions & 17 deletions

File tree

pyrefly/lib/lsp/non_wasm/will_rename_files.rs

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ use pyrefly_util::lock::RwLock;
2626
use rayon::prelude::*;
2727
use ruff_python_ast::Stmt;
2828
use ruff_text_size::Ranged;
29+
use ruff_text_size::TextRange;
30+
use ruff_text_size::TextSize;
2931
use tracing::info;
3032

3133
use crate::lsp::non_wasm::module_helpers::PathRemapper;
@@ -102,31 +104,45 @@ impl<'a> RenameUsageVisitor<'a> {
102104
let Some(replacement) = self.replacement_for_module(imported_module) else {
103105
return;
104106
};
105-
let new_import_name = if import_from.level == 0 {
106-
replacement
107+
let (range, new_text) = if import_from.level == 0 {
108+
(module.range(), replacement)
107109
} else {
108-
let Some(current_package) = self.current_module_name.new_maybe_relative(
109-
self.is_init,
110-
import_from.level,
111-
None,
112-
) else {
113-
return;
114-
};
110+
let current_package = self
111+
.current_module_name
112+
.new_maybe_relative(self.is_init, import_from.level, None)
113+
.expect("resolved above with the same number of dots");
115114
let current_package_prefix = if current_package.as_str().is_empty() {
116115
String::new()
117116
} else {
118117
format!("{}.", current_package.as_str())
119118
};
120-
let Some(relative_replacement) =
121-
replacement.strip_prefix(&current_package_prefix)
122-
else {
123-
return;
124-
};
125-
relative_replacement.to_owned()
119+
match replacement.strip_prefix(&current_package_prefix) {
120+
Some(relative_replacement) => {
121+
(module.range(), relative_replacement.to_owned())
122+
}
123+
// Keeping the original dots would resolve against the wrong package.
124+
// Ruff stores their count but not their range, so we can just locate them
125+
// in the source before replacing the whole path.
126+
None => {
127+
let before_module =
128+
TextRange::new(import_from.range().start(), module.range().start());
129+
let dots_offset = self
130+
.lined_buffer
131+
.code_at(before_module)
132+
.find('.')
133+
.expect("relative import has a dot before the module name");
134+
let dots_start = before_module.start()
135+
+ TextSize::try_from(dots_offset).expect("offset fits in u32");
136+
(
137+
TextRange::new(dots_start, module.range().end()),
138+
replacement,
139+
)
140+
}
141+
}
126142
};
127143
self.edits.push(TextEdit {
128-
range: self.lined_buffer.to_lsp_range(module.range(), None),
129-
new_text: new_import_name,
144+
range: self.lined_buffer.to_lsp_range(range, None),
145+
new_text,
130146
});
131147
}
132148
_ => {}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,50 @@ fn test_will_rename_files_updates_relative_package_import() {
433433
interaction.shutdown().unwrap();
434434
}
435435

436+
#[test]
437+
fn test_will_rename_files_rewrites_relative_import_as_absolute_on_move() {
438+
let root = get_test_files_root();
439+
let mut interaction = LspInteraction::new_with_args(LspInteractionArgs {
440+
args: LspArgs {
441+
indexing_mode: IndexingMode::LazyBlocking,
442+
..LspInteractionArgs::default().args
443+
},
444+
..Default::default()
445+
});
446+
let root_path = root.path().join("package_relative_rename");
447+
let scope_uri = Url::from_file_path(&root_path).unwrap();
448+
449+
interaction.set_root(root_path.clone());
450+
interaction
451+
.initialize(InitializeSettings {
452+
workspace_folders: Some(vec![("test".to_owned(), scope_uri)]),
453+
..Default::default()
454+
})
455+
.unwrap();
456+
457+
interaction.client.did_open("pkg/a.py");
458+
459+
interaction
460+
.client
461+
.will_rename_files("pkg/a.py", "other/a.py")
462+
.expect_response(json!({
463+
"changes": {
464+
Url::from_file_path(root_path.join("pkg/b.py")).unwrap().to_string(): [
465+
{
466+
"newText": "other.a",
467+
"range": {
468+
"start": {"line": 5, "character": 5},
469+
"end": {"line": 5, "character": 7}
470+
}
471+
},
472+
],
473+
}
474+
}))
475+
.unwrap();
476+
477+
interaction.shutdown().unwrap();
478+
}
479+
436480
#[test]
437481
fn test_will_rename_files_document_changes() {
438482
let root = get_test_files_root();

0 commit comments

Comments
 (0)