aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--docs/dev/README.md40
1 files changed, 20 insertions, 20 deletions
diff --git a/docs/dev/README.md b/docs/dev/README.md
index 4cb5dfaa0..1b63d8223 100644
--- a/docs/dev/README.md
+++ b/docs/dev/README.md
@@ -1,6 +1,6 @@
1# Contributing Quick Start 1# Contributing Quick Start
2 2
3Rust Analyzer is just a usual rust project, which is organized as a Cargo 3Rust Analyzer is an ordinary Rust project, which is organized as a Cargo
4workspace, builds on stable and doesn't depend on C libraries. So, just 4workspace, builds on stable and doesn't depend on C libraries. So, just
5 5
6``` 6```
@@ -65,7 +65,7 @@ directory).
65 65
66# Launching rust-analyzer 66# Launching rust-analyzer
67 67
68Debugging language server can be tricky: LSP is rather chatty, so driving it 68Debugging the language server can be tricky: LSP is rather chatty, so driving it
69from the command line is not really feasible, driving it via VS Code requires 69from the command line is not really feasible, driving it via VS Code requires
70interacting with two processes. 70interacting with two processes.
71 71
@@ -73,14 +73,14 @@ For this reason, the best way to see how rust-analyzer works is to find a
73relevant test and execute it (VS Code includes an action for running a single 73relevant test and execute it (VS Code includes an action for running a single
74test). 74test).
75 75
76However, launching a VS Code instance with locally build language server is 76However, launching a VS Code instance with a locally built language server is
77possible. There's **"Run Extension (Debug Build)"** launch configuration for this. 77possible. There's **"Run Extension (Debug Build)"** launch configuration for this.
78 78
79In general, I use one of the following workflows for fixing bugs and 79In general, I use one of the following workflows for fixing bugs and
80implementing features. 80implementing features.
81 81
82If the problem concerns only internal parts of rust-analyzer (i.e. I don't need 82If the problem concerns only internal parts of rust-analyzer (i.e. I don't need
83to touch `rust-analyzer` crate or TypeScript code), there is a unit-test for it. 83to touch the `rust-analyzer` crate or TypeScript code), there is a unit-test for it.
84So, I use **Rust Analyzer: Run** action in VS Code to run this single test, and 84So, I use **Rust Analyzer: Run** action in VS Code to run this single test, and
85then just do printf-driven development/debugging. As a sanity check after I'm 85then just do printf-driven development/debugging. As a sanity check after I'm
86done, I use `cargo xtask install --server` and **Reload Window** action in VS 86done, I use `cargo xtask install --server` and **Reload Window** action in VS
@@ -88,8 +88,8 @@ Code to sanity check that the thing works as I expect.
88 88
89If the problem concerns only the VS Code extension, I use **Run Installed Extension** 89If the problem concerns only the VS Code extension, I use **Run Installed Extension**
90launch configuration from `launch.json`. Notably, this uses the usual 90launch configuration from `launch.json`. Notably, this uses the usual
91`rust-analyzer` binary from `PATH`. For this it is important to have the following 91`rust-analyzer` binary from `PATH`. For this, it is important to have the following
92in `setting.json` file: 92in your `settings.json` file:
93```json 93```json
94{ 94{
95 "rust-analyzer.serverPath": "rust-analyzer" 95 "rust-analyzer.serverPath": "rust-analyzer"
@@ -107,7 +107,7 @@ things up, sometimes I open a temporary hello-world project which has
107`"rust-analyzer.withSysroot": false` in `.code/settings.json`. This flag causes 107`"rust-analyzer.withSysroot": false` in `.code/settings.json`. This flag causes
108rust-analyzer to skip loading the sysroot, which greatly reduces the amount of 108rust-analyzer to skip loading the sysroot, which greatly reduces the amount of
109things rust-analyzer needs to do, and makes printf's more useful. Note that you 109things rust-analyzer needs to do, and makes printf's more useful. Note that you
110should only use `eprint!` family of macros for debugging: stdout is used for LSP 110should only use the `eprint!` family of macros for debugging: stdout is used for LSP
111communication, and `print!` would break it. 111communication, and `print!` would break it.
112 112
113If I need to fix something simultaneously in the server and in the client, I 113If I need to fix something simultaneously in the server and in the client, I
@@ -119,20 +119,20 @@ performance optimizations, or for bug minimization.
119 119
120# Code Style & Review Process 120# Code Style & Review Process
121 121
122Our approach to "clean code" is two fold: 122Our approach to "clean code" is two-fold:
123 123
124* We generally don't block PRs on style changes. 124* We generally don't block PRs on style changes.
125* At the same time, all code in rust-analyzer is constantly refactored. 125* At the same time, all code in rust-analyzer is constantly refactored.
126 126
127It 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. 127It is explicitly OK for a reviewer to flag only some nits in the PR, and then send a follow-up cleanup PR for things which are easier to explain by example, cc-ing the original author.
128Sending small cleanup PRs (like rename a single local variable) is encouraged. 128Sending small cleanup PRs (like renaming a single local variable) is encouraged.
129 129
130## Scale of Changes 130## Scale of Changes
131 131
132Everyone knows that it's better to send small & focused pull requests. 132Everyone knows that it's better to send small & focused pull requests.
133The problem is, sometimes you *have* to, eg, rewrite the whole compiler, and that just doesn't fit into a set of isolated PRs. 133The problem is, sometimes you *have* to, eg, rewrite the whole compiler, and that just doesn't fit into a set of isolated PRs.
134 134
135The main thing too keep an eye on is the boundaries between various components. 135The main things to keep an eye on are the boundaries between various components.
136There are three kinds of changes: 136There are three kinds of changes:
137 137
1381. Internals of a single component are changed. 1381. Internals of a single component are changed.
@@ -144,20 +144,20 @@ There are three kinds of changes:
144 A good example here would be expansion of assist API, for example, to implement lazy assists or assists groups. 144 A good example here would be expansion of assist API, for example, to implement lazy assists or assists groups.
145 145
1463. A new dependency between components is introduced. 1463. 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`. 147 Specifically, you add a `pub use` reexport from another crate or you add a new line to the `[dependencies]` section of `Cargo.toml`.
148 A good example here would be adding reference search capability to the assists crates. 148 A good example here would be adding reference search capability to the assists crates.
149 149
150For the first group, the change is generally merged as long as: 150For the first group, the change is generally merged as long as:
151 151
152* it works for the happy case, 152* it works for the happy case,
153* it has tests, 153* it has tests,
154* it doesn't panic for unhappy case. 154* it doesn't panic for the unhappy case.
155 155
156For the second group, the change would be subjected to quite a bit of scrutiny and iteration. 156For the second group, the change would be subjected to quite a bit of scrutiny and iteration.
157The new API needs to be right (or at least easy to change later). 157The new API needs to be right (or at least easy to change later).
158The actual implementation doesn't matter that much. 158The actual implementation doesn't matter that much.
159It's very important to minimize the amount of changed lines of code for changes of the second kind. 159It's very important to minimize the amount of changed lines of code for changes of the second kind.
160Often, you start doing change of the first kind, only to realise that you need to elevate to a change of the second kind. 160Often, you start doing a change of the first kind, only to realise that you need to elevate to a change of the second kind.
161In this case, we'll probably ask you to split API changes into a separate PR. 161In this case, we'll probably ask you to split API changes into a separate PR.
162 162
163Changes of the third group should be pretty rare, so we don't specify any specific process for them. 163Changes of the third group should be pretty rare, so we don't specify any specific process for them.
@@ -239,7 +239,7 @@ struct Foo {
239## Variable Naming 239## Variable Naming
240 240
241We generally use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)). 241We generally use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)).
242The default name is lowercased named of the type: `global_state: GlobalState`. 242The default name is a lowercased name of the type: `global_state: GlobalState`.
243Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (`db`, `ctx`, `acc`). 243Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (`db`, `ctx`, `acc`).
244The default name for "result of the function" local variable is `res`. 244The default name for "result of the function" local variable is `res`.
245 245
@@ -265,8 +265,8 @@ fn frobnicate(walrus: Option<Walrus>) {
265 265
266## Premature Pessimization 266## Premature Pessimization
267 267
268While we don't specifically optimize code yet, avoid writing the code which is slower than it needs to be. 268While we don't specifically optimize code yet, avoid writing code which is slower than it needs to be.
269Don't allocate a `Vec` were an iterator would do, don't allocate strings needlessly. 269Don't allocate a `Vec` where an iterator would do, don't allocate strings needlessly.
270 270
271```rust 271```rust
272// Good 272// Good
@@ -305,7 +305,7 @@ always obvious from the low-level code.
305## Incomplete syntax trees 305## Incomplete syntax trees
306 306
307Syntax trees are by design incomplete and do not enforce well-formedness. 307Syntax trees are by design incomplete and do not enforce well-formedness.
308If ast method returns an `Option`, it *can* be `None` at runtime, even if this is forbidden by the grammar. 308If an AST method returns an `Option`, it *can* be `None` at runtime, even if this is forbidden by the grammar.
309 309
310## LSP independence 310## LSP independence
311 311
@@ -333,7 +333,7 @@ The results are 100% Rust specific though.
333 333
334## Parser Tests 334## Parser Tests
335 335
336Test for parser (`ra_parser`) live in `ra_syntax` crate (see `test_data` direcotory). 336Tests for the parser (`ra_parser`) live in the `ra_syntax` crate (see `test_data` directory).
337There are two kinds of tests: 337There are two kinds of tests:
338 338
339* Manually written test cases in `parser/ok` and `parser/err` 339* Manually written test cases in `parser/ok` and `parser/err`
@@ -374,7 +374,7 @@ To log all communication between the server and the client, there are two choice
374 [@DJMcNab](https://github.com/DJMcNab) for setting this awesome infra up! 374 [@DJMcNab](https://github.com/DJMcNab) for setting this awesome infra up!
375 375
376 376
377There's also two VS Code commands which might be of interest: 377There are also two VS Code commands which might be of interest:
378 378
379* `Rust Analyzer: Status` shows some memory-usage statistics. To take full 379* `Rust Analyzer: Status` shows some memory-usage statistics. To take full
380 advantage of it, you need to compile rust-analyzer with jemalloc support: 380 advantage of it, you need to compile rust-analyzer with jemalloc support: