From dc6e1e0dac318b36ec43ffced3d4059a9b8652e5 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Tue, 11 Aug 2020 10:55:26 +0800 Subject: [PATCH 1/5] Address some FIXMEs Signed-off-by: JmPotato --- Cargo.lock | 1 + crates/ra_assists/Cargo.toml | 1 + crates/ra_assists/src/ast_transform.rs | 13 +++++-------- crates/ra_assists/src/lib.rs | 25 ++++++++++++++++++++----- crates/rust-analyzer/src/handlers.rs | 2 +- crates/rust-analyzer/src/to_proto.rs | 8 ++++---- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a094ec4f7ae..936e863f52f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -912,6 +912,7 @@ dependencies = [ "ra_db", "ra_fmt", "ra_hir", + "ra_hir_expand", "ra_ide_db", "ra_prof", "ra_syntax", diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml index bd2905f080a..a436e861d7e 100644 --- a/crates/ra_assists/Cargo.toml +++ b/crates/ra_assists/Cargo.toml @@ -22,4 +22,5 @@ ra_prof = { path = "../ra_prof" } ra_db = { path = "../ra_db" } ra_ide_db = { path = "../ra_ide_db" } hir = { path = "../ra_hir", package = "ra_hir" } +hir_expand = { path = "../ra_hir_expand", package = "ra_hir_expand" } test_utils = { path = "../test_utils" } diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index 15ec75c956a..02c4a4baeb0 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -2,6 +2,7 @@ use rustc_hash::FxHashMap; use hir::{HirDisplay, PathResolution, SemanticsScope}; +use hir_expand::hygiene::Hygiene; use ra_syntax::{ algo::SyntaxRewriter, ast::{self, AstNode}, @@ -51,7 +52,7 @@ pub fn for_trait_impl( // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky .skip(1) // The actual list of trait type parameters may be longer than the one - // used in the `impl` block due to trailing default type parametrs. + // used in the `impl` block due to trailing default type parameters. // For that case we extend the `substs` with an empty iterator so we // can still hit those trailing values and check if they actually have // a default type. If they do, go for that type from `hir` to `ast` so @@ -110,9 +111,7 @@ fn get_substitution_inner( ast::Type::PathType(path_type) => path_type.path()?, _ => return None, }; - // FIXME: use `hir::Path::from_src` instead. - #[allow(deprecated)] - let path = hir::Path::from_ast(path)?; + let path = hir::Path::from_src(path, &Hygiene::new_unhygienic())?; let resolution = self.source_scope.resolve_hir_path(&path)?; match resolution { hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()), @@ -152,10 +151,8 @@ fn get_substitution_inner( // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } - // FIXME: use `hir::Path::from_src` instead. - #[allow(deprecated)] - let hir_path = hir::Path::from_ast(p.clone()); - let resolution = self.source_scope.resolve_hir_path(&hir_path?)?; + let hir_path = hir::Path::from_src(p.clone(), &Hygiene::new_unhygienic())?; + let resolution = self.source_scope.resolve_hir_path(&hir_path)?; match resolution { PathResolution::Def(def) => { let found_path = from.find_use_path(self.source_scope.db.upcast(), def)?; diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 507646cc802..890996a68d9 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -66,13 +66,13 @@ pub fn contains(self, other: AssistKind) -> bool { #[derive(Debug, Clone)] pub struct Assist { - pub id: AssistId, + id: AssistId, /// Short description of the assist, as shown in the UI. - pub label: String, - pub group: Option, + label: String, + group: Option, /// Target ranges are used to sort assists: the smaller the target range, /// the more specific assist is, and so it should be sorted first. - pub target: TextRange, + target: TextRange, } #[derive(Debug, Clone)] @@ -120,10 +120,25 @@ pub(crate) fn new( group: Option, target: TextRange, ) -> Assist { - // FIXME: make fields private, so that this invariant can't be broken assert!(label.starts_with(|c: char| c.is_uppercase())); Assist { id, label, group, target } } + + pub fn id(&self) -> AssistId { + self.id + } + + pub fn label(&self) -> String { + self.label.clone() + } + + pub fn group(&self) -> Option { + self.group.clone() + } + + pub fn target(&self) -> TextRange { + self.target + } } mod handlers { diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 895af1dd78e..c2afcf192de 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -864,7 +864,7 @@ pub(crate) fn handle_resolve_code_action( let (id_string, index) = split_once(¶ms.id, ':').unwrap(); let index = index.parse::().unwrap(); let assist = &assists[index]; - assert!(assist.assist.id.0 == id_string); + assert!(assist.assist.id().0 == id_string); Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit) } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 27460db78cc..62fda8a1f20 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -704,10 +704,10 @@ pub(crate) fn unresolved_code_action( index: usize, ) -> Result { let res = lsp_ext::CodeAction { - title: assist.label, - id: Some(format!("{}:{}", assist.id.0.to_owned(), index.to_string())), - group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0), - kind: Some(code_action_kind(assist.id.1)), + title: assist.label(), + id: Some(format!("{}:{}", assist.id().0.to_owned(), index.to_string())), + group: assist.group().filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0), + kind: Some(code_action_kind(assist.id().1)), edit: None, is_preferred: None, }; From ace75f95905564a34f46be57ead51828844da745 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Tue, 11 Aug 2020 12:09:11 +0800 Subject: [PATCH 2/5] Typo fix Signed-off-by: JmPotato --- crates/ra_assists/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index 18fcb904987..e738364220d 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -20,7 +20,7 @@ pub(crate) fn check_assist(assist: Handler, ra_fixture_before: &str, ra_fixture_ // FIXME: instead of having a separate function here, maybe use // `extract_ranges` and mark the target as ` ` in the -// fixuture? +// fixture? pub(crate) fn check_assist_target(assist: Handler, ra_fixture: &str, target: &str) { check(assist, ra_fixture, ExpectedResult::Target(target)); } From b69dfddb572b9182f4880065ca5034aba8b15ce3 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Tue, 11 Aug 2020 14:35:15 +0800 Subject: [PATCH 3/5] Remove redundant dependencies Signed-off-by: JmPotato --- Cargo.lock | 1 - crates/ra_assists/Cargo.toml | 1 - crates/ra_assists/src/ast_transform.rs | 5 ++--- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 936e863f52f..a094ec4f7ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -912,7 +912,6 @@ dependencies = [ "ra_db", "ra_fmt", "ra_hir", - "ra_hir_expand", "ra_ide_db", "ra_prof", "ra_syntax", diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml index a436e861d7e..bd2905f080a 100644 --- a/crates/ra_assists/Cargo.toml +++ b/crates/ra_assists/Cargo.toml @@ -22,5 +22,4 @@ ra_prof = { path = "../ra_prof" } ra_db = { path = "../ra_db" } ra_ide_db = { path = "../ra_ide_db" } hir = { path = "../ra_hir", package = "ra_hir" } -hir_expand = { path = "../ra_hir_expand", package = "ra_hir_expand" } test_utils = { path = "../test_utils" } diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index 02c4a4baeb0..6c92124eda1 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -2,7 +2,6 @@ use rustc_hash::FxHashMap; use hir::{HirDisplay, PathResolution, SemanticsScope}; -use hir_expand::hygiene::Hygiene; use ra_syntax::{ algo::SyntaxRewriter, ast::{self, AstNode}, @@ -111,7 +110,7 @@ fn get_substitution_inner( ast::Type::PathType(path_type) => path_type.path()?, _ => return None, }; - let path = hir::Path::from_src(path, &Hygiene::new_unhygienic())?; + let path = hir::Path::from_src(path, &hir::Hygiene::new_unhygienic())?; let resolution = self.source_scope.resolve_hir_path(&path)?; match resolution { hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()), @@ -151,7 +150,7 @@ fn get_substitution_inner( // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } - let hir_path = hir::Path::from_src(p.clone(), &Hygiene::new_unhygienic())?; + let hir_path = hir::Path::from_src(p.clone(), &hir::Hygiene::new_unhygienic())?; let resolution = self.source_scope.resolve_hir_path(&hir_path)?; match resolution { PathResolution::Def(def) => { From 7fbc9afca48240cf16c82a996ac7b14c554bade6 Mon Sep 17 00:00:00 2001 From: JmPotato Date: Tue, 11 Aug 2020 16:50:45 +0800 Subject: [PATCH 4/5] Typo fix Signed-off-by: JmPotato --- crates/ra_hir_expand/src/hygiene.rs | 2 +- crates/ra_hir_expand/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ra_hir_expand/src/hygiene.rs b/crates/ra_hir_expand/src/hygiene.rs index 6b482a60c54..aefe47bd32a 100644 --- a/crates/ra_hir_expand/src/hygiene.rs +++ b/crates/ra_hir_expand/src/hygiene.rs @@ -17,7 +17,7 @@ pub struct Hygiene { // This is what `$crate` expands to def_crate: Option, - // Indiciate this is a local inner macro + // Indicate this is a local inner macro local_inner: bool, } diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index 2e8d6369171..abae498d826 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -44,7 +44,7 @@ /// containing the call plus the offset of the macro call in the file. Note that /// this is a recursive definition! However, the size_of of `HirFileId` is /// finite (because everything bottoms out at the real `FileId`) and small -/// (`MacroCallId` uses the location interner). +/// (`MacroCallId` uses the location internal). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct HirFileId(HirFileIdRepr); From 6ef019bd4601b9dc36325e096d066a4ddbda1bdf Mon Sep 17 00:00:00 2001 From: JmPotato Date: Tue, 11 Aug 2020 17:19:02 +0800 Subject: [PATCH 5/5] Revert some FIXMEs Signed-off-by: JmPotato --- crates/ra_assists/src/ast_transform.rs | 10 +++++++--- crates/ra_hir_expand/src/lib.rs | 3 ++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index 6c92124eda1..07c978378a6 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -110,7 +110,9 @@ fn get_substitution_inner( ast::Type::PathType(path_type) => path_type.path()?, _ => return None, }; - let path = hir::Path::from_src(path, &hir::Hygiene::new_unhygienic())?; + // FIXME: use `hir::Path::from_src` instead. + #[allow(deprecated)] + let path = hir::Path::from_ast(path)?; let resolution = self.source_scope.resolve_hir_path(&path)?; match resolution { hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()), @@ -150,8 +152,10 @@ fn get_substitution_inner( // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } - let hir_path = hir::Path::from_src(p.clone(), &hir::Hygiene::new_unhygienic())?; - let resolution = self.source_scope.resolve_hir_path(&hir_path)?; + // FIXME: use `hir::Path::from_src` instead. + #[allow(deprecated)] + let hir_path = hir::Path::from_ast(p.clone()); + let resolution = self.source_scope.resolve_hir_path(&hir_path?)?; match resolution { PathResolution::Def(def) => { let found_path = from.find_use_path(self.source_scope.db.upcast(), def)?; diff --git a/crates/ra_hir_expand/src/lib.rs b/crates/ra_hir_expand/src/lib.rs index abae498d826..8bb735fc625 100644 --- a/crates/ra_hir_expand/src/lib.rs +++ b/crates/ra_hir_expand/src/lib.rs @@ -44,7 +44,8 @@ /// containing the call plus the offset of the macro call in the file. Note that /// this is a recursive definition! However, the size_of of `HirFileId` is /// finite (because everything bottoms out at the real `FileId`) and small -/// (`MacroCallId` uses the location internal). +/// (`MacroCallId` uses the location interning. You can check details here: +/// https://en.wikipedia.org/wiki/String_interning). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct HirFileId(HirFileIdRepr);