From c84d1619d0fdd9174acbd216cd44b5efdd6ec970 Mon Sep 17 00:00:00 2001 From: oxalica Date: Tue, 11 Oct 2022 02:44:14 +0800 Subject: [PATCH] Refactor API of VfsPath --- crates/ide/src/base.rs | 90 ++++++++++++++++----------- crates/ide/src/def/path.rs | 28 ++++----- crates/ide/src/ide/goto_definition.rs | 2 +- crates/ide/src/tests.rs | 6 +- 4 files changed, 70 insertions(+), 56 deletions(-) diff --git a/crates/ide/src/base.rs b/crates/ide/src/base.rs index ddae9a1..b30350a 100644 --- a/crates/ide/src/base.rs +++ b/crates/ide/src/base.rs @@ -2,7 +2,7 @@ use rowan::{TextRange, TextSize}; use salsa::Durability; use std::collections::HashMap; use std::fmt; -use std::path::Path; +use std::path::{Component, Path}; use std::sync::Arc; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -11,71 +11,85 @@ pub struct FileId(pub u32); #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct SourceRootId(pub u32); -/// An absolute path in format `(/.+)*` -/// Currently, it represent an absolute filesytem path. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +/// An absolute Unix-like path in the virtual filesystem. +/// +/// It must be in form `(/[^/]+)+` and every segment must be non-empty and not `.` or `..`. +/// The root represented by an empty path. +#[derive(Default, Debug, Clone, PartialEq, Eq, Hash)] pub struct VfsPath(String); impl VfsPath { pub fn root() -> Self { - Self(String::new()) + Self::default() } - pub fn from_path(p: &Path) -> Option { - Self::new(p.to_str()?) + /// Construct a new absolute path in Unix-like format. + pub fn new(s: impl Into) -> Result { + let s = s.into(); + if s == "/" { + return Ok(Self::root()); + } + if !s.starts_with('/') || s[1..].split('/').any(|seg| matches!(seg, "" | "." | "..")) { + return Err(ParseVfsPathError); + } + Ok(Self(s)) } - pub fn new(s: impl Into) -> Option { - let mut s: String = s.into(); - if s.is_empty() || s == "/" { - return Some(Self::root()); + pub fn from_path(path: &Path) -> Result { + let mut ret = Self::root(); + for comp in path.components() { + match comp { + Component::RootDir => {} + Component::Normal(seg) => { + ret.push_segment(seg.to_str().ok_or(ParseVfsPathError)?); + } + _ => return Err(ParseVfsPathError), + } } - if s.ends_with('/') || s.as_bytes().windows(2).any(|w| w == b"//") { - return None; - } - if !s.starts_with('/') { - s.insert(0, '/'); - } - Some(Self(s)) + Ok(ret) } - pub fn push(&mut self, relative: &Self) { + /// Assume another VfsPath as relative and append it to this one. + pub fn append(&mut self, relative: &Self) { self.0.push_str(&relative.0); } - pub fn push_segment(&mut self, segment: &str) -> Option<()> { - if !segment.contains('/') { - self.0 += "/"; - self.0 += segment; - Some(()) - } else { - None - } + /// Push a path segment at the end. + /// Panic if it is empty or contains `/`. + pub fn push_segment(&mut self, segment: &str) { + assert!(!segment.is_empty() && !segment.contains('/')); + self.0 += "/"; + self.0 += segment; } + /// Pop the last segment from the end. + /// Returns `None` if it is already the root. pub fn pop(&mut self) -> Option<()> { self.0.truncate(self.0.rsplit_once('/')?.0.len()); Some(()) } + /// Get the path in Unix-like form. pub fn as_str(&self) -> &str { - &self.0 + if !self.0.is_empty() { + &self.0 + } else { + "/" + } } } -impl TryFrom for VfsPath { - type Error = (); - fn try_from(s: String) -> Result { - Self::new(s).ok_or(()) +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub struct ParseVfsPathError; + +impl fmt::Display for ParseVfsPathError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("Invalid VfsPath") } } -impl<'a> TryFrom<&'a str> for VfsPath { - type Error = (); - fn try_from(s: &'a str) -> Result { - Self::new(s).ok_or(()) - } -} +impl std::error::Error for ParseVfsPathError {} /// A set of [`VfsPath`]s identified by [`FileId`]s. #[derive(Default, Clone, PartialEq, Eq)] diff --git a/crates/ide/src/def/path.rs b/crates/ide/src/def/path.rs index 3fa7a83..1d6dd59 100644 --- a/crates/ide/src/def/path.rs +++ b/crates/ide/src/def/path.rs @@ -28,7 +28,7 @@ impl Path { for _ in 0..(data.supers.saturating_add(1)) { vpath.pop()?; } - vpath.push(&data.relative); + vpath.append(&data.relative); Some(vpath) } @@ -58,7 +58,7 @@ impl PathData { .filter(|&seg| !seg.is_empty() && seg != ".") { if seg != ".." { - relative.push_segment(seg).expect("Checked by lexer"); + relative.push_segment(seg); // Extra ".." has no effect for absolute path. } else if relative.pop().is_none() && anchor != PathAnchor::Absolute { supers = supers.saturating_add(1); @@ -91,12 +91,12 @@ mod tests { for anchor in [PathAnchor::Relative(FileId(0)), PathAnchor::Home, PathAnchor::Search("foo".into())] { let norm = |s| PathData::normalize(anchor.clone(), s); let path = |supers, p: &str| PathData { anchor: anchor.clone(), supers, relative: VfsPath::new(p).unwrap() }; - assert_eq!(norm(""), path(0, "")); - assert_eq!(norm("./."), path(0, "")); - assert_eq!(norm("./.."), path(1, "")); - assert_eq!(norm("../."), path(1, "")); - assert_eq!(norm("foo/./bar/../.baz"), path(0, "foo/.baz")); - assert_eq!(norm("../../foo"), path(2, "foo")); + assert_eq!(norm(""), path(0, "/")); + assert_eq!(norm("./."), path(0, "/")); + assert_eq!(norm("./.."), path(1, "/")); + assert_eq!(norm("../."), path(1, "/")); + assert_eq!(norm("foo/./bar/../.baz"), path(0, "/foo/.baz")); + assert_eq!(norm("../../foo"), path(2, "/foo")); } } @@ -106,11 +106,11 @@ mod tests { let anchor = PathAnchor::Absolute; let norm = |s| PathData::normalize(anchor.clone(), s); let path = |p: &str| PathData { anchor: anchor.clone(), supers: 0, relative: VfsPath::new(p).unwrap() }; - assert_eq!(norm("/"), path("")); - assert_eq!(norm("./."), path("")); - assert_eq!(norm("./.."), path("")); - assert_eq!(norm("../."), path("")); - assert_eq!(norm("foo/./bar/../.baz"), path("foo/.baz")); - assert_eq!(norm("../../foo"), path("foo")); + assert_eq!(norm("/"), path("/")); + assert_eq!(norm("./."), path("/")); + assert_eq!(norm("./.."), path("/")); + assert_eq!(norm("../."), path("/")); + assert_eq!(norm("foo/./bar/../.baz"), path("/foo/.baz")); + assert_eq!(norm("../../foo"), path("/foo")); } } diff --git a/crates/ide/src/ide/goto_definition.rs b/crates/ide/src/ide/goto_definition.rs index 0eabb43..dce55a2 100644 --- a/crates/ide/src/ide/goto_definition.rs +++ b/crates/ide/src/ide/goto_definition.rs @@ -221,7 +221,7 @@ mod tests { #[test] fn path() { - check("1 + $0./.", expect!["file://"]); + check("1 + $0./.", expect!["file:///"]); check( " #- /default.nix diff --git a/crates/ide/src/tests.rs b/crates/ide/src/tests.rs index ae76265..62e8631 100644 --- a/crates/ide/src/tests.rs +++ b/crates/ide/src/tests.rs @@ -37,7 +37,7 @@ impl TestDB { change.change_file(file, text.to_owned().into()); } let entry = file_set - .file_for_path(&"/default.nix".try_into().unwrap()) + .file_for_path(&VfsPath::new("/default.nix").unwrap()) .context("Missing entry file")?; change.set_roots(vec![SourceRoot::new_local(file_set, Some(entry))]); change.apply(&mut db); @@ -101,7 +101,7 @@ impl Fixture { if let Some(path) = line.strip_prefix("#- ") { ensure!(!missing_header, "Missing path header at the first line"); - let path = VfsPath::new(path).context("Invalid path")?; + let path = VfsPath::new(path)?; if let Some(prev_path) = cur_path.replace(path) { this.insert_file(prev_path, mem::take(&mut cur_text))?; cur_file.0 += 1; @@ -109,7 +109,7 @@ impl Fixture { } else { if cur_path.is_none() { missing_header = true; - cur_path = Some("/default.nix".try_into().unwrap()); + cur_path = Some(VfsPath::new("/default.nix").unwrap()); } let mut iter = line.chars().peekable();