From c1da7d6f561c08028a594ee7d6ec39ee8f9cd333 Mon Sep 17 00:00:00 2001 From: WillLillis Date: Thu, 21 Nov 2024 21:44:19 -0500 Subject: [PATCH] fix: Remove leading slash from paths on windows with colon drive separators actually fix the problem --- asm-lsp/handle.rs | 43 +++++++++++++++++++++++++++++------------- asm-lsp/lsp.rs | 48 ++++++++++++++++++++++++++++++++++++----------- asm-lsp/types.rs | 2 +- 3 files changed, 68 insertions(+), 25 deletions(-) diff --git a/asm-lsp/handle.rs b/asm-lsp/handle.rs index 88fc70d0..fafae0f4 100644 --- a/asm-lsp/handle.rs +++ b/asm-lsp/handle.rs @@ -1,8 +1,6 @@ -use std::path::PathBuf; - use anyhow::{anyhow, Result}; use compile_commands::{CompilationDatabase, SourceFile}; -use log::{info, warn}; +use log::{error, info, warn}; use lsp_server::{Connection, Message, Notification, Request, RequestId, Response}; use lsp_types::{ notification::{ @@ -22,8 +20,9 @@ use tree_sitter::Parser; use crate::{ apply_compile_cmd, get_comp_resp, get_compile_cmd_for_req, get_default_compile_cmd, get_document_symbols, get_goto_def_resp, get_hover_resp, get_ref_resp, get_sig_help_resp, - get_word_from_pos_params, send_empty_resp, text_doc_change_to_ts_edit, CompletionItems, Config, - ConfigOptions, DocumentStore, NameToInstructionMap, RootConfig, ServerStore, TreeEntry, + get_word_from_pos_params, process_uri, send_empty_resp, text_doc_change_to_ts_edit, + CompletionItems, Config, ConfigOptions, DocumentStore, NameToInstructionMap, RootConfig, + ServerStore, TreeEntry, UriConversion, }; /// Handles `Request`s from the lsp client @@ -533,17 +532,35 @@ pub fn handle_diagnostics( cfg: &Config, compile_cmds: &CompilationDatabase, ) -> Result<()> { - let req_source_path = PathBuf::from(uri.path().as_str()); + let req_source_path = match process_uri(uri) { + UriConversion::Canonicalized(p) => p, + UriConversion::Unchecked(p) => { + error!( + "Failed to canonicalize request path {}, using {}", + uri.path().as_str(), + p.display() + ); + p + } + }; let source_entries = compile_cmds.iter().filter(|entry| match entry.file { SourceFile::File(ref file) => { - if file.is_absolute() { - file.eq(&req_source_path) - } else if let Ok(source_path) = file.canonicalize() { - source_path.eq(&req_source_path) - } else { - false - } + file.canonicalize().map_or(false, |source_path| { + // HACK: See comment inside `process_uri` + let cleaned_path = if cfg!(windows) { + #[allow(clippy::option_if_let_else)] + if let Some(tmp) = source_path.to_str().unwrap().strip_prefix("\\\\?\\") { + warn!("Stripping Windows canonicalization prefix \"\\\\?\\\" from path"); + tmp.into() + } else { + source_path + } + } else { + source_path + }; + cleaned_path.eq(&req_source_path) + }) } SourceFile::All => true, }); diff --git a/asm-lsp/lsp.rs b/asm-lsp/lsp.rs index ccce2e97..d4ea13d6 100644 --- a/asm-lsp/lsp.rs +++ b/asm-lsp/lsp.rs @@ -153,20 +153,46 @@ pub enum UriConversion { /// Will panic if `uri` cannot be interpreted as valid utf-8 after being percent-decoded #[must_use] pub fn process_uri(uri: &Uri) -> UriConversion { - let clean_path: String = url_escape::percent_encoding::percent_decode_str(uri.path().as_str()) - .decode_utf8() - .unwrap_or_else(|e| { - panic!( - "Invalid encoding for uri \"{}\" -- {e}", - uri.path().as_str() - ) - }) - .to_string(); + let mut clean_path: String = + url_escape::percent_encoding::percent_decode_str(uri.path().as_str()) + .decode_utf8() + .unwrap_or_else(|e| { + panic!( + "Invalid encoding for uri \"{}\" -- {e}", + uri.path().as_str() + ) + }) + .to_string(); + + // HACK: On Windows, sometimes a leading '/', e.g. /C:/Users/foo/bar/... + // is passed as part of the path -- Stuff like Git bash and MSYS2 will accept + // /C/Users/foo/bar/..., but *not* if the colon is present. Vanila windows + // will not accept a leading slash at all, but requires the colon after the + // drive letter, like C:/Users/foo/... So we do our best to clean up here + if cfg!(windows) && clean_path.contains(':') { + clean_path = clean_path.strip_prefix('/').unwrap_or(&clean_path).into(); + } let Ok(path) = PathBuf::from_str(&clean_path); path.canonicalize() .map_or(UriConversion::Unchecked(path), |canonicalized| { - UriConversion::Canonicalized(canonicalized) + // HACK: On Windows, when a path is canonicalized, sometimes it gets prefixed + // with "\\?\" -- https://stackoverflow.com/questions/41233684/why-does-my-canonicalized-path-get-prefixed-with + // That's great and all, but it looks like common tools (like gcc) don't handle + // this correctly, and you get something like the following: + // Error: can't open //test.s for reading: No such file or directory + // The solution? Just cut out the prefix and hope that doesn't break anything else + if cfg!(windows) { + #[allow(clippy::option_if_let_else)] + if let Some(tmp) = canonicalized.to_str().unwrap().strip_prefix("\\\\?\\") { + warn!("Stripping Windows canonicalization prefix \"\\\\?\\\" from path"); + UriConversion::Canonicalized(tmp.into()) + } else { + UriConversion::Canonicalized(canonicalized) + } + } else { + UriConversion::Canonicalized(canonicalized) + } }) } @@ -459,7 +485,7 @@ pub fn get_compile_cmd_for_req( UriConversion::Canonicalized(p) => p, UriConversion::Unchecked(p) => { error!( - "Failed to canonicalized request path {}, using {}", + "Failed to canonicalize request path {}, using {}", req_uri.path().as_str(), p.display() ); diff --git a/asm-lsp/types.rs b/asm-lsp/types.rs index 46ecd355..42711b65 100644 --- a/asm-lsp/types.rs +++ b/asm-lsp/types.rs @@ -1011,7 +1011,7 @@ impl RootConfig { UriConversion::Canonicalized(p) => p, UriConversion::Unchecked(p) => { warn!( - "Failed to canonicalized request path {}, using {}", + "Failed to canonicalize request path {}, using {}", req_uri.path().as_str(), p.display() );