Skip to content

Commit aa00ddc

Browse files
borsbors
committed
Auto merge of #17770 - Veykril:path-try-from, r=Veykril
internal: Remove AbsPathBuf::TryFrom impl that checks too many things at once #16889 (comment)
2 parents 670a5ab + 758ad25 commit aa00ddc

File tree

15 files changed

+90
-95
lines changed

15 files changed

+90
-95
lines changed

‎Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/paths/src/lib.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Thin wrappers around `std::path`/`camino::path`, distinguishing between absolute and
1+
//! Thin wrappers around [`camino::path`], distinguishing between absolute and
22
//! relative paths.
33
44
use std::{
@@ -8,9 +8,9 @@ use std::{
88
path::{Path, PathBuf},
99
};
1010

11-
pub use camino::*;
11+
pub use camino::{Utf8Component, Utf8Components, Utf8Path, Utf8PathBuf, Utf8Prefix};
1212

13-
/// Wrapper around an absolute [`Utf8PathBuf`].
13+
/// A [`Utf8PathBuf`] that is guaranteed to be absolute.
1414
#[derive(Debug, Clone, Ord, PartialOrd, Eq, Hash)]
1515
pub struct AbsPathBuf(Utf8PathBuf);
1616

@@ -73,16 +73,6 @@ impl TryFrom<Utf8PathBuf> for AbsPathBuf {
7373
}
7474
}
7575

76-
impl TryFrom<PathBuf> for AbsPathBuf {
77-
type Error = PathBuf;
78-
fn try_from(path_buf: PathBuf) -> Result<AbsPathBuf, PathBuf> {
79-
if !path_buf.is_absolute() {
80-
return Err(path_buf);
81-
}
82-
Ok(AbsPathBuf(Utf8PathBuf::from_path_buf(path_buf)?))
83-
}
84-
}
85-
8676
impl TryFrom<&str> for AbsPathBuf {
8777
type Error = Utf8PathBuf;
8878
fn try_from(path: &str) -> Result<AbsPathBuf, Utf8PathBuf> {

‎crates/project-model/src/build_scripts.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,12 @@
66
//! This module implements this second part. We use "build script" terminology
77
//! here, but it covers procedural macros as well.
88
9-
use std::{
10-
cell::RefCell,
11-
io, mem,
12-
path::{self, PathBuf},
13-
process::Command,
14-
};
9+
use std::{cell::RefCell, io, mem, path, process::Command};
1510

1611
use cargo_metadata::{camino::Utf8Path, Message};
1712
use itertools::Itertools;
1813
use la_arena::ArenaMap;
19-
use paths::{AbsPath, AbsPathBuf};
14+
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
2015
use rustc_hash::{FxHashMap, FxHashSet};
2116
use serde::Deserialize;
2217
use toolchain::Tool;
@@ -423,7 +418,7 @@ impl WorkspaceBuildScripts {
423418
utf8_stdout(cmd)
424419
})()?;
425420

426-
let target_libdir = AbsPathBuf::try_from(PathBuf::from(target_libdir))
421+
let target_libdir = AbsPathBuf::try_from(Utf8PathBuf::from(target_libdir))
427422
.map_err(|_| anyhow::format_err!("target-libdir was not an absolute path"))?;
428423
tracing::info!("Loading rustc proc-macro paths from {target_libdir}");
429424

@@ -435,7 +430,8 @@ impl WorkspaceBuildScripts {
435430
let extension = path.extension()?;
436431
if extension == std::env::consts::DLL_EXTENSION {
437432
let name = path.file_stem()?.to_str()?.split_once('-')?.0.to_owned();
438-
let path = AbsPathBuf::try_from(path).ok()?;
433+
let path = AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).ok()?)
434+
.ok()?;
439435
return Some((name, path));
440436
}
441437
}

‎crates/project-model/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use std::{
3737
};
3838

3939
use anyhow::{bail, format_err, Context};
40-
use paths::{AbsPath, AbsPathBuf};
40+
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
4141
use rustc_hash::FxHashSet;
4242

4343
pub use crate::{
@@ -132,8 +132,11 @@ impl ProjectManifest {
132132
.filter_map(Result::ok)
133133
.map(|it| it.path().join("Cargo.toml"))
134134
.filter(|it| it.exists())
135+
.map(Utf8PathBuf::from_path_buf)
136+
.filter_map(Result::ok)
135137
.map(AbsPathBuf::try_from)
136-
.filter_map(|it| it.ok()?.try_into().ok())
138+
.filter_map(Result::ok)
139+
.filter_map(|it| it.try_into().ok())
137140
.collect()
138141
}
139142
}

‎crates/rust-analyzer/src/bin/main.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::{env, fs, path::PathBuf, process::ExitCode, sync::Arc};
1414

1515
use anyhow::Context;
1616
use lsp_server::Connection;
17+
use paths::Utf8PathBuf;
1718
use rust_analyzer::{
1819
cli::flags,
1920
config::{Config, ConfigChange, ConfigErrors},
@@ -189,6 +190,7 @@ fn run_server() -> anyhow::Result<()> {
189190
let root_path = match root_uri
190191
.and_then(|it| it.to_file_path().ok())
191192
.map(patch_path_prefix)
193+
.and_then(|it| Utf8PathBuf::from_path_buf(it).ok())
192194
.and_then(|it| AbsPathBuf::try_from(it).ok())
193195
{
194196
Some(it) => it,
@@ -218,6 +220,7 @@ fn run_server() -> anyhow::Result<()> {
218220
.into_iter()
219221
.filter_map(|it| it.uri.to_file_path().ok())
220222
.map(patch_path_prefix)
223+
.filter_map(|it| Utf8PathBuf::from_path_buf(it).ok())
221224
.filter_map(|it| AbsPathBuf::try_from(it).ok())
222225
.collect::<Vec<_>>()
223226
})

‎crates/rust-analyzer/src/cli/rustc_tests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use std::{cell::RefCell, fs::read_to_string, panic::AssertUnwindSafe, path::Path
88
use hir::{ChangeWithProcMacros, Crate};
99
use ide::{AnalysisHost, DiagnosticCode, DiagnosticsConfig};
1010
use itertools::Either;
11+
use paths::Utf8PathBuf;
1112
use profile::StopWatch;
1213
use project_model::target_data_layout::RustcDataLayoutConfig;
1314
use project_model::{
@@ -64,7 +65,7 @@ impl Tester {
6465
fn new() -> Result<Self> {
6566
let mut path = std::env::temp_dir();
6667
path.push("ra-rustc-test.rs");
67-
let tmp_file = AbsPathBuf::try_from(path).unwrap();
68+
let tmp_file = AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).unwrap()).unwrap();
6869
std::fs::write(&tmp_file, "")?;
6970
let cargo_config =
7071
CargoConfig { sysroot: Some(RustLibSource::Discover), ..Default::default() };

‎crates/rust-analyzer/src/config.rs

Lines changed: 18 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3450,15 +3450,15 @@ mod tests {
34503450
let s = remove_ws(&schema);
34513451
if !p.contains(&s) {
34523452
package_json.replace_range(start..end, &schema);
3453-
ensure_file_contents(&package_json_path, &package_json)
3453+
ensure_file_contents(package_json_path.as_std_path(), &package_json)
34543454
}
34553455
}
34563456

34573457
#[test]
34583458
fn generate_config_documentation() {
34593459
let docs_path = project_root().join("docs/user/generated_config.adoc");
34603460
let expected = FullConfigInput::manual();
3461-
ensure_file_contents(&docs_path, &expected);
3461+
ensure_file_contents(docs_path.as_std_path(), &expected);
34623462
}
34633463

34643464
fn remove_ws(text: &str) -> String {
@@ -3467,13 +3467,8 @@ mod tests {
34673467

34683468
#[test]
34693469
fn proc_macro_srv_null() {
3470-
let mut config = Config::new(
3471-
AbsPathBuf::try_from(project_root()).unwrap(),
3472-
Default::default(),
3473-
vec![],
3474-
None,
3475-
None,
3476-
);
3470+
let mut config =
3471+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
34773472

34783473
let mut change = ConfigChange::default();
34793474
change.change_client_config(serde_json::json!({
@@ -3487,32 +3482,22 @@ mod tests {
34873482

34883483
#[test]
34893484
fn proc_macro_srv_abs() {
3490-
let mut config = Config::new(
3491-
AbsPathBuf::try_from(project_root()).unwrap(),
3492-
Default::default(),
3493-
vec![],
3494-
None,
3495-
None,
3496-
);
3485+
let mut config =
3486+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
34973487
let mut change = ConfigChange::default();
34983488
change.change_client_config(serde_json::json!({
34993489
"procMacro" : {
3500-
"server": project_root().display().to_string(),
3490+
"server": project_root().to_string(),
35013491
}}));
35023492

35033493
(config, _, _) = config.apply_change(change);
3504-
assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::try_from(project_root()).unwrap()));
3494+
assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::assert(project_root())));
35053495
}
35063496

35073497
#[test]
35083498
fn proc_macro_srv_rel() {
3509-
let mut config = Config::new(
3510-
AbsPathBuf::try_from(project_root()).unwrap(),
3511-
Default::default(),
3512-
vec![],
3513-
None,
3514-
None,
3515-
);
3499+
let mut config =
3500+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
35163501

35173502
let mut change = ConfigChange::default();
35183503

@@ -3531,13 +3516,8 @@ mod tests {
35313516

35323517
#[test]
35333518
fn cargo_target_dir_unset() {
3534-
let mut config = Config::new(
3535-
AbsPathBuf::try_from(project_root()).unwrap(),
3536-
Default::default(),
3537-
vec![],
3538-
None,
3539-
None,
3540-
);
3519+
let mut config =
3520+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
35413521

35423522
let mut change = ConfigChange::default();
35433523

@@ -3554,13 +3534,8 @@ mod tests {
35543534

35553535
#[test]
35563536
fn cargo_target_dir_subdir() {
3557-
let mut config = Config::new(
3558-
AbsPathBuf::try_from(project_root()).unwrap(),
3559-
Default::default(),
3560-
vec![],
3561-
None,
3562-
None,
3563-
);
3537+
let mut config =
3538+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
35643539

35653540
let mut change = ConfigChange::default();
35663541
change.change_client_config(serde_json::json!({
@@ -3577,13 +3552,8 @@ mod tests {
35773552

35783553
#[test]
35793554
fn cargo_target_dir_relative_dir() {
3580-
let mut config = Config::new(
3581-
AbsPathBuf::try_from(project_root()).unwrap(),
3582-
Default::default(),
3583-
vec![],
3584-
None,
3585-
None,
3586-
);
3555+
let mut config =
3556+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
35873557

35883558
let mut change = ConfigChange::default();
35893559
change.change_client_config(serde_json::json!({
@@ -3603,13 +3573,8 @@ mod tests {
36033573

36043574
#[test]
36053575
fn toml_unknown_key() {
3606-
let config = Config::new(
3607-
AbsPathBuf::try_from(project_root()).unwrap(),
3608-
Default::default(),
3609-
vec![],
3610-
None,
3611-
None,
3612-
);
3576+
let config =
3577+
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
36133578

36143579
let mut change = ConfigChange::default();
36153580

‎crates/rust-analyzer/src/handlers/notification.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use lsp_types::{
99
DidChangeWatchedFilesParams, DidChangeWorkspaceFoldersParams, DidCloseTextDocumentParams,
1010
DidOpenTextDocumentParams, DidSaveTextDocumentParams, WorkDoneProgressCancelParams,
1111
};
12+
use paths::Utf8PathBuf;
1213
use triomphe::Arc;
1314
use vfs::{AbsPathBuf, ChangeKind, VfsPath};
1415

@@ -240,6 +241,7 @@ pub(crate) fn handle_did_change_workspace_folders(
240241

241242
for workspace in params.event.removed {
242243
let Ok(path) = workspace.uri.to_file_path() else { continue };
244+
let Ok(path) = Utf8PathBuf::from_path_buf(path) else { continue };
243245
let Ok(path) = AbsPathBuf::try_from(path) else { continue };
244246
config.remove_workspace(&path);
245247
}
@@ -249,6 +251,7 @@ pub(crate) fn handle_did_change_workspace_folders(
249251
.added
250252
.into_iter()
251253
.filter_map(|it| it.uri.to_file_path().ok())
254+
.filter_map(|it| Utf8PathBuf::from_path_buf(it).ok())
252255
.filter_map(|it| AbsPathBuf::try_from(it).ok());
253256
config.add_workspaces(added);
254257

‎crates/rust-analyzer/src/handlers/request.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -781,9 +781,12 @@ pub(crate) fn handle_parent_module(
781781
if let Ok(file_path) = &params.text_document.uri.to_file_path() {
782782
if file_path.file_name().unwrap_or_default() == "Cargo.toml" {
783783
// search workspaces for parent packages or fallback to workspace root
784-
let abs_path_buf = match AbsPathBuf::try_from(file_path.to_path_buf()).ok() {
785-
Some(abs_path_buf) => abs_path_buf,
786-
None => return Ok(None),
784+
let abs_path_buf = match Utf8PathBuf::from_path_buf(file_path.to_path_buf())
785+
.ok()
786+
.map(AbsPathBuf::try_from)
787+
{
788+
Some(Ok(abs_path_buf)) => abs_path_buf,
789+
_ => return Ok(None),
787790
};
788791

789792
let manifest_path = match ManifestPath::try_from(abs_path_buf).ok() {

‎crates/rust-analyzer/src/integrated_benchmarks.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,19 @@ fn integrated_highlighting_benchmark() {
4646

4747
let (db, vfs, _proc_macro) = {
4848
let _it = stdx::timeit("workspace loading");
49-
load_workspace_at(&workspace_to_load, &cargo_config, &load_cargo_config, &|_| {}).unwrap()
49+
load_workspace_at(
50+
workspace_to_load.as_std_path(),
51+
&cargo_config,
52+
&load_cargo_config,
53+
&|_| {},
54+
)
55+
.unwrap()
5056
};
5157
let mut host = AnalysisHost::with_database(db);
5258

5359
let file_id = {
5460
let file = workspace_to_load.join(file);
55-
let path = VfsPath::from(AbsPathBuf::assert_utf8(file));
61+
let path = VfsPath::from(AbsPathBuf::assert(file));
5662
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
5763
};
5864

@@ -106,13 +112,19 @@ fn integrated_completion_benchmark() {
106112

107113
let (db, vfs, _proc_macro) = {
108114
let _it = stdx::timeit("workspace loading");
109-
load_workspace_at(&workspace_to_load, &cargo_config, &load_cargo_config, &|_| {}).unwrap()
115+
load_workspace_at(
116+
workspace_to_load.as_std_path(),
117+
&cargo_config,
118+
&load_cargo_config,
119+
&|_| {},
120+
)
121+
.unwrap()
110122
};
111123
let mut host = AnalysisHost::with_database(db);
112124

113125
let file_id = {
114126
let file = workspace_to_load.join(file);
115-
let path = VfsPath::from(AbsPathBuf::assert_utf8(file));
127+
let path = VfsPath::from(AbsPathBuf::assert(file));
116128
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
117129
};
118130

@@ -274,13 +286,19 @@ fn integrated_diagnostics_benchmark() {
274286

275287
let (db, vfs, _proc_macro) = {
276288
let _it = stdx::timeit("workspace loading");
277-
load_workspace_at(&workspace_to_load, &cargo_config, &load_cargo_config, &|_| {}).unwrap()
289+
load_workspace_at(
290+
workspace_to_load.as_std_path(),
291+
&cargo_config,
292+
&load_cargo_config,
293+
&|_| {},
294+
)
295+
.unwrap()
278296
};
279297
let mut host = AnalysisHost::with_database(db);
280298

281299
let file_id = {
282300
let file = workspace_to_load.join(file);
283-
let path = VfsPath::from(AbsPathBuf::assert_utf8(file));
301+
let path = VfsPath::from(AbsPathBuf::assert(file));
284302
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
285303
};
286304

0 commit comments

Comments
 (0)