diff options
Diffstat (limited to 'docs/dev')
-rw-r--r-- | docs/dev/README.md | 105 | ||||
-rw-r--r-- | docs/dev/lsp-extensions.md | 80 |
2 files changed, 148 insertions, 37 deletions
diff --git a/docs/dev/README.md b/docs/dev/README.md index 65cc9fc12..1de5a2aab 100644 --- a/docs/dev/README.md +++ b/docs/dev/README.md | |||
@@ -30,7 +30,7 @@ https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0 | |||
30 | 30 | ||
31 | * [good-first-issue](https://github.com/rust-analyzer/rust-analyzer/labels/good%20first%20issue) | 31 | * [good-first-issue](https://github.com/rust-analyzer/rust-analyzer/labels/good%20first%20issue) |
32 | are good issues to get into the project. | 32 | are good issues to get into the project. |
33 | * [E-mentor](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-mentor) | 33 | * [E-has-instructions](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-has-instructions) |
34 | issues have links to the code in question and tests. | 34 | issues have links to the code in question and tests. |
35 | * [E-easy](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-easy), | 35 | * [E-easy](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-easy), |
36 | [E-medium](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-medium), | 36 | [E-medium](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-medium), |
@@ -117,6 +117,109 @@ Additionally, I use `cargo run --release -p rust-analyzer -- analysis-stats | |||
117 | path/to/some/rust/crate` to run a batch analysis. This is primarily useful for | 117 | path/to/some/rust/crate` to run a batch analysis. This is primarily useful for |
118 | performance optimizations, or for bug minimization. | 118 | performance optimizations, or for bug minimization. |
119 | 119 | ||
120 | # Code Style & Review Process | ||
121 | |||
122 | Our approach to "clean code" is two fold: | ||
123 | |||
124 | * We generally don't block PRs on style changes. | ||
125 | * At the same time, all code in rust-analyzer is constantly refactored. | ||
126 | |||
127 | It is explicitly OK for reviewer to flag only some nits in the PR, and than send a follow up cleanup PR for things which are easier to explain by example, cc-ing the original author. | ||
128 | Sending small cleanup PRs (like rename a single local variable) is encouraged. | ||
129 | |||
130 | ## Scale of Changes | ||
131 | |||
132 | Everyone knows that it's better to send small & focused pull requests. | ||
133 | The problem is, sometimes you *have* to, eg, rewrite the whole compiler, and that just doesn't fit into a set of isolated PRs. | ||
134 | |||
135 | The main thing too keep an eye on is the boundaries between various components. | ||
136 | There are three kinds of changes: | ||
137 | |||
138 | 1. Internals of a single component are changed. | ||
139 | Specifically, you don't change any `pub` items. | ||
140 | A good example here would be an addition of a new assist. | ||
141 | |||
142 | 2. API of a component is expanded. | ||
143 | Specifically, you add a new `pub` function which wasn't there before. | ||
144 | A good example here would be expansion of assist API, for example, to implement lazy assists or assists groups. | ||
145 | |||
146 | 3. A new dependency between components is introduced. | ||
147 | Specifically, you add a `pub use` reexport from another crate or you add a new line to `[dependencies]` section of `Cargo.toml`. | ||
148 | A good example here would be adding reference search capability to the assists crates. | ||
149 | |||
150 | For the first group, the change is generally merged as long as: | ||
151 | |||
152 | * it works for the happy case, | ||
153 | * it has tests, | ||
154 | * it doesn't panic for unhappy case. | ||
155 | |||
156 | For the second group, the change would be subjected to quite a bit of scrutiny and iteration. | ||
157 | The new API needs to be right (or at least easy to change later). | ||
158 | The actual implementation doesn't matter that much. | ||
159 | It's very important to minimize the amount of changed lines of code for changes of the second kind. | ||
160 | Often, you start doing change of the first kind, only to realise that you need to elevate to a change of the second kind. | ||
161 | In this case, we'll probably ask you to split API changes into a separate PR. | ||
162 | |||
163 | Changes of the third group should be pretty rare, so we don't specify any specific process for them. | ||
164 | That said, adding an innocent-looking `pub use` is a very simple way to break encapsulation, keep an eye on it! | ||
165 | |||
166 | Note: if you enjoyed this abstract hand-waving about boundaries, you might appreciate | ||
167 | https://www.tedinski.com/2018/02/06/system-boundaries.html | ||
168 | |||
169 | ## Order of Imports | ||
170 | |||
171 | We separate import groups with blank lines | ||
172 | |||
173 | ``` | ||
174 | mod x; | ||
175 | mod y; | ||
176 | |||
177 | use std::{ ... } | ||
178 | |||
179 | use crate_foo::{ ... } | ||
180 | use crate_bar::{ ... } | ||
181 | |||
182 | use crate::{} | ||
183 | |||
184 | use super::{} // but prefer `use crate::` | ||
185 | ``` | ||
186 | |||
187 | ## Order of Items | ||
188 | |||
189 | Optimize for the reader who sees the file for the first time, and wants to get the general idea about what's going on. | ||
190 | People read things from top to bottom, so place most important things first. | ||
191 | |||
192 | Specifically, if all items except one are private, always put the non-private item on top. | ||
193 | |||
194 | Put `struct`s and `enum`s first, functions and impls last. | ||
195 | |||
196 | Do | ||
197 | |||
198 | ``` | ||
199 | // Good | ||
200 | struct Foo { | ||
201 | bars: Vec<Bar> | ||
202 | } | ||
203 | |||
204 | struct Bar; | ||
205 | ``` | ||
206 | |||
207 | rather than | ||
208 | |||
209 | ``` | ||
210 | // Not as good | ||
211 | struct Bar; | ||
212 | |||
213 | struct Foo { | ||
214 | bars: Vec<Bar> | ||
215 | } | ||
216 | ``` | ||
217 | |||
218 | ## Documentation | ||
219 | |||
220 | For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines. | ||
221 | If the line is too long, you want to split the sentence in two :-) | ||
222 | |||
120 | # Logging | 223 | # Logging |
121 | 224 | ||
122 | Logging is done by both rust-analyzer and VS Code, so it might be tricky to | 225 | Logging is done by both rust-analyzer and VS Code, so it might be tricky to |
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index c57a93f12..647cf6107 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md | |||
@@ -311,6 +311,50 @@ Moreover, it would be cool if editors didn't need to implement even basic langua | |||
311 | This is how `SelectionRange` request works. | 311 | This is how `SelectionRange` request works. |
312 | * Alternatively, should we perhaps flag certain `SelectionRange`s as being brace pairs? | 312 | * Alternatively, should we perhaps flag certain `SelectionRange`s as being brace pairs? |
313 | 313 | ||
314 | ## Runnables | ||
315 | |||
316 | **Issue:** https://github.com/microsoft/language-server-protocol/issues/944 | ||
317 | |||
318 | **Server Capability:** `{ "runnables": { "kinds": string[] } }` | ||
319 | |||
320 | This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`). | ||
321 | |||
322 | **Method:** `experimental/runnables` | ||
323 | |||
324 | **Request:** | ||
325 | |||
326 | ```typescript | ||
327 | interface RunnablesParams { | ||
328 | textDocument: TextDocumentIdentifier; | ||
329 | /// If null, compute runnables for the whole file. | ||
330 | position?: Position; | ||
331 | } | ||
332 | ``` | ||
333 | |||
334 | **Response:** `Runnable[]` | ||
335 | |||
336 | ```typescript | ||
337 | interface Runnable { | ||
338 | label: string; | ||
339 | /// If this Runnable is associated with a specific function/module, etc, the location of this item | ||
340 | location?: LocationLink; | ||
341 | /// Running things is necessary technology specific, `kind` needs to be advertised via server capabilities, | ||
342 | // the type of `args` is specific to `kind`. The actual running is handled by the client. | ||
343 | kind: string; | ||
344 | args: any; | ||
345 | } | ||
346 | ``` | ||
347 | |||
348 | rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look like this: | ||
349 | |||
350 | ```typescript | ||
351 | { | ||
352 | workspaceRoot?: string; | ||
353 | cargoArgs: string[]; | ||
354 | executableArgs: string[]; | ||
355 | } | ||
356 | ``` | ||
357 | |||
314 | ## Analyzer Status | 358 | ## Analyzer Status |
315 | 359 | ||
316 | **Method:** `rust-analyzer/analyzerStatus` | 360 | **Method:** `rust-analyzer/analyzerStatus` |
@@ -399,39 +443,3 @@ interface InlayHint { | |||
399 | label: string, | 443 | label: string, |
400 | } | 444 | } |
401 | ``` | 445 | ``` |
402 | |||
403 | ## Runnables | ||
404 | |||
405 | **Method:** `rust-analyzer/runnables` | ||
406 | |||
407 | This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`). | ||
408 | Note that we plan to move this request to `experimental/runnables`, as it is not really Rust-specific, but the current API is not necessary the right one. | ||
409 | Upstream issue: https://github.com/microsoft/language-server-protocol/issues/944 | ||
410 | |||
411 | **Request:** | ||
412 | |||
413 | ```typescript | ||
414 | interface RunnablesParams { | ||
415 | textDocument: TextDocumentIdentifier; | ||
416 | /// If null, compute runnables for the whole file. | ||
417 | position?: Position; | ||
418 | } | ||
419 | ``` | ||
420 | |||
421 | **Response:** `Runnable[]` | ||
422 | |||
423 | ```typescript | ||
424 | interface Runnable { | ||
425 | /// The range this runnable is applicable for. | ||
426 | range: lc.Range; | ||
427 | /// The label to show in the UI. | ||
428 | label: string; | ||
429 | /// The following fields describe a process to spawn. | ||
430 | bin: string; | ||
431 | args: string[]; | ||
432 | /// Args for cargo after `--`. | ||
433 | extraArgs: string[]; | ||
434 | env: { [key: string]: string }; | ||
435 | cwd: string | null; | ||
436 | } | ||
437 | ``` | ||