diff options
author | Mikhail Rakhmanov <[email protected]> | 2020-06-13 07:42:15 +0100 |
---|---|---|
committer | Mikhail Rakhmanov <[email protected]> | 2020-06-13 07:42:15 +0100 |
commit | 16bbf4ab7f132e6e5e5318dccdef9a5d71afdd7f (patch) | |
tree | 4b79fa8c046be56b02427ba843e70cdf3ac05767 /docs/dev | |
parent | eeb8b9e236796da8734ba81a49164864497f7226 (diff) | |
parent | b56ad148db0c69eb279c225f45d324b4e80e7367 (diff) |
Merge branch 'master' into keyword_completion
# Conflicts:
# docs/user/generated_features.adoc
Diffstat (limited to 'docs/dev')
-rw-r--r-- | docs/dev/README.md | 209 | ||||
-rw-r--r-- | docs/dev/lsp-extensions.md | 122 |
2 files changed, 299 insertions, 32 deletions
diff --git a/docs/dev/README.md b/docs/dev/README.md index 65cc9fc12..ef5ffbf59 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), |
@@ -55,7 +55,7 @@ You can run `cargo xtask install-pre-commit-hook` to install git-hook to run rus | |||
55 | All Rust code lives in the `crates` top-level directory, and is organized as a | 55 | All Rust code lives in the `crates` top-level directory, and is organized as a |
56 | single Cargo workspace. The `editors` top-level directory contains code for | 56 | single Cargo workspace. The `editors` top-level directory contains code for |
57 | integrating with editors. Currently, it contains the plugin for VS Code (in | 57 | integrating with editors. Currently, it contains the plugin for VS Code (in |
58 | typescript). The `docs` top-level directory contains both developer and user | 58 | TypeScript). The `docs` top-level directory contains both developer and user |
59 | documentation. | 59 | documentation. |
60 | 60 | ||
61 | We have some automation infra in Rust in the `xtask` package. It contains | 61 | We have some automation infra in Rust in the `xtask` package. It contains |
@@ -79,8 +79,8 @@ possible. There's **"Run Extension (Debug Build)"** launch configuration for thi | |||
79 | In general, I use one of the following workflows for fixing bugs and | 79 | In general, I use one of the following workflows for fixing bugs and |
80 | implementing features. | 80 | implementing features. |
81 | 81 | ||
82 | If the problem concerns only internal parts of rust-analyzer (ie, I don't need | 82 | If the problem concerns only internal parts of rust-analyzer (i.e. I don't need |
83 | to touch `rust-analyzer` crate or typescript code), there is a unit-test for it. | 83 | to touch `rust-analyzer` crate or TypeScript code), there is a unit-test for it. |
84 | So, I use **Rust Analyzer: Run** action in VS Code to run this single test, and | 84 | So, I use **Rust Analyzer: Run** action in VS Code to run this single test, and |
85 | then just do printf-driven development/debugging. As a sanity check after I'm | 85 | then just do printf-driven development/debugging. As a sanity check after I'm |
86 | done, I use `cargo xtask install --server` and **Reload Window** action in VS | 86 | done, I use `cargo xtask install --server` and **Reload Window** action in VS |
@@ -117,6 +117,203 @@ 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 | ```rust | ||
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 | ## Import Style | ||
188 | |||
189 | Items from `hir` and `ast` should be used qualified: | ||
190 | |||
191 | ```rust | ||
192 | // Good | ||
193 | use ra_syntax::ast; | ||
194 | |||
195 | fn frobnicate(func: hir::Function, strukt: ast::StructDef) {} | ||
196 | |||
197 | // Not as good | ||
198 | use hir::Function; | ||
199 | use ra_syntax::ast::StructDef; | ||
200 | |||
201 | fn frobnicate(func: Function, strukt: StructDef) {} | ||
202 | ``` | ||
203 | |||
204 | Avoid local `use MyEnum::*` imports. | ||
205 | |||
206 | Prefer `use crate::foo::bar` to `use super::bar`. | ||
207 | |||
208 | ## Order of Items | ||
209 | |||
210 | Optimize for the reader who sees the file for the first time, and wants to get the general idea about what's going on. | ||
211 | People read things from top to bottom, so place most important things first. | ||
212 | |||
213 | Specifically, if all items except one are private, always put the non-private item on top. | ||
214 | |||
215 | Put `struct`s and `enum`s first, functions and impls last. | ||
216 | |||
217 | Do | ||
218 | |||
219 | ```rust | ||
220 | // Good | ||
221 | struct Foo { | ||
222 | bars: Vec<Bar> | ||
223 | } | ||
224 | |||
225 | struct Bar; | ||
226 | ``` | ||
227 | |||
228 | rather than | ||
229 | |||
230 | ```rust | ||
231 | // Not as good | ||
232 | struct Bar; | ||
233 | |||
234 | struct Foo { | ||
235 | bars: Vec<Bar> | ||
236 | } | ||
237 | ``` | ||
238 | |||
239 | ## Documentation | ||
240 | |||
241 | For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines. | ||
242 | If the line is too long, you want to split the sentence in two :-) | ||
243 | |||
244 | ## Preconditions | ||
245 | |||
246 | Function preconditions should generally be expressed in types and provided by the caller (rather than checked by callee): | ||
247 | |||
248 | ```rust | ||
249 | // Good | ||
250 | fn frbonicate(walrus: Walrus) { | ||
251 | ... | ||
252 | } | ||
253 | |||
254 | // Not as good | ||
255 | fn frobnicate(walrus: Option<Walrus>) { | ||
256 | let walrus = match walrus { | ||
257 | Some(it) => it, | ||
258 | None => return, | ||
259 | }; | ||
260 | ... | ||
261 | } | ||
262 | ``` | ||
263 | |||
264 | ## Commit Style | ||
265 | |||
266 | We don't have specific rules around git history hygiene. | ||
267 | Maintaining clean git history is encouraged, but not enforced. | ||
268 | We use rebase workflow, it's OK to rewrite history during PR review process. | ||
269 | |||
270 | Avoid @mentioning people in commit messages, as such messages create a lot of duplicate notification traffic during rebases. | ||
271 | |||
272 | # Architecture Invariants | ||
273 | |||
274 | This section tries to document high-level design constraints, which are not | ||
275 | always obvious from the low-level code. | ||
276 | |||
277 | ## Incomplete syntax trees | ||
278 | |||
279 | Syntax trees are by design incomplete and do not enforce well-formedness. | ||
280 | If ast method returns an `Option`, it *can* be `None` at runtime, even if this is forbidden by the grammar. | ||
281 | |||
282 | ## LSP independence | ||
283 | |||
284 | rust-analyzer is independent from LSP. | ||
285 | It provides features for a hypothetical perfect Rust-specific IDE client. | ||
286 | Internal representations are lowered to LSP in the `rust-analyzer` crate (the only crate which is allowed to use LSP types). | ||
287 | |||
288 | ## IDE/Compiler split | ||
289 | |||
290 | There's a semi-hard split between "compiler" and "IDE", at the `ra_hir` crate. | ||
291 | Compiler derives new facts about source code. | ||
292 | It explicitly acknowledges that not all info is available (i.e. you can't look at types during name resolution). | ||
293 | |||
294 | IDE assumes that all information is available at all times. | ||
295 | |||
296 | IDE should use only types from `ra_hir`, and should not depend on the underling compiler types. | ||
297 | `ra_hir` is a facade. | ||
298 | |||
299 | ## IDE API | ||
300 | |||
301 | The main IDE crate (`ra_ide`) uses "Plain Old Data" for the API. | ||
302 | Rather than talking in definitions and references, it talks in Strings and textual offsets. | ||
303 | In general, API is centered around UI concerns -- the result of the call is what the user sees in the editor, and not what the compiler sees underneath. | ||
304 | The results are 100% Rust specific though. | ||
305 | |||
306 | ## Parser Tests | ||
307 | |||
308 | Test for parser (`ra_parser`) live in `ra_syntax` crate (see `test_data` direcotory). | ||
309 | There are two kinds of tests: | ||
310 | |||
311 | * Manually written test cases in `parser/ok` and `parser/err` | ||
312 | * "Inline" tests in `parser/inline` (these are generated) from comments in `ra_parser` crate. | ||
313 | |||
314 | The purpose of inline tests is not to achieve full coverage by test cases, but to explain to the reader of the code what each particular `if` and `match` is responsible for. | ||
315 | If you are tempted to add a large inline test, it might be a good idea to leave only the simplest example in place, and move the test to a manual `parser/ok` test. | ||
316 | |||
120 | # Logging | 317 | # Logging |
121 | 318 | ||
122 | Logging is done by both rust-analyzer and VS Code, so it might be tricky to | 319 | Logging is done by both rust-analyzer and VS Code, so it might be tricky to |
@@ -159,8 +356,8 @@ There's also two VS Code commands which might be of interest: | |||
159 | rust code that it refers to and the rust editor will also highlight the proper | 356 | rust code that it refers to and the rust editor will also highlight the proper |
160 | text range. | 357 | text range. |
161 | 358 | ||
162 | If you press <kbd>Ctrl</kbd> (i.e. trigger goto definition) in the inspected | 359 | If you trigger Go to Definition in the inspected Rust source file, |
163 | Rust source file the syntax tree read-only editor should scroll to and select the | 360 | the syntax tree read-only editor should scroll to and select the |
164 | appropriate syntax node token. | 361 | appropriate syntax node token. |
165 | 362 | ||
166 |  | 363 |  |
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index c57a93f12..a0847dad3 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md | |||
@@ -97,6 +97,30 @@ Invoking code action at this position will yield two code actions for importing | |||
97 | * Is a fixed two-level structure enough? | 97 | * Is a fixed two-level structure enough? |
98 | * Should we devise a general way to encode custom interaction protocols for GUI refactorings? | 98 | * Should we devise a general way to encode custom interaction protocols for GUI refactorings? |
99 | 99 | ||
100 | ## Lazy assists with `ResolveCodeAction` | ||
101 | |||
102 | **Issue:** https://github.com/microsoft/language-server-protocol/issues/787 | ||
103 | |||
104 | **Client Capability** `{ "resolveCodeAction": boolean }` | ||
105 | |||
106 | If this capability is set, the assists will be computed lazily. Thus `CodeAction` returned from the server will only contain `id` but not `edit` or `command` fields. The only exclusion from the rule is the diagnostic edits. | ||
107 | |||
108 | After the client got the id, it should then call `experimental/resolveCodeAction` command on the server and provide the following payload: | ||
109 | |||
110 | ```typescript | ||
111 | interface ResolveCodeActionParams { | ||
112 | id: string; | ||
113 | codeActionParams: lc.CodeActionParams; | ||
114 | } | ||
115 | ``` | ||
116 | |||
117 | As a result of the command call the client will get the respective workspace edit (`lc.WorkspaceEdit`). | ||
118 | |||
119 | ### Unresolved Questions | ||
120 | |||
121 | * Apply smarter filtering for ids? | ||
122 | * Upon `resolveCodeAction` command only call the assits which should be resolved and not all of them? | ||
123 | |||
100 | ## Parent Module | 124 | ## Parent Module |
101 | 125 | ||
102 | **Issue:** https://github.com/microsoft/language-server-protocol/issues/1002 | 126 | **Issue:** https://github.com/microsoft/language-server-protocol/issues/1002 |
@@ -311,6 +335,50 @@ Moreover, it would be cool if editors didn't need to implement even basic langua | |||
311 | This is how `SelectionRange` request works. | 335 | This is how `SelectionRange` request works. |
312 | * Alternatively, should we perhaps flag certain `SelectionRange`s as being brace pairs? | 336 | * Alternatively, should we perhaps flag certain `SelectionRange`s as being brace pairs? |
313 | 337 | ||
338 | ## Runnables | ||
339 | |||
340 | **Issue:** https://github.com/microsoft/language-server-protocol/issues/944 | ||
341 | |||
342 | **Server Capability:** `{ "runnables": { "kinds": string[] } }` | ||
343 | |||
344 | This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`). | ||
345 | |||
346 | **Method:** `experimental/runnables` | ||
347 | |||
348 | **Request:** | ||
349 | |||
350 | ```typescript | ||
351 | interface RunnablesParams { | ||
352 | textDocument: TextDocumentIdentifier; | ||
353 | /// If null, compute runnables for the whole file. | ||
354 | position?: Position; | ||
355 | } | ||
356 | ``` | ||
357 | |||
358 | **Response:** `Runnable[]` | ||
359 | |||
360 | ```typescript | ||
361 | interface Runnable { | ||
362 | label: string; | ||
363 | /// If this Runnable is associated with a specific function/module, etc, the location of this item | ||
364 | location?: LocationLink; | ||
365 | /// Running things is necessary technology specific, `kind` needs to be advertised via server capabilities, | ||
366 | // the type of `args` is specific to `kind`. The actual running is handled by the client. | ||
367 | kind: string; | ||
368 | args: any; | ||
369 | } | ||
370 | ``` | ||
371 | |||
372 | rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look like this: | ||
373 | |||
374 | ```typescript | ||
375 | { | ||
376 | workspaceRoot?: string; | ||
377 | cargoArgs: string[]; | ||
378 | executableArgs: string[]; | ||
379 | } | ||
380 | ``` | ||
381 | |||
314 | ## Analyzer Status | 382 | ## Analyzer Status |
315 | 383 | ||
316 | **Method:** `rust-analyzer/analyzerStatus` | 384 | **Method:** `rust-analyzer/analyzerStatus` |
@@ -400,38 +468,40 @@ interface InlayHint { | |||
400 | } | 468 | } |
401 | ``` | 469 | ``` |
402 | 470 | ||
403 | ## Runnables | 471 | ## Hover Actions |
404 | |||
405 | **Method:** `rust-analyzer/runnables` | ||
406 | 472 | ||
407 | This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`). | 473 | **Client Capability:** `{ "hoverActions": boolean }` |
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 | 474 | ||
411 | **Request:** | 475 | If this capability is set, `Hover` request returned from the server might contain an additional field, `actions`: |
412 | 476 | ||
413 | ```typescript | 477 | ```typescript |
414 | interface RunnablesParams { | 478 | interface Hover { |
415 | textDocument: TextDocumentIdentifier; | 479 | ... |
416 | /// If null, compute runnables for the whole file. | 480 | actions?: CommandLinkGroup[]; |
417 | position?: Position; | ||
418 | } | 481 | } |
419 | ``` | ||
420 | 482 | ||
421 | **Response:** `Runnable[]` | 483 | interface CommandLink extends Command { |
484 | /** | ||
485 | * A tooltip for the command, when represented in the UI. | ||
486 | */ | ||
487 | tooltip?: string; | ||
488 | } | ||
422 | 489 | ||
423 | ```typescript | 490 | interface CommandLinkGroup { |
424 | interface Runnable { | 491 | title?: string; |
425 | /// The range this runnable is applicable for. | 492 | commands: CommandLink[]; |
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 | } | 493 | } |
437 | ``` | 494 | ``` |
495 | |||
496 | Such actions on the client side are appended to a hover bottom as command links: | ||
497 | ``` | ||
498 | +-----------------------------+ | ||
499 | | Hover content | | ||
500 | | | | ||
501 | +-----------------------------+ | ||
502 | | _Action1_ | _Action2_ | <- first group, no TITLE | ||
503 | +-----------------------------+ | ||
504 | | TITLE _Action1_ | _Action2_ | <- second group | ||
505 | +-----------------------------+ | ||
506 | ... | ||
507 | ``` \ No newline at end of file | ||