aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-08-17 15:13:07 +0100
committerGitHub <[email protected]>2020-08-17 15:13:07 +0100
commit64bbdb5bc7e61d2d6ff67e722e34275ef2a47554 (patch)
tree25305a222e79648fff2363789a5047faf0d4f562
parentdf225472762c92aec3c991a3d0f78bad5058a39b (diff)
parent6a4c9fc9fd6cb9ecf08bd5a22890eb19d22ad34e (diff)
Merge #5785
5785: Don't make fields private unless you have to r=matklad a=matklad bors r+ 🤖 Co-authored-by: Aleksey Kladov <[email protected]>
-rw-r--r--crates/assists/src/lib.rs37
-rw-r--r--crates/rust-analyzer/src/handlers.rs4
-rw-r--r--crates/rust-analyzer/src/to_proto.rs8
-rw-r--r--docs/dev/style.md29
4 files changed, 45 insertions, 33 deletions
diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs
index ae90d68a3..c589b08dc 100644
--- a/crates/assists/src/lib.rs
+++ b/crates/assists/src/lib.rs
@@ -66,13 +66,13 @@ pub struct GroupLabel(pub String);
66 66
67#[derive(Debug, Clone)] 67#[derive(Debug, Clone)]
68pub struct Assist { 68pub struct Assist {
69 id: AssistId, 69 pub id: AssistId,
70 /// Short description of the assist, as shown in the UI. 70 /// Short description of the assist, as shown in the UI.
71 label: String, 71 label: String,
72 group: Option<GroupLabel>, 72 pub group: Option<GroupLabel>,
73 /// Target ranges are used to sort assists: the smaller the target range, 73 /// Target ranges are used to sort assists: the smaller the target range,
74 /// the more specific assist is, and so it should be sorted first. 74 /// the more specific assist is, and so it should be sorted first.
75 target: TextRange, 75 pub target: TextRange,
76} 76}
77 77
78#[derive(Debug, Clone)] 78#[derive(Debug, Clone)]
@@ -82,6 +82,11 @@ pub struct ResolvedAssist {
82} 82}
83 83
84impl Assist { 84impl Assist {
85 fn new(id: AssistId, label: String, group: Option<GroupLabel>, target: TextRange) -> Assist {
86 assert!(label.starts_with(char::is_uppercase));
87 Assist { id, label, group, target }
88 }
89
85 /// Return all the assists applicable at the given position. 90 /// Return all the assists applicable at the given position.
86 /// 91 ///
87 /// Assists are returned in the "unresolved" state, that is only labels are 92 /// Assists are returned in the "unresolved" state, that is only labels are
@@ -114,30 +119,8 @@ impl Assist {
114 acc.finish_resolved() 119 acc.finish_resolved()
115 } 120 }
116 121
117 pub(crate) fn new( 122 pub fn label(&self) -> &str {
118 id: AssistId, 123 self.label.as_str()
119 label: String,
120 group: Option<GroupLabel>,
121 target: TextRange,
122 ) -> Assist {
123 assert!(label.starts_with(|c: char| c.is_uppercase()));
124 Assist { id, label, group, target }
125 }
126
127 pub fn id(&self) -> AssistId {
128 self.id
129 }
130
131 pub fn label(&self) -> String {
132 self.label.clone()
133 }
134
135 pub fn group(&self) -> Option<GroupLabel> {
136 self.group.clone()
137 }
138
139 pub fn target(&self) -> TextRange {
140 self.target
141 } 124 }
142} 125}
143 126
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 74f73655a..e05ffc768 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -859,10 +859,10 @@ pub(crate) fn handle_resolve_code_action(
859 .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect()); 859 .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect());
860 860
861 let assists = snap.analysis.resolved_assists(&snap.config.assist, frange)?; 861 let assists = snap.analysis.resolved_assists(&snap.config.assist, frange)?;
862 let (id_string, index) = split_once(&params.id, ':').unwrap(); 862 let (id, index) = split_once(&params.id, ':').unwrap();
863 let index = index.parse::<usize>().unwrap(); 863 let index = index.parse::<usize>().unwrap();
864 let assist = &assists[index]; 864 let assist = &assists[index];
865 assert!(assist.assist.id().0 == id_string); 865 assert!(assist.assist.id.0 == id);
866 Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit) 866 Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit)
867} 867}
868 868
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 8a2cfa2ae..535de2f71 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(
704 index: usize, 704 index: usize,
705) -> Result<lsp_ext::CodeAction> { 705) -> Result<lsp_ext::CodeAction> {
706 let res = lsp_ext::CodeAction { 706 let res = lsp_ext::CodeAction {
707 title: assist.label(), 707 title: assist.label().to_string(),
708 id: Some(format!("{}:{}", assist.id().0.to_owned(), index.to_string())), 708 id: Some(format!("{}:{}", assist.id.0, index.to_string())),
709 group: assist.group().filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0), 709 group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0),
710 kind: Some(code_action_kind(assist.id().1)), 710 kind: Some(code_action_kind(assist.id.1)),
711 edit: None, 711 edit: None,
712 is_preferred: None, 712 is_preferred: None,
713 }; 713 };
diff --git a/docs/dev/style.md b/docs/dev/style.md
index 963a6d73d..8effddcda 100644
--- a/docs/dev/style.md
+++ b/docs/dev/style.md
@@ -176,6 +176,35 @@ fn frobnicate(walrus: Option<Walrus>) {
176} 176}
177``` 177```
178 178
179# Getters & Setters
180
181If a field can have any value without breaking invariants, make the field public.
182Conversely, if there is an invariant, document it, enforce it in the "constructor" function, make the field private, and provide a getter.
183Never provide setters.
184
185Getters should return borrowed data:
186
187```
188struct Person {
189 // Invariant: never empty
190 first_name: String,
191 middle_name: Option<String>
192}
193
194// Good
195impl Person {
196 fn first_name(&self) -> &str { self.first_name.as_str() }
197 fn middle_name(&self) -> Option<&str> { self.middle_name.as_ref() }
198}
199
200// Not as good
201impl Person {
202 fn first_name(&self) -> String { self.first_name.clone() }
203 fn middle_name(&self) -> &Option<String> { &self.middle_name }
204}
205```
206
207
179# Premature Pessimization 208# Premature Pessimization
180 209
181Avoid writing code which is slower than it needs to be. 210Avoid writing code which is slower than it needs to be.