From d34588bf83898870d7f9b4b49ac2a5f71c77dabb Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 28 Aug 2018 18:22:52 +0300 Subject: create module smartly --- crates/libanalysis/Cargo.toml | 1 + crates/libanalysis/src/lib.rs | 83 +++++++++++++++++++++++++++--------- crates/libanalysis/src/module_map.rs | 80 ++++++++++++++++++++++++++-------- crates/libanalysis/tests/tests.rs | 72 +++++++++++++++++++------------ 4 files changed, 171 insertions(+), 65 deletions(-) (limited to 'crates/libanalysis') diff --git a/crates/libanalysis/Cargo.toml b/crates/libanalysis/Cargo.toml index a8ef5e5f4..5aca84f0e 100644 --- a/crates/libanalysis/Cargo.toml +++ b/crates/libanalysis/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" authors = ["Aleksey Kladov "] [dependencies] +relative-path = "0.3.7" log = "0.4.2" failure = "0.1.2" parking_lot = "0.6.3" diff --git a/crates/libanalysis/src/lib.rs b/crates/libanalysis/src/lib.rs index fe2c3c2e6..96d10a087 100644 --- a/crates/libanalysis/src/lib.rs +++ b/crates/libanalysis/src/lib.rs @@ -8,13 +8,13 @@ extern crate libsyntax2; extern crate libeditor; extern crate fst; extern crate rayon; +extern crate relative_path; mod symbol_index; mod module_map; use std::{ fmt, - path::{Path, PathBuf}, panic, sync::{ Arc, @@ -24,6 +24,7 @@ use std::{ time::Instant, }; +use relative_path::{RelativePath,RelativePathBuf}; use once_cell::sync::OnceCell; use rayon::prelude::*; @@ -37,13 +38,16 @@ use libeditor::{Diagnostic, LineIndex, FileSymbol, find_node_at_offset}; use self::{ symbol_index::FileSymbols, - module_map::{ModuleMap, ChangeKind}, + module_map::{ModuleMap, ChangeKind, Problem}, }; pub use self::symbol_index::Query; pub type Result = ::std::result::Result; -pub type FileResolver = dyn Fn(FileId, &Path) -> Option + Send + Sync; +pub trait FileResolver: Send + Sync + 'static { + fn file_stem(&self, id: FileId) -> String; + fn resolve(&self, id: FileId, path: &RelativePath) -> Option; +} #[derive(Debug)] pub struct WorldState { @@ -84,7 +88,7 @@ impl WorldState { pub fn snapshot( &self, - file_resolver: impl Fn(FileId, &Path) -> Option + 'static + Send + Sync, + file_resolver: impl FileResolver, ) -> World { World { needs_reindex: AtomicBool::new(false), @@ -132,8 +136,20 @@ impl WorldState { } #[derive(Debug)] -pub enum QuickFix { - CreateFile(PathBuf), +pub struct QuickFix { + pub fs_ops: Vec, +} + +#[derive(Debug)] +pub enum FsOp { + CreateFile { + anchor: FileId, + path: RelativePathBuf, + }, + MoveFile { + file: FileId, + path: RelativePathBuf, + } } impl World { @@ -221,20 +237,49 @@ impl World { .into_iter() .map(|d| (d, None)) .collect::>(); - for module in syntax.ast().modules() { - if module.has_semi() && self.resolve_module(file_id, module).is_empty() { - if let Some(name) = module.name() { - let d = Diagnostic { - range: name.syntax().range(), - msg: "unresolved module".to_string(), - }; - let quick_fix = self.data.module_map.suggested_child_mod_path(module) - .map(QuickFix::CreateFile); - - res.push((d, quick_fix)) - } + + self.data.module_map.problems( + file_id, + &*self.file_resolver, + &|file_id| self.file_syntax(file_id).unwrap(), + |name_node, problem| { + let (diag, fix) = match problem { + Problem::UnresolvedModule { candidate } => { + let diag = Diagnostic { + range: name_node.syntax().range(), + msg: "unresolved module".to_string(), + }; + let fix = QuickFix { + fs_ops: vec![FsOp::CreateFile { + anchor: file_id, + path: candidate.clone(), + }] + }; + (diag, fix) + } + Problem::NotDirOwner { move_to, candidate } => { + let diag = Diagnostic { + range: name_node.syntax().range(), + msg: "can't declare module at this location".to_string(), + }; + let fix = QuickFix { + fs_ops: vec![ + FsOp::MoveFile { + file: file_id, + path: move_to.clone(), + }, + FsOp::CreateFile { + anchor: file_id, + path: move_to.join(candidate), + } + ], + }; + (diag, fix) + } + }; + res.push((diag, Some(fix))) } - } + ); Ok(res) } diff --git a/crates/libanalysis/src/module_map.rs b/crates/libanalysis/src/module_map.rs index 4f480591e..b65569c46 100644 --- a/crates/libanalysis/src/module_map.rs +++ b/crates/libanalysis/src/module_map.rs @@ -1,6 +1,4 @@ -use std::{ - path::{PathBuf}, -}; +use relative_path::RelativePathBuf; use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use libsyntax2::{ @@ -43,6 +41,18 @@ struct Link { owner: ModuleId, syntax: SyntaxNode, points_to: Vec, + problem: Option, +} + +#[derive(Clone, Debug)] +pub enum Problem { + UnresolvedModule { + candidate: RelativePathBuf, + }, + NotDirOwner { + move_to: RelativePathBuf, + candidate: RelativePathBuf, + } } impl ModuleMap { @@ -93,9 +103,24 @@ impl ModuleMap { res } - pub fn suggested_child_mod_path(&self, m: ast::Module) -> Option { - let name = m.name()?; - Some(PathBuf::from(format!("../{}.rs", name.text()))) + pub fn problems( + &self, + file: FileId, + file_resolver: &FileResolver, + syntax_provider: &SyntaxProvider, + mut cb: impl FnMut(ast::Name, &Problem), + ) { + let module = self.file2module(file); + let links = self.links(file_resolver, syntax_provider); + links + .links + .iter() + .filter(|link| link.owner == module) + .filter_map(|link| { + let problem = link.problem.as_ref()?; + Some((link, problem)) + }) + .for_each(|(link, problem)| cb(link.name_node(), problem)) } fn links( @@ -176,14 +201,17 @@ impl Link { owner, syntax: module.syntax().owned(), points_to: Vec::new(), + problem: None, }; Some(link) } fn name(&self) -> SmolStr { - self.ast().name() - .unwrap() - .text() + self.name_node().text() + } + + fn name_node(&self) -> ast::Name { + self.ast().name().unwrap() } fn ast(&self) -> ast::Module { @@ -192,14 +220,30 @@ impl Link { } fn resolve(&mut self, file_resolver: &FileResolver) { - let name = self.name(); - let paths = &[ - PathBuf::from(format!("../{}.rs", name)), - PathBuf::from(format!("../{}/mod.rs", name)), - ]; - self.points_to = paths.iter() - .filter_map(|path| file_resolver(self.owner.0, path)) - .map(ModuleId) - .collect(); + let mod_name = file_resolver.file_stem(self.owner.0); + let is_dir_owner = + mod_name == "mod" || mod_name == "lib" || mod_name == "main"; + + let file_mod = RelativePathBuf::from(format!("../{}.rs", self.name())); + let dir_mod = RelativePathBuf::from(format!("../{}/mod.rs", self.name())); + if is_dir_owner { + self.points_to = [&file_mod, &dir_mod].iter() + .filter_map(|path| file_resolver.resolve(self.owner.0, path)) + .map(ModuleId) + .collect(); + self.problem = if self.points_to.is_empty() { + Some(Problem::UnresolvedModule { + candidate: file_mod, + }) + } else { + None + } + } else { + self.points_to = Vec::new(); + self.problem = Some(Problem::NotDirOwner { + move_to: RelativePathBuf::from(format!("../{}/mod.rs", mod_name)), + candidate: file_mod, + }); + } } } diff --git a/crates/libanalysis/tests/tests.rs b/crates/libanalysis/tests/tests.rs index 7c2950ccc..e378ab986 100644 --- a/crates/libanalysis/tests/tests.rs +++ b/crates/libanalysis/tests/tests.rs @@ -1,11 +1,39 @@ extern crate libanalysis; +extern crate relative_path; extern crate test_utils; -use std::path::PathBuf; +use std::path::{Path}; -use libanalysis::{WorldState, FileId}; +use relative_path::RelativePath; +use libanalysis::{WorldState, FileId, FileResolver}; use test_utils::assert_eq_dbg; +struct FileMap(&'static [(u32, &'static str)]); + +impl FileMap { + fn path(&self, id: FileId) -> &'static Path { + let s = self.0.iter() + .find(|it| it.0 == id.0) + .unwrap() + .1; + Path::new(s) + } +} + +impl FileResolver for FileMap { + fn file_stem(&self, id: FileId) -> String { + self.path(id).file_stem().unwrap().to_str().unwrap().to_string() + } + fn resolve(&self, id: FileId, rel: &RelativePath) -> Option { + let path = rel.to_path(self.path(id)); + let path = path.to_str().unwrap(); + let path = RelativePath::new(&path[1..]).normalize(); + let &(id, _) = self.0.iter() + .find(|it| path == RelativePath::new(&it.1[1..]).normalize())?; + Some(FileId(id)) + } +} + #[test] fn test_resolve_module() { @@ -13,14 +41,10 @@ fn test_resolve_module() { world.change_file(FileId(1), Some("mod foo;".to_string())); world.change_file(FileId(2), Some("".to_string())); - let snap = world.snapshot(|id, path| { - assert_eq!(id, FileId(1)); - if path == PathBuf::from("../foo/mod.rs") { - return None; - } - assert_eq!(path, PathBuf::from("../foo.rs")); - Some(FileId(2)) - }); + let snap = world.snapshot(FileMap(&[ + (1, "/lib.rs"), + (2, "/foo.rs"), + ])); let symbols = snap.approximately_resolve_symbol(FileId(1), 4.into()) .unwrap(); assert_eq_dbg( @@ -28,14 +52,10 @@ fn test_resolve_module() { &symbols, ); - let snap = world.snapshot(|id, path| { - assert_eq!(id, FileId(1)); - if path == PathBuf::from("../foo.rs") { - return None; - } - assert_eq!(path, PathBuf::from("../foo/mod.rs")); - Some(FileId(2)) - }); + let snap = world.snapshot(FileMap(&[ + (1, "/lib.rs"), + (2, "/foo/mod.rs") + ])); let symbols = snap.approximately_resolve_symbol(FileId(1), 4.into()) .unwrap(); assert_eq_dbg( @@ -49,11 +69,11 @@ fn test_unresolved_module_diagnostic() { let mut world = WorldState::new(); world.change_file(FileId(1), Some("mod foo;".to_string())); - let snap = world.snapshot(|_id, _path| None); + let snap = world.snapshot(FileMap(&[(1, "/lib.rs")])); let diagnostics = snap.diagnostics(FileId(1)).unwrap(); assert_eq_dbg( r#"[(Diagnostic { range: [4; 7), msg: "unresolved module" }, - Some(CreateFile("../foo.rs")))]"#, + Some(QuickFix { fs_ops: [CreateFile { anchor: FileId(1), path: "../foo.rs" }] }))]"#, &diagnostics, ); } @@ -64,14 +84,10 @@ fn test_resolve_parent_module() { world.change_file(FileId(1), Some("mod foo;".to_string())); world.change_file(FileId(2), Some("".to_string())); - let snap = world.snapshot(|id, path| { - assert_eq!(id, FileId(1)); - if path == PathBuf::from("../foo/mod.rs") { - return None; - } - assert_eq!(path, PathBuf::from("../foo.rs")); - Some(FileId(2)) - }); + let snap = world.snapshot(FileMap(&[ + (1, "/lib.rs"), + (2, "/foo.rs"), + ])); let symbols = snap.parent_module(FileId(2)); assert_eq_dbg( r#"[(FileId(1), FileSymbol { name: "foo", node_range: [0; 8), kind: MODULE })]"#, -- cgit v1.2.3