fix Goto-definition on stale file can result in the wrong location #1332

[asukaminato0721]

Summary

Fixes #1332

stale on-disk source files can no longer leave textDocument/definition pointing at old locations or returning null.

the definition path now detects when the request document has drifted from disk, invalidates all known filesystem-backed modules, reruns the handle, and, if the first lookup is still empty, retries once with that source file refreshed from disk.

Test Plan

add test

[Copilot]

Pull request overview

This PR fixes an LSP textDocument/definition edge case where the language server’s view of a file can drift from the on-disk contents (e.g., after rebases/external edits), leading to missing or incorrect go-to-definition locations. The server now detects request-file drift from disk, invalidates filesystem-backed modules, re-runs analysis, and (when needed) retries definition with refreshed contents.

Changes:

• Add stale-on-disk detection and recovery logic in the non-WASM LSP server’s go-to-definition flow (including module invalidation + retry).
• Refactor go-to-definition logic to separate “compute response” from the higher-level retry/refresh behavior.
• Add a regression test covering recovery when both the requesting file and the target definition file moved on disk without LSP change notifications.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pyrefly/lib/lsp/non_wasm/server.rs	Detects stale on-disk contents during go-to-definition, invalidates filesystem modules, re-runs analysis, and retries with refreshed source when necessary.
pyrefly/lib/test/lsp/lsp_interaction/definition.rs	Adds a regression test ensuring go-to-definition recovers after on-disk edits without corresponding LSP updates.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

stale_definition_request_disk_contents unconditionally reads the entire file from disk for every textDocument/definition request (before even checking whether the module is already known to be filesystem-backed / whether the URI is a real file). This adds synchronous disk I/O to a latency-sensitive LSP path and can be expensive for large files or network filesystems. Consider short-circuiting for non-file:// URIs / unsaved paths, and/or using a cheaper change detector first (e.g., file metadata/mtime+size caching) before falling back to read_to_string only when the file appears to have changed.

Suggested change

	let disk_contents = fs_anyhow::read_to_string(path).ok()?;
	if let Some(open_file) = self.open_files.read().get(path) {
	return match &**open_file {
	LspFile::Source(contents) if contents.as_str() != disk_contents.as_str() => {
	Some(disk_contents)
	}
	LspFile::Source(_) | LspFile::Notebook(_) => None,
	};
	}
	let module_info = transaction.get_module_info(handle)?;
	if module_info.is_notebook() || module_info.contents().as_str() == disk_contents.as_str() {
	let module_info = transaction.get_module_info(handle)?;
	if module_info.is_notebook() {
	return None;
	}
	if !matches!(handle.path().details(), ModulePathDetails::FileSystem(_)) {
	return None;
	}
	let expected_contents = if let Some(open_file) = self.open_files.read().get(path) {
	match &**open_file {
	LspFile::Source(contents) => contents.as_str(),
	LspFile::Notebook(_) => return None,
	}
	} else {
	module_info.contents().as_str()
	};
	let metadata = std::fs::metadata(path).ok()?;
	if !metadata.is_file() {
	return None;
	}
	if metadata.len() != expected_contents.len() as u64 {
	return fs_anyhow::read_to_string(path).ok();
	}
	let disk_contents = fs_anyhow::read_to_string(path).ok()?;
	if expected_contents == disk_contents.as_str() {

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅