Merge #4125

4125: Avoid lossy OsString conversions r=matklad a=lnicola

This is a bit invasive, and perhaps for not much benefit since non-UTF-8 environment variables don't work anyway.

Co-authored-by: Laurențiu Nicola <[email protected]>

5 files changed, 14 insertions, 9 deletions

diff --git a/crates/ra_hir_expand/src/builtin_macro.rs b/crates/ra_hir_expand/src/builtin_macro.rs
index 2ccebda28..e0fef613d 100644
--- a/crates/ra_hir_expand/src/builtin_macro.rs
+++ b/crates/ra_hir_expand/src/builtin_macro.rs
@@ -358,7 +358,7 @@ fn env_expand(
     // However, we cannot use an empty string here, because for
     // `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` will become
     // `include!("foo.rs"), which might go to infinite loop
-    let s = get_env_inner(db, arg_id, &key).unwrap_or("__RA_UNIMPLEMENTATED__".to_string());
+    let s = get_env_inner(db, arg_id, &key).unwrap_or_else(|| "__RA_UNIMPLEMENTATED__".to_string());
     let expanded = quote! { #s };
 
     Ok((expanded, FragmentKind::Expr))
diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs
index 03f2629da..731cbd291 100644
--- a/crates/ra_project_model/src/lib.rs
+++ b/crates/ra_project_model/src/lib.rs
@@ -285,8 +285,10 @@ impl ProjectWorkspace {
                         let mut env = Env::default();
                         let mut extern_source = ExternSource::default();
                         if let Some(out_dir) = &krate.out_dir {
-                            // FIXME: We probably mangle non UTF-8 paths here, figure out a better solution
-                            env.set("OUT_DIR", out_dir.to_string_lossy().to_string());
+                            // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!()
+                            if let Some(out_dir) = out_dir.to_str().map(|s| s.to_owned()) {
+                                env.set("OUT_DIR", out_dir);
+                            }
                             if let Some(&extern_source_id) = extern_source_roots.get(out_dir) {
                                 extern_source.set_extern_path(&out_dir, extern_source_id);
                             }
@@ -402,8 +404,10 @@ impl ProjectWorkspace {
                             let mut env = Env::default();
                             let mut extern_source = ExternSource::default();
                             if let Some(out_dir) = &cargo[pkg].out_dir {
-                                // FIXME: We probably mangle non UTF-8 paths here, figure out a better solution
-                                env.set("OUT_DIR", out_dir.to_string_lossy().to_string());
+                                // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!()
+                                if let Some(out_dir) = out_dir.to_str().map(|s| s.to_owned()) {
+                                    env.set("OUT_DIR", out_dir);
+                                }
                                 if let Some(&extern_source_id) = extern_source_roots.get(out_dir) {
                                     extern_source.set_extern_path(&out_dir, extern_source_id);
                                 }
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index fc4c77f8a..d993c4146 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -696,7 +696,7 @@ fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state:
         let uri = match url_from_path_with_drive_lowercasing(&path) {
             Ok(uri) => uri,
             Err(err) => {
-                log::error!("Couldn't convert path to url ({}): {:?}", err, path.to_string_lossy());
+                log::error!("Couldn't convert path to url ({}): {}", err, path.display());
                 continue;
             }
         };
diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs
index 2303ebfdb..c2c1a23cd 100644
--- a/crates/rust-analyzer/src/main_loop/handlers.rs
+++ b/crates/rust-analyzer/src/main_loop/handlers.rs
@@ -384,7 +384,7 @@ pub fn handle_runnables(
         args: check_args,
         extra_args: Vec::new(),
         env: FxHashMap::default(),
-        cwd: workspace_root.map(|root| root.to_string_lossy().to_string()),
+        cwd: workspace_root.map(|root| root.to_owned()),
     });
     Ok(res)
 }
@@ -984,7 +984,7 @@ fn to_lsp_runnable(
             m.insert("RUST_BACKTRACE".to_string(), "short".to_string());
             m
         },
-        cwd: world.workspace_root_for(file_id).map(|root| root.to_string_lossy().to_string()),
+        cwd: world.workspace_root_for(file_id).map(|root| root.to_owned()),
     })
 }
 
diff --git a/crates/rust-analyzer/src/req.rs b/crates/rust-analyzer/src/req.rs
index b8b627e28..ae3448892 100644
--- a/crates/rust-analyzer/src/req.rs
+++ b/crates/rust-analyzer/src/req.rs
@@ -17,6 +17,7 @@ pub use lsp_types::{
     SignatureHelp, SymbolKind, TextDocumentEdit, TextDocumentPositionParams, TextEdit,
     WorkDoneProgressParams, WorkspaceEdit, WorkspaceSymbolParams,
 };
+use std::path::PathBuf;
 
 pub enum AnalyzerStatus {}
 
@@ -141,7 +142,7 @@ pub struct Runnable {
     pub args: Vec<String>,
     pub extra_args: Vec<String>,
     pub env: FxHashMap<String, String>,
-    pub cwd: Option<String>,
+    pub cwd: Option<PathBuf>,
 }
 
 #[derive(Deserialize, Serialize, Debug)]