internal: Remove AbsPathBuf::TryFrom impl that checks too many things at once

This commit is contained in:
Lukas Wirth 2024-08-02 11:08:19 +02:00
parent 670a5ab4a9
commit 758ad25229
15 changed files with 90 additions and 95 deletions

1
Cargo.lock generated
View file

@ -1987,6 +1987,7 @@ name = "test-utils"
version = "0.0.0"
dependencies = [
"dissimilar",
"paths",
"profile",
"rustc-hash",
"stdx",

View file

@ -1,4 +1,4 @@
//! Thin wrappers around `std::path`/`camino::path`, distinguishing between absolute and
//! Thin wrappers around [`camino::path`], distinguishing between absolute and
//! relative paths.
use std::{
@ -8,9 +8,9 @@ use std::{
path::{Path, PathBuf},
};
pub use camino::*;
pub use camino::{Utf8Component, Utf8Components, Utf8Path, Utf8PathBuf, Utf8Prefix};
/// Wrapper around an absolute [`Utf8PathBuf`].
/// A [`Utf8PathBuf`] that is guaranteed to be absolute.
#[derive(Debug, Clone, Ord, PartialOrd, Eq, Hash)]
pub struct AbsPathBuf(Utf8PathBuf);
@ -73,16 +73,6 @@ impl TryFrom<Utf8PathBuf> for AbsPathBuf {
}
}
impl TryFrom<PathBuf> for AbsPathBuf {
type Error = PathBuf;
fn try_from(path_buf: PathBuf) -> Result<AbsPathBuf, PathBuf> {
if !path_buf.is_absolute() {
return Err(path_buf);
}
Ok(AbsPathBuf(Utf8PathBuf::from_path_buf(path_buf)?))
}
}
impl TryFrom<&str> for AbsPathBuf {
type Error = Utf8PathBuf;
fn try_from(path: &str) -> Result<AbsPathBuf, Utf8PathBuf> {

View file

@ -6,17 +6,12 @@
//! This module implements this second part. We use "build script" terminology
//! here, but it covers procedural macros as well.
use std::{
cell::RefCell,
io, mem,
path::{self, PathBuf},
process::Command,
};
use std::{cell::RefCell, io, mem, path, process::Command};
use cargo_metadata::{camino::Utf8Path, Message};
use itertools::Itertools;
use la_arena::ArenaMap;
use paths::{AbsPath, AbsPathBuf};
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
use rustc_hash::{FxHashMap, FxHashSet};
use serde::Deserialize;
use toolchain::Tool;
@ -423,7 +418,7 @@ impl WorkspaceBuildScripts {
utf8_stdout(cmd)
})()?;
let target_libdir = AbsPathBuf::try_from(PathBuf::from(target_libdir))
let target_libdir = AbsPathBuf::try_from(Utf8PathBuf::from(target_libdir))
.map_err(|_| anyhow::format_err!("target-libdir was not an absolute path"))?;
tracing::info!("Loading rustc proc-macro paths from {target_libdir}");
@ -435,7 +430,8 @@ impl WorkspaceBuildScripts {
let extension = path.extension()?;
if extension == std::env::consts::DLL_EXTENSION {
let name = path.file_stem()?.to_str()?.split_once('-')?.0.to_owned();
let path = AbsPathBuf::try_from(path).ok()?;
let path = AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).ok()?)
.ok()?;
return Some((name, path));
}
}

View file

@ -37,7 +37,7 @@ use std::{
};
use anyhow::{bail, format_err, Context};
use paths::{AbsPath, AbsPathBuf};
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
use rustc_hash::FxHashSet;
pub use crate::{
@ -132,8 +132,11 @@ impl ProjectManifest {
.filter_map(Result::ok)
.map(|it| it.path().join("Cargo.toml"))
.filter(|it| it.exists())
.map(Utf8PathBuf::from_path_buf)
.filter_map(Result::ok)
.map(AbsPathBuf::try_from)
.filter_map(|it| it.ok()?.try_into().ok())
.filter_map(Result::ok)
.filter_map(|it| it.try_into().ok())
.collect()
}
}

View file

@ -14,6 +14,7 @@ use std::{env, fs, path::PathBuf, process::ExitCode, sync::Arc};
use anyhow::Context;
use lsp_server::Connection;
use paths::Utf8PathBuf;
use rust_analyzer::{
cli::flags,
config::{Config, ConfigChange, ConfigErrors},
@ -189,6 +190,7 @@ fn run_server() -> anyhow::Result<()> {
let root_path = match root_uri
.and_then(|it| it.to_file_path().ok())
.map(patch_path_prefix)
.and_then(|it| Utf8PathBuf::from_path_buf(it).ok())
.and_then(|it| AbsPathBuf::try_from(it).ok())
{
Some(it) => it,
@ -218,6 +220,7 @@ fn run_server() -> anyhow::Result<()> {
.into_iter()
.filter_map(|it| it.uri.to_file_path().ok())
.map(patch_path_prefix)
.filter_map(|it| Utf8PathBuf::from_path_buf(it).ok())
.filter_map(|it| AbsPathBuf::try_from(it).ok())
.collect::<Vec<_>>()
})

View file

@ -8,6 +8,7 @@ use std::{cell::RefCell, fs::read_to_string, panic::AssertUnwindSafe, path::Path
use hir::{ChangeWithProcMacros, Crate};
use ide::{AnalysisHost, DiagnosticCode, DiagnosticsConfig};
use itertools::Either;
use paths::Utf8PathBuf;
use profile::StopWatch;
use project_model::target_data_layout::RustcDataLayoutConfig;
use project_model::{
@ -64,7 +65,7 @@ impl Tester {
fn new() -> Result<Self> {
let mut path = std::env::temp_dir();
path.push("ra-rustc-test.rs");
let tmp_file = AbsPathBuf::try_from(path).unwrap();
let tmp_file = AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).unwrap()).unwrap();
std::fs::write(&tmp_file, "")?;
let cargo_config =
CargoConfig { sysroot: Some(RustLibSource::Discover), ..Default::default() };

View file

@ -3450,7 +3450,7 @@ mod tests {
let s = remove_ws(&schema);
if !p.contains(&s) {
package_json.replace_range(start..end, &schema);
ensure_file_contents(&package_json_path, &package_json)
ensure_file_contents(package_json_path.as_std_path(), &package_json)
}
}
@ -3458,7 +3458,7 @@ mod tests {
fn generate_config_documentation() {
let docs_path = project_root().join("docs/user/generated_config.adoc");
let expected = FullConfigInput::manual();
ensure_file_contents(&docs_path, &expected);
ensure_file_contents(docs_path.as_std_path(), &expected);
}
fn remove_ws(text: &str) -> String {
@ -3467,13 +3467,8 @@ mod tests {
#[test]
fn proc_macro_srv_null() {
let mut config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let mut config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();
change.change_client_config(serde_json::json!({
@ -3487,32 +3482,22 @@ mod tests {
#[test]
fn proc_macro_srv_abs() {
let mut config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let mut config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();
change.change_client_config(serde_json::json!({
"procMacro" : {
"server": project_root().display().to_string(),
"server": project_root().to_string(),
}}));
(config, _, _) = config.apply_change(change);
assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::try_from(project_root()).unwrap()));
assert_eq!(config.proc_macro_srv(), Some(AbsPathBuf::assert(project_root())));
}
#[test]
fn proc_macro_srv_rel() {
let mut config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let mut config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();
@ -3531,13 +3516,8 @@ mod tests {
#[test]
fn cargo_target_dir_unset() {
let mut config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let mut config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();
@ -3554,13 +3534,8 @@ mod tests {
#[test]
fn cargo_target_dir_subdir() {
let mut config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let mut config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();
change.change_client_config(serde_json::json!({
@ -3577,13 +3552,8 @@ mod tests {
#[test]
fn cargo_target_dir_relative_dir() {
let mut config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let mut config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();
change.change_client_config(serde_json::json!({
@ -3603,13 +3573,8 @@ mod tests {
#[test]
fn toml_unknown_key() {
let config = Config::new(
AbsPathBuf::try_from(project_root()).unwrap(),
Default::default(),
vec![],
None,
None,
);
let config =
Config::new(AbsPathBuf::assert(project_root()), Default::default(), vec![], None, None);
let mut change = ConfigChange::default();

View file

@ -9,6 +9,7 @@ use lsp_types::{
DidChangeWatchedFilesParams, DidChangeWorkspaceFoldersParams, DidCloseTextDocumentParams,
DidOpenTextDocumentParams, DidSaveTextDocumentParams, WorkDoneProgressCancelParams,
};
use paths::Utf8PathBuf;
use triomphe::Arc;
use vfs::{AbsPathBuf, ChangeKind, VfsPath};
@ -240,6 +241,7 @@ pub(crate) fn handle_did_change_workspace_folders(
for workspace in params.event.removed {
let Ok(path) = workspace.uri.to_file_path() else { continue };
let Ok(path) = Utf8PathBuf::from_path_buf(path) else { continue };
let Ok(path) = AbsPathBuf::try_from(path) else { continue };
config.remove_workspace(&path);
}
@ -249,6 +251,7 @@ pub(crate) fn handle_did_change_workspace_folders(
.added
.into_iter()
.filter_map(|it| it.uri.to_file_path().ok())
.filter_map(|it| Utf8PathBuf::from_path_buf(it).ok())
.filter_map(|it| AbsPathBuf::try_from(it).ok());
config.add_workspaces(added);

View file

@ -781,9 +781,12 @@ pub(crate) fn handle_parent_module(
if let Ok(file_path) = &params.text_document.uri.to_file_path() {
if file_path.file_name().unwrap_or_default() == "Cargo.toml" {
// search workspaces for parent packages or fallback to workspace root
let abs_path_buf = match AbsPathBuf::try_from(file_path.to_path_buf()).ok() {
Some(abs_path_buf) => abs_path_buf,
None => return Ok(None),
let abs_path_buf = match Utf8PathBuf::from_path_buf(file_path.to_path_buf())
.ok()
.map(AbsPathBuf::try_from)
{
Some(Ok(abs_path_buf)) => abs_path_buf,
_ => return Ok(None),
};
let manifest_path = match ManifestPath::try_from(abs_path_buf).ok() {

View file

@ -46,13 +46,19 @@ fn integrated_highlighting_benchmark() {
let (db, vfs, _proc_macro) = {
let _it = stdx::timeit("workspace loading");
load_workspace_at(&workspace_to_load, &cargo_config, &load_cargo_config, &|_| {}).unwrap()
load_workspace_at(
workspace_to_load.as_std_path(),
&cargo_config,
&load_cargo_config,
&|_| {},
)
.unwrap()
};
let mut host = AnalysisHost::with_database(db);
let file_id = {
let file = workspace_to_load.join(file);
let path = VfsPath::from(AbsPathBuf::assert_utf8(file));
let path = VfsPath::from(AbsPathBuf::assert(file));
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
};
@ -106,13 +112,19 @@ fn integrated_completion_benchmark() {
let (db, vfs, _proc_macro) = {
let _it = stdx::timeit("workspace loading");
load_workspace_at(&workspace_to_load, &cargo_config, &load_cargo_config, &|_| {}).unwrap()
load_workspace_at(
workspace_to_load.as_std_path(),
&cargo_config,
&load_cargo_config,
&|_| {},
)
.unwrap()
};
let mut host = AnalysisHost::with_database(db);
let file_id = {
let file = workspace_to_load.join(file);
let path = VfsPath::from(AbsPathBuf::assert_utf8(file));
let path = VfsPath::from(AbsPathBuf::assert(file));
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
};
@ -274,13 +286,19 @@ fn integrated_diagnostics_benchmark() {
let (db, vfs, _proc_macro) = {
let _it = stdx::timeit("workspace loading");
load_workspace_at(&workspace_to_load, &cargo_config, &load_cargo_config, &|_| {}).unwrap()
load_workspace_at(
workspace_to_load.as_std_path(),
&cargo_config,
&load_cargo_config,
&|_| {},
)
.unwrap()
};
let mut host = AnalysisHost::with_database(db);
let file_id = {
let file = workspace_to_load.join(file);
let path = VfsPath::from(AbsPathBuf::assert_utf8(file));
let path = VfsPath::from(AbsPathBuf::assert(file));
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
};

View file

@ -2,6 +2,7 @@
use anyhow::format_err;
use ide::{Annotation, AnnotationKind, AssistKind, LineCol};
use ide_db::{line_index::WideLineCol, FileId, FilePosition, FileRange};
use paths::Utf8PathBuf;
use syntax::{TextRange, TextSize};
use vfs::AbsPathBuf;
@ -13,7 +14,7 @@ use crate::{
pub(crate) fn abs_path(url: &lsp_types::Url) -> anyhow::Result<AbsPathBuf> {
let path = url.to_file_path().map_err(|()| anyhow::format_err!("url is not a file"))?;
Ok(AbsPathBuf::try_from(path).unwrap())
Ok(AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).unwrap()).unwrap())
}
pub(crate) fn vfs_path(url: &lsp_types::Url) -> anyhow::Result<vfs::VfsPath> {

View file

@ -79,7 +79,7 @@ fn self_hosting_parsing() {
let crates_dir = project_root().join("crates");
let mut files = Vec::new();
let mut work = vec![crates_dir.to_path_buf()];
let mut work = vec![crates_dir.into_std_path_buf()];
while let Some(dir) = work.pop() {
for entry in dir.read_dir().unwrap() {
let entry = entry.unwrap();
@ -127,7 +127,7 @@ fn self_hosting_parsing() {
}
fn test_data_dir() -> PathBuf {
project_root().join("crates/syntax/test_data")
project_root().into_std_path_buf().join("crates/syntax/test_data")
}
fn assert_errors_are_present(errors: &[SyntaxError], path: &Path) {

View file

@ -18,6 +18,7 @@ text-size.workspace = true
tracing.workspace = true
rustc-hash.workspace = true
paths.workspace = true
stdx.workspace = true
profile.workspace = true

View file

@ -18,6 +18,7 @@ use std::{
path::{Path, PathBuf},
};
use paths::Utf8PathBuf;
use profile::StopWatch;
use stdx::is_ci;
use text_size::{TextRange, TextSize};
@ -402,9 +403,10 @@ pub fn skip_slow_tests() -> bool {
}
/// Returns the path to the root directory of `rust-analyzer` project.
pub fn project_root() -> PathBuf {
pub fn project_root() -> Utf8PathBuf {
let dir = env!("CARGO_MANIFEST_DIR");
PathBuf::from(dir).parent().unwrap().parent().unwrap().to_owned()
Utf8PathBuf::from_path_buf(PathBuf::from(dir).parent().unwrap().parent().unwrap().to_owned())
.unwrap()
}
pub fn format_diff(chunks: Vec<dissimilar::Chunk<'_>>) -> String {

View file

@ -14,7 +14,7 @@ use std::{
use crossbeam_channel::{never, select, unbounded, Receiver, Sender};
use notify::{Config, RecommendedWatcher, RecursiveMode, Watcher};
use paths::{AbsPath, AbsPathBuf};
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
use vfs::loader;
use walkdir::WalkDir;
@ -145,7 +145,12 @@ impl NotifyActor {
let files = event
.paths
.into_iter()
.map(|path| AbsPathBuf::try_from(path).unwrap())
.filter_map(|path| {
Some(
AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).ok()?)
.expect("path is absolute"),
)
})
.filter_map(|path| {
let meta = fs::metadata(&path).ok()?;
if meta.file_type().is_dir()
@ -220,7 +225,10 @@ impl NotifyActor {
let depth = entry.depth();
let is_dir = entry.file_type().is_dir();
let is_file = entry.file_type().is_file();
let abs_path = AbsPathBuf::try_from(entry.into_path()).ok()?;
let abs_path = AbsPathBuf::try_from(
Utf8PathBuf::from_path_buf(entry.into_path()).ok()?,
)
.ok()?;
if depth < 2 && is_dir {
self.send(make_message(abs_path.clone()));
}