From 35e4bb35062a5e9e72282f33a0feaa9aea2151c5 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 8 Jul 2020 18:17:45 +0200 Subject: [PATCH 1/3] Document failed refactor --- crates/ra_db/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ra_db/src/lib.rs b/crates/ra_db/src/lib.rs index 590efffa44..1838226018 100644 --- a/crates/ra_db/src/lib.rs +++ b/crates/ra_db/src/lib.rs @@ -93,9 +93,9 @@ pub trait FileLoader { fn file_text(&self, file_id: FileId) -> Arc; /// Note that we intentionally accept a `&str` and not a `&Path` here. This /// method exists to handle `#[path = "/some/path.rs"] mod foo;` and such, - /// so the input is guaranteed to be utf-8 string. We might introduce - /// `struct StrPath(str)` for clarity some day, but it's a bit messy, so we - /// get by with a `&str` for the time being. + /// so the input is guaranteed to be utf-8 string. One might be tempted to + /// introduce some kind of "utf-8 path with / separators", but that's a bad idea. Behold + /// `#[path = "C://no/way"]` fn resolve_path(&self, anchor: FileId, path: &str) -> Option; fn relevant_crates(&self, file_id: FileId) -> Arc>; } From 7ae696ba7642aba92c2eed012d9e02c09bab7460 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 8 Jul 2020 18:22:57 +0200 Subject: [PATCH 2/3] Remove unwanted dependency --- crates/ra_db/src/lib.rs | 2 +- crates/rust-analyzer/src/bin/args.rs | 2 +- crates/rust-analyzer/src/bin/main.rs | 5 ++--- crates/rust-analyzer/src/cargo_target_spec.rs | 2 +- crates/rust-analyzer/src/cli/analysis_bench.rs | 3 ++- crates/rust-analyzer/src/cli/load_cargo.rs | 4 ++-- crates/rust-analyzer/src/config.rs | 2 +- crates/rust-analyzer/tests/heavy_tests/support.rs | 11 +++++------ 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/crates/ra_db/src/lib.rs b/crates/ra_db/src/lib.rs index 1838226018..3a8fd44c4c 100644 --- a/crates/ra_db/src/lib.rs +++ b/crates/ra_db/src/lib.rs @@ -18,7 +18,7 @@ pub use crate::{ }; pub use relative_path::{RelativePath, RelativePathBuf}; pub use salsa; -pub use vfs::{file_set::FileSet, AbsPathBuf, VfsPath}; +pub use vfs::{file_set::FileSet, VfsPath}; #[macro_export] macro_rules! impl_intern_key { diff --git a/crates/rust-analyzer/src/bin/args.rs b/crates/rust-analyzer/src/bin/args.rs index 85c2e415a1..d0e566f47e 100644 --- a/crates/rust-analyzer/src/bin/args.rs +++ b/crates/rust-analyzer/src/bin/args.rs @@ -7,9 +7,9 @@ use std::{env, fmt::Write, path::PathBuf}; use anyhow::{bail, Result}; use pico_args::Arguments; -use ra_db::AbsPathBuf; use ra_ssr::{SsrPattern, SsrRule}; use rust_analyzer::cli::{BenchWhat, Position, Verbosity}; +use vfs::AbsPathBuf; pub(crate) struct Args { pub(crate) verbosity: Verbosity, diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index eec76d4156..047772d0c3 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -6,14 +6,13 @@ mod args; use std::convert::TryFrom; use lsp_server::Connection; +use ra_project_model::ProjectManifest; use rust_analyzer::{ cli, config::{Config, LinkedProject}, from_json, Result, }; - -use ra_db::AbsPathBuf; -use ra_project_model::ProjectManifest; +use vfs::AbsPathBuf; use crate::args::HelpPrinted; diff --git a/crates/rust-analyzer/src/cargo_target_spec.rs b/crates/rust-analyzer/src/cargo_target_spec.rs index 929926cbc5..3183996247 100644 --- a/crates/rust-analyzer/src/cargo_target_spec.rs +++ b/crates/rust-analyzer/src/cargo_target_spec.rs @@ -1,9 +1,9 @@ //! See `CargoTargetSpec` use ra_cfg::CfgExpr; -use ra_db::AbsPathBuf; use ra_ide::{FileId, RunnableKind, TestId}; use ra_project_model::{self, TargetKind}; +use vfs::AbsPathBuf; use crate::{global_state::GlobalStateSnapshot, Result}; diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs index 90990d3e77..a93d5fb739 100644 --- a/crates/rust-analyzer/src/cli/analysis_bench.rs +++ b/crates/rust-analyzer/src/cli/analysis_bench.rs @@ -5,9 +5,10 @@ use std::{env, path::Path, str::FromStr, sync::Arc, time::Instant}; use anyhow::{format_err, Result}; use ra_db::{ salsa::{Database, Durability}, - AbsPathBuf, FileId, + FileId, }; use ra_ide::{Analysis, AnalysisChange, AnalysisHost, CompletionConfig, FilePosition, LineCol}; +use vfs::AbsPathBuf; use crate::cli::{load_cargo::load_cargo, Verbosity}; diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index d8677c2311..a43bf2244d 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -4,10 +4,10 @@ use std::{path::Path, sync::Arc}; use anyhow::Result; use crossbeam_channel::{unbounded, Receiver}; -use ra_db::{AbsPathBuf, CrateGraph}; +use ra_db::CrateGraph; use ra_ide::{AnalysisChange, AnalysisHost}; use ra_project_model::{CargoConfig, ProcMacroClient, ProjectManifest, ProjectWorkspace}; -use vfs::{loader::Handle, AbsPath}; +use vfs::{loader::Handle, AbsPath, AbsPathBuf}; use crate::reload::{ProjectFolders, SourceRootConfig}; diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index e42e41a410..9e7de0243e 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -11,10 +11,10 @@ use std::{ffi::OsString, path::PathBuf}; use flycheck::FlycheckConfig; use lsp_types::ClientCapabilities; -use ra_db::AbsPathBuf; use ra_ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; use ra_project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest}; use serde::Deserialize; +use vfs::AbsPathBuf; use crate::diagnostics::DiagnosticsConfig; diff --git a/crates/rust-analyzer/tests/heavy_tests/support.rs b/crates/rust-analyzer/tests/heavy_tests/support.rs index 7bf687794c..e51796d36e 100644 --- a/crates/rust-analyzer/tests/heavy_tests/support.rs +++ b/crates/rust-analyzer/tests/heavy_tests/support.rs @@ -12,17 +12,16 @@ use lsp_types::{ notification::Exit, request::Shutdown, TextDocumentIdentifier, Url, WorkDoneProgress, }; use lsp_types::{ProgressParams, ProgressParamsValue}; -use serde::Serialize; -use serde_json::{to_string_pretty, Value}; -use tempfile::TempDir; -use test_utils::{find_mismatch, Fixture}; - -use ra_db::AbsPathBuf; use ra_project_model::ProjectManifest; use rust_analyzer::{ config::{ClientCapsConfig, Config, FilesConfig, FilesWatcher, LinkedProject}, main_loop, }; +use serde::Serialize; +use serde_json::{to_string_pretty, Value}; +use tempfile::TempDir; +use test_utils::{find_mismatch, Fixture}; +use vfs::AbsPathBuf; pub struct Project<'a> { fixture: &'a str, From dab7f3d2c6cd035f446fbdcda2442954da4afd3a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 8 Jul 2020 19:09:42 +0200 Subject: [PATCH 3/3] Remove relative_path dependency --- Cargo.lock | 7 - crates/ra_db/Cargo.toml | 1 - crates/ra_db/src/lib.rs | 1 - .../ra_hir_def/src/nameres/mod_resolution.rs | 120 +++++++++++++----- crates/ra_ide_db/src/change.rs | 7 +- 5 files changed, 92 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 385e7a7bc8..242fca7004 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1020,7 +1020,6 @@ dependencies = [ "ra_prof", "ra_syntax", "ra_tt", - "relative-path", "rustc-hash", "salsa", "stdx", @@ -1408,12 +1407,6 @@ version = "0.6.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26412eb97c6b088a6997e05f69403a802a92d520de2f8e63c2b65f9e0f47c4e8" -[[package]] -name = "relative-path" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c602122c47b382cd045b10866a084b184035d45d8c2609cdd3762852ddfae2a1" - [[package]] name = "remove_dir_all" version = "0.5.3" diff --git a/crates/ra_db/Cargo.toml b/crates/ra_db/Cargo.toml index ef85266dbe..889142442a 100644 --- a/crates/ra_db/Cargo.toml +++ b/crates/ra_db/Cargo.toml @@ -9,7 +9,6 @@ doctest = false [dependencies] salsa = "0.15.0" -relative-path = "1.0.0" rustc-hash = "1.1.0" ra_syntax = { path = "../ra_syntax" } diff --git a/crates/ra_db/src/lib.rs b/crates/ra_db/src/lib.rs index 3a8fd44c4c..f25be24fe2 100644 --- a/crates/ra_db/src/lib.rs +++ b/crates/ra_db/src/lib.rs @@ -16,7 +16,6 @@ pub use crate::{ SourceRoot, SourceRootId, }, }; -pub use relative_path::{RelativePath, RelativePathBuf}; pub use salsa; pub use vfs::{file_set::FileSet, VfsPath}; diff --git a/crates/ra_hir_def/src/nameres/mod_resolution.rs b/crates/ra_hir_def/src/nameres/mod_resolution.rs index 39e9a6d977..9539616325 100644 --- a/crates/ra_hir_def/src/nameres/mod_resolution.rs +++ b/crates/ra_hir_def/src/nameres/mod_resolution.rs @@ -1,23 +1,24 @@ //! This module resolves `mod foo;` declaration to file. use hir_expand::name::Name; -use ra_db::{FileId, RelativePathBuf}; +use ra_db::FileId; use ra_syntax::SmolStr; use crate::{db::DefDatabase, HirFileId}; #[derive(Clone, Debug)] pub(super) struct ModDir { - /// `.` for `mod.rs`, `lib.rs` - /// `./foo` for `foo.rs` - /// `./foo/bar` for `mod bar { mod x; }` nested in `foo.rs` - path: RelativePathBuf, + /// `` for `mod.rs`, `lib.rs` + /// `foo/` for `foo.rs` + /// `foo/bar/` for `mod bar { mod x; }` nested in `foo.rs` + /// Invariant: path.is_empty() || path.ends_with('/') + dir_path: DirPath, /// inside `./foo.rs`, mods with `#[path]` should *not* be relative to `./foo/` root_non_dir_owner: bool, } impl ModDir { pub(super) fn root() -> ModDir { - ModDir { path: RelativePathBuf::default(), root_non_dir_owner: false } + ModDir { dir_path: DirPath::empty(), root_non_dir_owner: false } } pub(super) fn descend_into_definition( @@ -25,17 +26,21 @@ impl ModDir { name: &Name, attr_path: Option<&SmolStr>, ) -> ModDir { - let mut path = self.path.clone(); - match attr_to_path(attr_path) { - None => path.push(&name.to_string()), - Some(attr_path) => { - if self.root_non_dir_owner { - assert!(path.pop()); - } - path.push(attr_path); + let path = match attr_path.map(|it| it.as_str()) { + None => { + let mut path = self.dir_path.clone(); + path.push(&name.to_string()); + path } - } - ModDir { path, root_non_dir_owner: false } + Some(attr_path) => { + let mut path = self.dir_path.join_attr(attr_path, self.root_non_dir_owner); + if !(path.is_empty() || path.ends_with('/')) { + path.push('/') + } + DirPath::new(path) + } + }; + ModDir { dir_path: path, root_non_dir_owner: false } } pub(super) fn resolve_declaration( @@ -48,34 +53,87 @@ impl ModDir { let file_id = file_id.original_file(db.upcast()); let mut candidate_files = Vec::new(); - match attr_to_path(attr_path) { + match attr_path { Some(attr_path) => { - let base = - if self.root_non_dir_owner { self.path.parent().unwrap() } else { &self.path }; - candidate_files.push(base.join(attr_path).to_string()) + candidate_files.push(self.dir_path.join_attr(attr_path, self.root_non_dir_owner)) } None => { - candidate_files.push(self.path.join(&format!("{}.rs", name)).to_string()); - candidate_files.push(self.path.join(&format!("{}/mod.rs", name)).to_string()); + candidate_files.push(format!("{}{}.rs", self.dir_path.0, name)); + candidate_files.push(format!("{}{}/mod.rs", self.dir_path.0, name)); } }; for candidate in candidate_files.iter() { if let Some(file_id) = db.resolve_path(file_id, candidate.as_str()) { - let mut root_non_dir_owner = false; - let mut mod_path = RelativePathBuf::new(); let is_mod_rs = candidate.ends_with("mod.rs"); - if !(is_mod_rs || attr_path.is_some()) { - root_non_dir_owner = true; - mod_path.push(&name.to_string()); - } - return Ok((file_id, is_mod_rs, ModDir { path: mod_path, root_non_dir_owner })); + + let (dir_path, root_non_dir_owner) = if is_mod_rs || attr_path.is_some() { + (DirPath::empty(), false) + } else { + (DirPath::new(format!("{}/", name)), true) + }; + return Ok((file_id, is_mod_rs, ModDir { dir_path, root_non_dir_owner })); } } Err(candidate_files.remove(0)) } } -fn attr_to_path(attr: Option<&SmolStr>) -> Option { - attr.and_then(|it| RelativePathBuf::from_path(&it.replace("\\", "/")).ok()) +#[derive(Clone, Debug)] +struct DirPath(String); + +impl DirPath { + fn assert_invariant(&self) { + assert!(self.0.is_empty() || self.0.ends_with('/')); + } + fn new(repr: String) -> DirPath { + let res = DirPath(repr); + res.assert_invariant(); + res + } + fn empty() -> DirPath { + DirPath::new(String::new()) + } + fn push(&mut self, name: &str) { + self.0.push_str(name); + self.0.push('/'); + self.assert_invariant(); + } + fn parent(&self) -> Option<&str> { + if self.0.is_empty() { + return None; + }; + let idx = + self.0[..self.0.len() - '/'.len_utf8()].rfind('/').map_or(0, |it| it + '/'.len_utf8()); + Some(&self.0[..idx]) + } + /// So this is the case which doesn't really work I think if we try to be + /// 100% platform agnostic: + /// + /// ``` + /// mod a { + /// #[path="C://sad/face"] + /// mod b { mod c; } + /// } + /// ``` + /// + /// Here, we need to join logical dir path to a string path from an + /// attribute. Ideally, we should somehow losslessly communicate the whole + /// construction to `FileLoader`. + fn join_attr(&self, mut attr: &str, relative_to_parent: bool) -> String { + let base = if relative_to_parent { self.parent().unwrap() } else { &self.0 }; + + if attr.starts_with("./") { + attr = &attr["./".len()..]; + } + let tmp; + let attr = if attr.contains('\\') { + tmp = attr.replace('\\', "/"); + &tmp + } else { + attr + }; + let res = format!("{}{}", base, attr); + res + } } diff --git a/crates/ra_ide_db/src/change.rs b/crates/ra_ide_db/src/change.rs index 84c6f40ff2..d1a255dcfc 100644 --- a/crates/ra_ide_db/src/change.rs +++ b/crates/ra_ide_db/src/change.rs @@ -5,8 +5,7 @@ use std::{fmt, sync::Arc, time}; use ra_db::{ salsa::{Database, Durability, SweepStrategy}, - CrateGraph, FileId, RelativePathBuf, SourceDatabase, SourceDatabaseExt, SourceRoot, - SourceRootId, + CrateGraph, FileId, SourceDatabase, SourceDatabaseExt, SourceRoot, SourceRootId, }; use ra_prof::{memory_usage, profile, Bytes}; use rustc_hash::FxHashSet; @@ -57,14 +56,14 @@ impl AnalysisChange { #[derive(Debug)] struct AddFile { file_id: FileId, - path: RelativePathBuf, + path: String, text: Arc, } #[derive(Debug)] struct RemoveFile { file_id: FileId, - path: RelativePathBuf, + path: String, } #[derive(Default)]