Merge #5707

5707: Address some FIXMEs for ra_assists r=jonas-schievink a=JmPotato

Signed-off-by: JmPotato <[email protected]>

Co-authored-by: JmPotato <[email protected]>

7 files changed, 30 insertions, 14 deletions

diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs
index 15ec75c95..07c978378 100644
--- a/crates/ra_assists/src/ast_transform.rs
+++ b/crates/ra_assists/src/ast_transform.rs
@@ -51,7 +51,7 @@ impl<'a> SubstituteTypeParams<'a> {
             // 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
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 507646cc8..890996a68 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -66,13 +66,13 @@ pub struct GroupLabel(pub String);
 
 #[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<GroupLabel>,
+    label: String,
+    group: Option<GroupLabel>,
     /// 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 @@ impl Assist {
         group: Option<GroupLabel>,
         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<GroupLabel> {
+        self.group.clone()
+    }
+
+    pub fn target(&self) -> TextRange {
+        self.target
+    }
 }
 
 mod handlers {
diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs
index 18fcb9049..e73836422 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 `<target> </target>` in the
-// fixuture?
+// fixture?
 pub(crate) fn check_assist_target(assist: Handler, ra_fixture: &str, target: &str) {
     check(assist, ra_fixture, ExpectedResult::Target(target));
 }
diff --git a/crates/ra_hir_expand/src/hygiene.rs b/crates/ra_hir_expand/src/hygiene.rs
index 6b482a60c..aefe47bd3 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<CrateId>,
 
-    // 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 2e8d63691..8bb735fc6 100644
--- a/crates/ra_hir_expand/src/lib.rs
+++ b/crates/ra_hir_expand/src/lib.rs
@@ -44,7 +44,8 @@ mod test_db;
 /// 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 interning. You can check details here:
+/// https://en.wikipedia.org/wiki/String_interning).
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct HirFileId(HirFileIdRepr);
 
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 895af1dd7..c2afcf192 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(&params.id, ':').unwrap();
     let index = index.parse::<usize>().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 27460db78..62fda8a1f 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<lsp_ext::CodeAction> {
     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,
     };