diff --git a/pyrefly/lib/lsp/non_wasm/server.rs b/pyrefly/lib/lsp/non_wasm/server.rs index d1be83be33..c45acc3fa8 100644 --- a/pyrefly/lib/lsp/non_wasm/server.rs +++ b/pyrefly/lib/lsp/non_wasm/server.rs @@ -915,6 +915,7 @@ pub struct Server { /// should be mapped through here in case they correspond to a cell. open_notebook_cells: RwLock>, open_files: RwLock>>, + open_files_with_unsaved_changes: Mutex>, /// Last published fingerprint for unversioned file-backed workspace diagnostics. published_workspace_diagnostics: Mutex>, /// Tracks URIs (including virtual/untitled ones) to synthetic on-disk paths so we can @@ -2586,6 +2587,7 @@ impl Server { state: State::new(config_finder, thread_count), open_notebook_cells: RwLock::new(HashMap::new()), open_files: RwLock::new(HashMap::new()), + open_files_with_unsaved_changes: Mutex::new(HashSet::new()), published_workspace_diagnostics: Mutex::new(HashMap::new()), unsaved_file_tracker: UnsavedFileTracker::new(), indexed_configs: Mutex::new(HashSet::new()), @@ -3488,6 +3490,7 @@ impl Server { fn did_save(&self, url: Url) { if let Some(path) = self.path_for_uri(&url) { + self.open_files_with_unsaved_changes.lock().remove(&path); self.invalidate(TelemetryEventKind::InvalidateDisk, None, move |t| { t.invalidate_disk(&[path]) }) @@ -3525,6 +3528,7 @@ impl Server { } else { None }; + self.open_files_with_unsaved_changes.lock().remove(&path); self.version_info.lock().insert(path.clone(), version); self.open_files.write().insert(path.clone(), contents); self.queue_source_db_rebuild_and_recheck(telemetry, telemetry_event, false); @@ -3590,6 +3594,9 @@ impl Server { params.content_changes, ))); drop(lock); + self.open_files_with_unsaved_changes + .lock() + .insert(file_path.clone()); // Update version_info only after the mutation has fully succeeded. self.version_info.lock().insert(file_path.clone(), version); if !subsequent_mutation { @@ -3752,6 +3759,9 @@ impl Server { let new_notebook = Arc::new(LspNotebook::new(ruff_notebook, notebook_document)); *original = Arc::new(LspFile::Notebook(new_notebook)); drop(lock); + self.open_files_with_unsaved_changes + .lock() + .insert(file_path.clone()); // Update version_info only after the mutation has fully succeeded, so // that on error the version stays at the old value and subsequent // notifications operate against consistent state. @@ -3789,6 +3799,28 @@ impl Server { config_changed || files_added_or_removed } + fn refresh_clean_open_files_from_disk(&self, events: &CategorizedEvents) { + let unsaved = self.open_files_with_unsaved_changes.lock().clone(); + let mut open_files = self.open_files.write(); + for path in events.modified.iter() { + if unsaved.contains(path) { + continue; + } + let Some(open_file) = open_files.get_mut(path) else { + continue; + }; + let LspFile::Source(current_contents) = open_file.as_ref() else { + continue; + }; + let Ok(updated_contents) = std::fs::read_to_string(path) else { + continue; + }; + if current_contents.as_str() != updated_contents { + *open_file = Arc::new(LspFile::from_source(updated_contents)); + } + } + } + fn did_change_watched_files( &self, params: DidChangeWatchedFilesParams, @@ -3828,6 +3860,8 @@ impl Server { let should_requery_build_system = should_requery_build_system(&events); + self.refresh_clean_open_files_from_disk(&events); + // Rewatch files if necessary (config changed, files added/removed, etc.) if Self::should_rewatch(&events) { info!("[Pyrefly] Re-registering file watchers"); @@ -3921,6 +3955,7 @@ impl Server { }, } drop(open_files); + self.open_files_with_unsaved_changes.lock().remove(&path); self.unsaved_file_tracker.forget_uri_path(&url); self.queue_source_db_rebuild_and_recheck(telemetry, telemetry_event, false); self.recheck_queue.queue_task( diff --git a/pyrefly/lib/module/finder.rs b/pyrefly/lib/module/finder.rs index 86fe158b18..6b35a86531 100644 --- a/pyrefly/lib/module/finder.rs +++ b/pyrefly/lib/module/finder.rs @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +use std::fs::read_link; use std::iter; use std::path::Path; use std::path::PathBuf; @@ -77,8 +78,29 @@ enum FindResult { CompiledModule(PathBuf), } +fn symlink_target(path: &Path) -> Option { + if !path + .symlink_metadata() + .map(|metadata| metadata.file_type().is_symlink()) + .unwrap_or(false) + { + return None; + } + let target = read_link(path).ok()?; + if target.is_absolute() { + Some(target) + } else { + Some(path.parent().unwrap_or_else(|| Path::new("")).join(target)) + } +} + +fn resolve_symlink_path(path: PathBuf) -> PathBuf { + symlink_target(&path).unwrap_or(path) +} + impl FindResult { fn single_file(path: PathBuf, ext: &str) -> Self { + let path = resolve_symlink_path(path); if ext == "pyi" { Self::SingleFilePyiModule(path) } else { @@ -86,6 +108,25 @@ impl FindResult { } } + fn regular_package(init_path: PathBuf, next_root: PathBuf) -> Self { + if let Some(next_root) = symlink_target(&next_root) { + let init_name = init_path + .file_name() + .expect("regular package init path should have a file name"); + Self::RegularPackage(next_root.join(init_name), next_root) + } else { + Self::RegularPackage(resolve_symlink_path(init_path), next_root) + } + } + + fn namespace_package(path: PathBuf) -> Self { + Self::NamespacePackage(Vec1::new(resolve_symlink_path(path))) + } + + fn compiled_module(path: PathBuf) -> Self { + Self::CompiledModule(resolve_symlink_path(path)) + } + fn style(&self) -> Option { match self { Self::SingleFilePyiModule(_) => Some(ModuleStyle::Interface), @@ -163,7 +204,7 @@ fn find_one_part_in_root( for candidate_init_suffix in candidate_init_suffixes { let init_path = candidate_dir.join(candidate_init_suffix); if timed_stat(timing, || init_path.exists()) { - return Some(FindResult::RegularPackage(init_path, candidate_dir)); + return Some(FindResult::regular_package(init_path, candidate_dir)); } else if let Some(v) = phantom_paths.as_deref_mut() { v.push(init_path); } @@ -199,7 +240,7 @@ fn find_one_part_in_root( for candidate_compiled_suffix in COMPILED_FILE_SUFFIXES { let candidate_path = root.join(format!("{name}.{candidate_compiled_suffix}")); if timed_stat(timing, || candidate_path.exists()) { - let result = FindResult::CompiledModule(candidate_path); + let result = FindResult::compiled_module(candidate_path); if let Some(filter) = style_filter { // compiled files are considered executable match filter { @@ -215,7 +256,7 @@ fn find_one_part_in_root( // Finally check if `name` corresponds to a namespace package. if dir_exists { - return Some(FindResult::NamespacePackage(Vec1::new(candidate_dir))); + return Some(FindResult::namespace_package(candidate_dir)); } else if let Some(v) = phantom_paths.as_deref_mut() { v.push(candidate_dir); } @@ -280,21 +321,20 @@ fn find_one_part_prefix<'a>( if name.starts_with(prefix.as_str()) { // Check if it's a regular package if path.is_dir() { + let mut found_regular_package = false; for candidate_init_suffix in ["__init__.pyi", "__init__.py"] { let init_path = path.join(candidate_init_suffix); if init_path.exists() { results.push(( - FindResult::RegularPackage(init_path, path.clone()), + FindResult::regular_package(init_path, path.clone()), ModuleName::from_str(name), )); + found_regular_package = true; break; } } - if !results.iter().any(|r| match r { - (FindResult::RegularPackage(_, p), _) => p == &path, - _ => false, - }) { + if !found_regular_package { namespace_roots .entry(ModuleName::from_str(name)) .or_default() @@ -311,7 +351,7 @@ fn find_one_part_prefix<'a>( )); } else if COMPILED_FILE_SUFFIXES.contains(&ext) { results.push(( - FindResult::CompiledModule(path.clone()), + FindResult::compiled_module(path.clone()), ModuleName::from_str(stem), )); } @@ -325,6 +365,13 @@ fn find_one_part_prefix<'a>( // Add namespace packages to results for (name, roots) in namespace_roots { if let Ok(namespace_roots) = Vec1::try_from_vec(roots) { + let namespace_roots = Vec1::try_from_vec( + namespace_roots + .into_iter() + .map(resolve_symlink_path) + .collect(), + ) + .expect("canonicalizing a non-empty namespace package should stay non-empty"); results.push((FindResult::NamespacePackage(namespace_roots), name)); } } @@ -2452,6 +2499,75 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn test_find_module_resolves_symlink_to_real_path() { + use std::os::unix::fs::symlink; + + let tempdir = tempfile::tempdir().unwrap(); + let root = tempdir.path(); + let target = root.join("test_module.py"); + TestPath::setup_test_directory( + root, + vec![TestPath::file_with_contents( + "test_module.py", + "def hello(name):\n pass\n", + )], + ); + symlink(&target, root.join("sym.py")).unwrap(); + + assert_eq!( + find_module( + ModuleName::from_str("sym"), + [root.to_path_buf()].iter(), + &mut vec![], + None, + None, + false, + &mut None, + None, + ) + .unwrap(), + FindingOrError::new_finding(ModulePath::filesystem(target)) + ); + } + + #[cfg(unix)] + #[test] + fn test_find_module_preserves_symlinked_search_root_path() { + use std::fs::create_dir; + use std::os::unix::fs::symlink; + + let tempdir = tempfile::tempdir().unwrap(); + let root = tempdir.path().join("root"); + let alias = tempdir.path().join("alias"); + create_dir(&root).unwrap(); + TestPath::setup_test_directory( + &root, + vec![TestPath::file_with_contents( + "test_module.py", + "def hello(name):\n pass\n", + )], + ); + symlink(&root, &alias).unwrap(); + let module_path = alias.join("test_module.py"); + + assert_eq!( + find_module( + ModuleName::from_str("test_module"), + [alias].iter(), + &mut vec![], + None, + None, + false, + &mut None, + None, + ) + .unwrap(), + FindingOrError::new_finding(ModulePath::filesystem(module_path)) + ); + } + fn get_config(source: ConfigSource) -> ConfigFile { let mut interpreters = Interpreters::default(); interpreters.skip_interpreter_query = true; diff --git a/pyrefly/lib/state/load.rs b/pyrefly/lib/state/load.rs index bca5b47058..6cffdc9007 100644 --- a/pyrefly/lib/state/load.rs +++ b/pyrefly/lib/state/load.rs @@ -84,6 +84,20 @@ pub struct Load { } impl Load { + fn load_open_filesystem_path( + path: &Path, + memory_lookup: &MemoryFilesLookup, + ) -> Option> { + if let Some(contents) = memory_lookup.get(path) { + return Some(Arc::clone(contents)); + } + let canonical_path = path.canonicalize().ok()?; + if canonical_path == path { + return None; + } + memory_lookup.get(&canonical_path).map(Arc::clone) + } + /// Return the code for this module, optional notebook cell mapping, and whether there was an error while loading (a self-error). pub fn load_from_path( path: &ModulePath, @@ -91,7 +105,13 @@ impl Load { timing: Option<&TransactionTimingCounters>, ) -> (FileContents, Option) { let res = match path.details() { - ModulePathDetails::FileSystem(path) => Self::load_from_filesystem(path, timing), + ModulePathDetails::FileSystem(path) => { + if let Some(contents) = Self::load_open_filesystem_path(path, memory_lookup) { + Ok((*contents).clone()) + } else { + Self::load_from_filesystem(path, timing) + } + } ModulePathDetails::Namespace(_) => Ok(FileContents::from_source("".to_owned())), ModulePathDetails::Memory(path) => memory_lookup .get(path) diff --git a/pyrefly/lib/state/state.rs b/pyrefly/lib/state/state.rs index ed2a256fc9..d71c9920aa 100644 --- a/pyrefly/lib/state/state.rs +++ b/pyrefly/lib/state/state.rs @@ -2039,8 +2039,18 @@ impl<'a> Transaction<'a> { /// Invalidate based on what a watcher told you. pub fn invalidate_events(&mut self, events: &CategorizedEvents) { + let modified_symlink = events.modified.iter().any(|path| { + std::fs::symlink_metadata(path) + .map(|metadata| metadata.file_type().is_symlink()) + .unwrap_or(false) + }); + // If any files were added or removed, we need to invalidate the find step. - if !events.created.is_empty() || !events.removed.is_empty() || !events.unknown.is_empty() { + if !events.created.is_empty() + || !events.removed.is_empty() + || !events.unknown.is_empty() + || modified_symlink + { self.invalidate_find(); } @@ -2156,7 +2166,13 @@ impl<'a> Transaction<'a> { for (path, contents) in files { if self.memory_lookup().get(&path) != contents.as_ref() { self.data.memory_overlay.set(path.clone(), contents); - changed.insert(ModulePath::memory(path)); + changed.insert(ModulePath::memory(path.clone())); + changed.insert(ModulePath::filesystem(path.clone())); + if let Ok(canonical_path) = path.canonicalize() + && canonical_path != path + { + changed.insert(ModulePath::filesystem(canonical_path)); + } } } self.stats.lock().set_memory_dirty = changed.len(); diff --git a/pyrefly/lib/test/lsp/lsp_interaction/diagnostic.rs b/pyrefly/lib/test/lsp/lsp_interaction/diagnostic.rs index 1c3b1e81e0..81fd664905 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/diagnostic.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/diagnostic.rs @@ -403,10 +403,10 @@ fn test_edit_file_during_recheck() { let d_contents = std::fs::read_to_string(&d_path).unwrap(); let edited_d_contents = format!("{}\nY: int = ''\nZ: int = ''", d_contents); interaction.client.did_change("d.py", &edited_d_contents); - // Streamed errors are replaced w/ diagnostics based on old state + edit + // Diagnostics now reflect the already-updated dependency state plus the local edit. interaction .client - .expect_publish_diagnostics_must_have_error_count(d_path.clone(), 2) + .expect_publish_diagnostics_must_have_error_count(d_path.clone(), 3) .expect("Failed to receive streamed diagnostics for first edit"); // After recheck completes, error count reflects new state + edit interaction.continue_recheck(); diff --git a/pyrefly/lib/test/lsp/lsp_interaction/did_change.rs b/pyrefly/lib/test/lsp/lsp_interaction/did_change.rs index 303d5b59f6..05a8787be3 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/did_change.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/did_change.rs @@ -7,6 +7,7 @@ use lsp_types::Url; use lsp_types::notification::DidChangeTextDocument; +use pyrefly::commands::lsp::IndexingMode; use serde_json::json; use crate::object_model::InitializeSettings; @@ -182,3 +183,45 @@ fn test_text_document_did_change_unicode() { interaction.shutdown().unwrap(); } + +#[cfg(unix)] +#[test] +fn test_text_document_did_change_updates_symlinked_imports() { + use std::os::unix::fs::symlink; + + let root = tempfile::tempdir().unwrap(); + let module_path = root.path().join("test_module.py"); + let symlink_path = root.path().join("sym.py"); + let importer_path = root.path().join("test_import.py"); + std::fs::write(&module_path, "def hello(name: str) -> None:\n pass\n").unwrap(); + symlink(&module_path, &symlink_path).unwrap(); + std::fs::write(&importer_path, "from sym import hello\nhello(\"John\")\n").unwrap(); + + let mut interaction = LspInteraction::new_with_indexing_mode(IndexingMode::LazyBlocking); + interaction.set_root(root.path().to_path_buf()); + interaction + .initialize(InitializeSettings { + configuration: Some(Some( + json!([{"pyrefly": {"displayTypeErrors": "force-on"}}]), + )), + workspace_folders: Some(vec![( + "test".to_owned(), + Url::from_file_path(root.path()).unwrap(), + )]), + ..Default::default() + }) + .unwrap(); + + interaction.client.did_open("test_module.py"); + interaction.client.did_open("test_import.py"); + interaction.client.did_change( + "test_module.py", + "def hello(name: str, times: int) -> None:\n pass\n", + ); + interaction + .client + .expect_publish_diagnostics_eventual_error_count(importer_path, 1) + .unwrap(); + + interaction.shutdown().unwrap(); +} diff --git a/pyrefly/lib/test/lsp/lsp_interaction/references.rs b/pyrefly/lib/test/lsp/lsp_interaction/references.rs index 9b74ff48f5..50d6e45b47 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/references.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/references.rs @@ -637,11 +637,11 @@ fn test_references_after_file_modification_with_config() { .references("foo.py", 6, 17, true) .expect_response(json!([ { - "range": {"start":{"line":6,"character":6},"end":{"character":9,"line":6}}, + "range": {"start":{"line":8,"character":6},"end":{"character":9,"line":8}}, "uri": Url::from_file_path(bar.clone()).unwrap().to_string() }, { - "range": {"start":{"line":10,"character":0},"end":{"character":3,"line":10}}, + "range": {"start":{"line":12,"character":0},"end":{"character":3,"line":12}}, "uri": Url::from_file_path(bar.clone()).unwrap().to_string() }, { @@ -715,6 +715,34 @@ fn test_references_after_file_modification_with_line_offset_with_config() { "range": {"start":{"line":12,"character":0},"end":{"character":3,"line":12}}, "uri": Url::from_file_path(bar.clone()).unwrap().to_string() }, + { + "range": {"start":{"line":6,"character":16},"end":{"line":6, "character":19}}, + "uri": Url::from_file_path(root_path.join("foo.py")).unwrap().to_string() + }, + { + "range": {"start":{"line":8,"character":0},"end":{"line":8,"character":3}}, + "uri": Url::from_file_path(root_path.join("foo.py")).unwrap().to_string() + }, + { + "range": {"start":{"line":9,"character":4},"end":{"line":9,"character":7}}, + "uri": Url::from_file_path(root_path.join("foo.py")).unwrap().to_string() + }, + { + "range": {"start":{"line":5,"character":16},"end":{"line":5,"character":19}}, + "uri": Url::from_file_path(root_path.join("various_imports.py")).unwrap().to_string() + }, + { + "range": {"start":{"line":5,"character":26},"end":{"line":5,"character":29}}, + "uri": Url::from_file_path(root_path.join("various_imports.py")).unwrap().to_string() + }, + { + "range": {"start":{"line":5,"character":16},"end":{"character":19,"line":5}}, + "uri": Url::from_file_path(root_path.join("with_synthetic_bindings.py")).unwrap().to_string() + }, + { + "range": {"start":{"line":10,"character":4},"end":{"character":7,"line":10}}, + "uri": Url::from_file_path(root_path.join("with_synthetic_bindings.py")).unwrap().to_string() + }, ])) .unwrap(); diff --git a/pyrefly/lib/test/tsp/tsp_interaction/get_snapshot.rs b/pyrefly/lib/test/tsp/tsp_interaction/get_snapshot.rs index 493c758830..5877df1cfa 100644 --- a/pyrefly/lib/test/tsp/tsp_interaction/get_snapshot.rs +++ b/pyrefly/lib/test/tsp/tsp_interaction/get_snapshot.rs @@ -115,8 +115,8 @@ y = "hello" tsp.server .did_change_watched_files("changing_test.py", "changed"); - // Wait for the async RecheckFinished event to be processed - tsp.client.expect_any_message(); + // Wait for the async recheck to publish the snapshot change notification. + tsp.client.expect_notification("typeServer/snapshotChanged"); // Get snapshot after async recheck completes tsp.server.get_snapshot();