diff options
-rw-r--r-- | docs/dev/README.md | 40 |
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 | ||
3 | Rust Analyzer is just a usual rust project, which is organized as a Cargo | 3 | Rust Analyzer is an ordinary Rust project, which is organized as a Cargo |
4 | workspace, builds on stable and doesn't depend on C libraries. So, just | 4 | workspace, 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 | ||
68 | Debugging language server can be tricky: LSP is rather chatty, so driving it | 68 | Debugging the language server can be tricky: LSP is rather chatty, so driving it |
69 | from the command line is not really feasible, driving it via VS Code requires | 69 | from the command line is not really feasible, driving it via VS Code requires |
70 | interacting with two processes. | 70 | interacting 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 | |||
73 | relevant test and execute it (VS Code includes an action for running a single | 73 | relevant test and execute it (VS Code includes an action for running a single |
74 | test). | 74 | test). |
75 | 75 | ||
76 | However, launching a VS Code instance with locally build language server is | 76 | However, launching a VS Code instance with a locally built language server is |
77 | possible. There's **"Run Extension (Debug Build)"** launch configuration for this. | 77 | possible. There's **"Run Extension (Debug Build)"** launch configuration for this. |
78 | 78 | ||
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 (i.e. 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 the `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 |
@@ -88,8 +88,8 @@ Code to sanity check that the thing works as I expect. | |||
88 | 88 | ||
89 | If the problem concerns only the VS Code extension, I use **Run Installed Extension** | 89 | If the problem concerns only the VS Code extension, I use **Run Installed Extension** |
90 | launch configuration from `launch.json`. Notably, this uses the usual | 90 | launch 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 |
92 | in `setting.json` file: | 92 | in 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 |
108 | rust-analyzer to skip loading the sysroot, which greatly reduces the amount of | 108 | rust-analyzer to skip loading the sysroot, which greatly reduces the amount of |
109 | things rust-analyzer needs to do, and makes printf's more useful. Note that you | 109 | things rust-analyzer needs to do, and makes printf's more useful. Note that you |
110 | should only use `eprint!` family of macros for debugging: stdout is used for LSP | 110 | should only use the `eprint!` family of macros for debugging: stdout is used for LSP |
111 | communication, and `print!` would break it. | 111 | communication, and `print!` would break it. |
112 | 112 | ||
113 | If I need to fix something simultaneously in the server and in the client, I | 113 | If 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 | ||
122 | Our approach to "clean code" is two fold: | 122 | Our 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 | ||
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. | 127 | It 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. |
128 | Sending small cleanup PRs (like rename a single local variable) is encouraged. | 128 | Sending small cleanup PRs (like renaming a single local variable) is encouraged. |
129 | 129 | ||
130 | ## Scale of Changes | 130 | ## Scale of Changes |
131 | 131 | ||
132 | Everyone knows that it's better to send small & focused pull requests. | 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. | 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 | 134 | ||
135 | The main thing too keep an eye on is the boundaries between various components. | 135 | The main things to keep an eye on are the boundaries between various components. |
136 | There are three kinds of changes: | 136 | There are three kinds of changes: |
137 | 137 | ||
138 | 1. Internals of a single component are changed. | 138 | 1. 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 | ||
146 | 3. A new dependency between components is introduced. | 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`. | 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 | ||
150 | For the first group, the change is generally merged as long as: | 150 | For 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 | ||
156 | For the second group, the change would be subjected to quite a bit of scrutiny and iteration. | 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). | 157 | The new API needs to be right (or at least easy to change later). |
158 | The actual implementation doesn't matter that much. | 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. | 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. | 160 | Often, you start doing a 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. | 161 | In this case, we'll probably ask you to split API changes into a separate PR. |
162 | 162 | ||
163 | Changes of the third group should be pretty rare, so we don't specify any specific process for them. | 163 | Changes 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 | ||
241 | We generally use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)). | 241 | We generally use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)). |
242 | The default name is lowercased named of the type: `global_state: GlobalState`. | 242 | The default name is a lowercased name of the type: `global_state: GlobalState`. |
243 | Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (`db`, `ctx`, `acc`). | 243 | Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (`db`, `ctx`, `acc`). |
244 | The default name for "result of the function" local variable is `res`. | 244 | The 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 | ||
268 | While we don't specifically optimize code yet, avoid writing the code which is slower than it needs to be. | 268 | While we don't specifically optimize code yet, avoid writing code which is slower than it needs to be. |
269 | Don't allocate a `Vec` were an iterator would do, don't allocate strings needlessly. | 269 | Don'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 | ||
307 | Syntax trees are by design incomplete and do not enforce well-formedness. | 307 | Syntax trees are by design incomplete and do not enforce well-formedness. |
308 | If ast method returns an `Option`, it *can* be `None` at runtime, even if this is forbidden by the grammar. | 308 | If 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 | ||
336 | Test for parser (`ra_parser`) live in `ra_syntax` crate (see `test_data` direcotory). | 336 | Tests for the parser (`ra_parser`) live in the `ra_syntax` crate (see `test_data` directory). |
337 | There are two kinds of tests: | 337 | There 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 | ||
377 | There's also two VS Code commands which might be of interest: | 377 | There 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: |