Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/ty_ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod hover;
mod importer;
mod inlay_hints;
mod markup;
mod module_rename;
mod references;
mod rename;
mod selection_range;
Expand Down Expand Up @@ -69,6 +70,9 @@ pub use inlay_hints::{
InlayHintKind, InlayHintLabel, InlayHintSettings, InlayHintTextEdit, inlay_hints,
};
pub use markup::MarkupKind;
pub use module_rename::{
FileMove, ModuleRenameEdits, SkipReason, SkippedImport, module_rename_edits,
};
pub use references::ReferencesMode;
pub use rename::{can_rename, rename};
pub use selection_range::selection_range;
Expand Down
800 changes: 800 additions & 0 deletions crates/ty_ide/src/module_rename.rs

Large diffs are not rendered by default.

135 changes: 135 additions & 0 deletions crates/ty_module_resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,52 @@ type FxOrderMap<K, V> = ordermap::map::OrderMap<K, V, BuildHasherDefault<FxHashe
#[cfg(test)]
mod testing;

/// The name a module at `path` would have, whether or not anything is there yet.
///
/// [`file_to_module`] answers this for a file the system already knows about, and it does more than
/// convert a path: it resolves the name it derived back to a file and checks that the answer is the
/// same file, so that a `src/foo.py` sitting beside a `src/foo/__init__.py` is correctly reported as
/// *not* being the module `foo`.
///
/// That check is exactly what cannot be done for a path nothing is at. This answers the narrower,
/// purely path-shaped question — which search path covers it, and what does the rest of the path
/// spell — which is what an editor asks when it is about to *move* a file: at that moment the old
/// path still holds the file and the new path holds nothing, and both names are needed to work out
/// what the move costs.
///
/// Works for a directory as well as a file, because a package is a directory: `src/foo/bar` and
/// `src/foo/bar.py` both give `foo.bar`.
///
/// # Which search path names it
///
/// The **deepest** one that contains it, not the first one consulted. Search paths nest all the
/// time — a project root with a `src/` layout under it, and, in a uv workspace, one editable entry
/// per member pointing at that member's own `src` — so a member's package is inside two of them at
/// once. Taking the first would name `packages/alpha/src/alpha` after the project root, as
/// `packages.alpha.src.alpha`, which is not a module anything can import and not the name any
/// `import alpha` in the project resolves to. The deepest entry is the one whose name resolves back
/// to that path, which is what [`file_to_module`] verifies for a file that exists.
pub fn path_to_module_name<'db>(
db: &'db dyn Db,
resolver_environment: ResolverEnvironment<'db>,
path: &SystemPath,
) -> Option<ModuleName> {
// `Typing` mode for the reason `system_module_search_paths` gives below: the question is which
// paths belong to the project at all, not which of two stdlib variants a name resolves to.
search_paths(db, resolver_environment, ModuleResolveMode::Typing)
.filter_map(|search_path| {
let name = search_path.relativize_system_path(path)?.to_module_name()?;
// How much of `path` this search path accounts for. A vendored path accounts for none of
// it and sorts last, which is right: nothing under the project is named by the stdlib.
let depth = search_path
.as_system_path()
.map_or(0, |root| root.as_str().len());
Some((depth, name))
})
.max_by_key(|(depth, _)| *depth)
.map(|(_, name)| name)
}

/// Returns an iterator over all search paths pointing to a system path
pub fn system_module_search_paths<'db>(
db: &'db dyn Db,
Expand Down Expand Up @@ -76,3 +122,92 @@ impl<'db> Iterator for SystemModuleSearchPathsIter<'db> {
}

impl FusedIterator for SystemModuleSearchPathsIter<'_> {}

#[cfg(test)]
mod tests {
use ruff_db::Db as _;
use ruff_db::system::{DbWithWritableSystem as _, SystemPathBuf};

use crate::db::tests::TestDb;
use crate::settings::SearchPathSettings;
use crate::strategy::FallibleStrategy;

use super::*;

/// A project whose search paths nest, which is what a uv workspace's editable installs produce:
/// the project root, and one entry per member pointing at that member's own `src`.
fn workspace() -> TestDb {
let project = SystemPathBuf::from("/project");
let member_src = project.join("packages/alpha/src");

let mut db = TestDb::new();
db.write_file(member_src.join("alpha/__init__.py"), "")
.unwrap();

let search_paths = SearchPathSettings {
src_roots: vec![project],
extra_paths: vec![member_src],
..SearchPathSettings::empty()
}
.to_search_paths(db.system(), db.vendored(), &FallibleStrategy)
.expect("valid search path settings");
db.set_search_paths(search_paths);
db
}

/// The bug this rule exists for: the project root also contains the member's package, and naming
/// it from there gives `packages.alpha.src.alpha` — a name nothing imports and nothing resolves.
#[test]
fn a_path_is_named_by_the_deepest_search_path_that_contains_it() {
let db = workspace();
assert_eq!(
path_to_module_name(
&db,
db.resolver_environment(),
SystemPath::new("/project/packages/alpha/src/alpha"),
),
ModuleName::new("alpha"),
);
}

#[test]
fn a_module_inside_a_package_is_named_under_it() {
let db = workspace();
assert_eq!(
path_to_module_name(
&db,
db.resolver_environment(),
SystemPath::new("/project/packages/alpha/src/alpha/util.py"),
),
ModuleName::new("alpha.util"),
);
}

/// The point of asking about a path rather than a file: at the moment an editor asks, the answer
/// is about somewhere nothing has been written yet.
#[test]
fn a_path_nothing_is_at_still_has_a_name() {
let db = workspace();
assert_eq!(
path_to_module_name(
&db,
db.resolver_environment(),
SystemPath::new("/project/packages/alpha/src/gamma"),
),
ModuleName::new("gamma"),
);
}

#[test]
fn a_path_no_search_path_covers_is_not_a_module() {
let db = workspace();
assert_eq!(
path_to_module_name(
&db,
db.resolver_environment(),
SystemPath::new("/elsewhere/thing.py")
),
None,
);
}
}
42 changes: 42 additions & 0 deletions crates/ty_server/src/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,10 @@ pub(crate) fn server_capabilities(
supported: Some(true),
change_notifications: Some(true.into()),
}),
file_operations: Some(lsp_types::FileOperationOptions {
will_rename: Some(will_rename_registration()),
..Default::default()
}),
..Default::default()
}),
type_hierarchy_provider: Some(true.into()),
Expand All @@ -580,6 +584,44 @@ pub(crate) fn server_capabilities(
}
}

/// Which renames the client should ask about before it carries them out.
///
/// Two filters, and both are needed for the same feature. A **file** rename is a module renamed —
/// every module lives in one file, and the extensions listed are the ones the module resolver
/// accepts. A **folder** rename is a *package* renamed, which renames every module inside it, and
/// the client sends only the folder (never its contents), so a server that asked for files alone
/// would be told nothing at all about the rename that changes the most names.
///
/// The folder pattern cannot be narrowed the way the file one is: a directory has no extension to
/// match on, and whether it is a package is a question about the search paths rather than about its
/// name. So every folder rename is asked about, and the ones that turn out not to be packages cost
/// one request that answers with no edits.
fn will_rename_registration() -> lsp_types::FileOperationRegistrationOptions {
fn filter(
glob: &str,
kind: lsp_types::FileOperationPatternKind,
) -> lsp_types::FileOperationFilter {
lsp_types::FileOperationFilter {
scheme: Some("file".to_string()),
pattern: lsp_types::FileOperationPattern {
glob: glob.to_string(),
matches: Some(kind),
options: None,
},
}
}

lsp_types::FileOperationRegistrationOptions {
filters: vec![
filter(
"**/*.{py,pyi,by,byi}",
lsp_types::FileOperationPatternKind::File,
),
filter("**", lsp_types::FileOperationPatternKind::Folder),
],
}
}

/// Creates the default [`DiagnosticOptions`] for the server.
pub(crate) fn server_diagnostic_options(workspace_diagnostics: bool) -> DiagnosticOptions {
DiagnosticOptions {
Expand Down
8 changes: 8 additions & 0 deletions crates/ty_server/src/server/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ pub(super) fn request(req: server::Request) -> Task {
>(
req, BackgroundSchedule::Worker
),
// The client is holding a file move open waiting for this, so it is scheduled like the
// other things a person is watching for rather than as background work.
requests::WillRenameFilesRequestHandler::METHOD => {
background_request_task::<requests::WillRenameFilesRequestHandler>(
req,
BackgroundSchedule::LatencySensitive,
)
}
requests::PrepareTypeHierarchyRequestHandler::METHOD => background_document_request_task::<
requests::PrepareTypeHierarchyRequestHandler,
>(
Expand Down
2 changes: 2 additions & 0 deletions crates/ty_server/src/server/api/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ mod signature_help;
mod transpile;
mod type_hierarchy_subtypes;
mod type_hierarchy_supertypes;
mod will_rename_files;
mod workspace_diagnostic;
mod workspace_symbols;

Expand Down Expand Up @@ -79,5 +80,6 @@ pub(super) use signature_help::SignatureHelpRequestHandler;
pub(super) use transpile::TranspileRequestHandler;
pub(super) use type_hierarchy_subtypes::TypeHierarchySubtypesRequestHandler;
pub(super) use type_hierarchy_supertypes::TypeHierarchySupertypesRequestHandler;
pub(super) use will_rename_files::WillRenameFilesRequestHandler;
pub(super) use workspace_diagnostic::WorkspaceDiagnosticRequestHandler;
pub(super) use workspace_symbols::WorkspaceSymbolRequestHandler;
115 changes: 115 additions & 0 deletions crates/ty_server/src/server/api/requests/will_rename_files.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
use lsp_types::{RenameFilesParams, TextEdit, Uri, WillRenameFilesRequest, WorkspaceEdit};
use ruff_db::files::FileRange;
use ruff_db::system::SystemPathBuf;
use ruff_text_size::Ranged;
use rustc_hash::FxHashMap;
use ty_ide::{FileMove, module_rename_edits};

use crate::document::FileRangeExt;
use crate::server::api::traits::{
BackgroundRequestHandler, RequestHandler, RetriableRequestHandler,
};
use crate::session::SessionSnapshot;
use crate::session::client::Client;

/// `workspace/willRenameFiles` — the edits that keep imports working across a rename the editor is
/// about to perform.
///
/// The client asks *before* it moves anything, applies whatever comes back, and only then does the
/// move. That ordering is what makes the answer computable at all: the old path still holds the
/// file, so the module it is today can be resolved, while the new path is just a path — see
/// [`ty_module_resolver::path_to_module_name`].
///
/// A rename that changes no module's name — a `README.md`, a directory no search path covers —
/// answers `None` rather than an empty edit, which is what tells the client to get on with the move
/// without showing the user an empty preview.
pub(crate) struct WillRenameFilesRequestHandler;

impl RequestHandler for WillRenameFilesRequestHandler {
type RequestType = WillRenameFilesRequest;
}

impl BackgroundRequestHandler for WillRenameFilesRequestHandler {
fn run(
snapshot: &SessionSnapshot,
_client: &Client,
params: RenameFilesParams,
) -> crate::server::Result<Option<WorkspaceEdit>> {
let moves: Vec<FileMove> = params
.files
.iter()
.filter_map(|rename| {
Some(FileMove {
old_path: system_path(&rename.old_uri)?,
new_path: system_path(&rename.new_uri)?,
})
})
.collect();

if moves.is_empty() {
return Ok(None);
}

let mut changes: FxHashMap<Uri, Vec<TextEdit>> = FxHashMap::default();

// Every project, because a rename in one workspace folder can move a module that another
// folder imports; a project the paths have nothing to do with contributes nothing, since
// the paths resolve to no module of its.
for db in snapshot.projects() {
let result = module_rename_edits(db, &moves);

for skipped in &result.skipped {
// Not an error: the rename can still go ahead, and this is the one import the user
// will have to look at themselves. Logged with its location so that "which line?"
// has an answer that does not involve searching the project.
tracing::info!(
"willRenameFiles: leaving an import in {} alone ({:?}); it would need a \
different statement to name the module's new home",
skipped.file.path(db),
skipped.reason,
);
}

for file_edit in result.edits {
let range = FileRange::new(file_edit.file, file_edit.edit.range());
let Some(location) = range
.to_lsp_range(db, snapshot.position_encoding())
.and_then(|range| range.to_location())
else {
continue;
};
changes.entry(location.uri).or_default().push(TextEdit {
range: location.range,
new_text: file_edit.edit.content().unwrap_or_default().to_string(),
});
}
}

if changes.is_empty() {
return Ok(None);
}

Ok(Some(WorkspaceEdit {
changes: Some(changes.into_iter().collect()),
document_changes: None,
change_annotations: None,
}))
}
}

/// The path a `file:` URI names, or nothing for a URI that is not one.
///
/// A client may send `untitled:` for a buffer that has never been saved, which cannot be a module
/// and cannot be moved; those are dropped rather than refused, so a mixed rename still gets the
/// edits for the files that do exist.
fn system_path(uri: &Uri) -> Option<SystemPathBuf> {
SystemPathBuf::from_path_buf(uri.to_file_path().ok()?).ok()
}

impl RetriableRequestHandler for WillRenameFilesRequestHandler {
/// A rename is a one-shot gesture the user is waiting on, and the client is holding the file
/// move until it answers. Retrying on a database change is the right trade here for the same
/// reason it is for the other whole-project requests: the alternative is telling the editor to
/// go ahead with a rename this never got to check.
const RETRY_ON_CANCELLATION: bool = true;
}
1 change: 1 addition & 0 deletions crates/ty_server/tests/e2e/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ mod rename;
mod semantic_tokens;
mod signature_help;
mod type_hierarchy;
mod will_rename_files;
mod workspace_folders;

use std::collections::{BTreeMap, HashMap, VecDeque};
Expand Down
Loading
Loading