Skip to content

Commit edee52f

Browse files
matkladmatklad
committed
reorg docs
1 parent e4d0f19 commit edee52f

File tree

2 files changed

+263
-268
lines changed

2 files changed

+263
-268
lines changed

‎docs/dev/README.md

Lines changed: 52 additions & 268 deletions
Original file line numberDiff line numberDiff line change
@@ -50,277 +50,85 @@ We use bors-ng to enforce the [not rocket science](https://graydon2.dreamwidth.o
5050

5151
You can run `cargo xtask install-pre-commit-hook` to install git-hook to run rustfmt on commit.
5252

53-
# Code organization
54-
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
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
59-
documentation.
60-
61-
We have some automation infra in Rust in the `xtask` package. It contains
62-
stuff like formatting checking, code generation and powers `cargo xtask install`.
63-
The latter syntax is achieved with the help of cargo aliases (see `.cargo`
64-
directory).
65-
6653
# Launching rust-analyzer
6754

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
70-
interacting with two processes.
55+
Debugging the language server can be tricky.
56+
LSP is rather chatty, so driving it from the command line is not really feasible, driving it via VS Code requires interacting with two processes.
7157

72-
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
74-
test).
58+
For this reason, the best way to see how rust-analyzer works is to find a relevant test and execute it.
59+
VS Code & Emacs include an action for running a single test.
7560

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.
61+
Launching a VS Code instance with a locally built language server is also possible.
62+
There's **"Run Extension (Debug Build)"** launch configuration for this in VS Code.
7863

79-
In general, I use one of the following workflows for fixing bugs and
80-
implementing features.
64+
In general, I use one of the following workflows for fixing bugs and implementing features:
8165

82-
If the problem concerns only internal parts of rust-analyzer (i.e. I don't need
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
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
87-
Code to sanity check that the thing works as I expect.
66+
If the problem concerns only internal parts of rust-analyzer (i.e. I don't need to touch the `rust-analyzer` crate or TypeScript code), there is a unit-test for it.
67+
So, I use **Rust Analyzer: Run** action in VS Code to run this single test, and then just do printf-driven development/debugging.
68+
As a sanity check after I'm done, I use `cargo xtask install --server` and **Reload Window** action in VS Code to verify that the thing works as I expect.
8869

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
91-
`rust-analyzer` binary from `PATH`. For this, it is important to have the following
92-
in your `settings.json` file:
70+
If the problem concerns only the VS Code extension, I use **Run Installed Extension** launch configuration from `launch.json`.
71+
Notably, this uses the usual `rust-analyzer` binary from `PATH`.
72+
For this, it is important to have the following in your `settings.json` file:
9373
```json
9474
{
9575
"rust-analyzer.serverPath": "rust-analyzer"
9676
}
9777
```
98-
After I am done with the fix, I use `cargo
99-
xtask install --client-code` to try the new extension for real.
100-
101-
If I need to fix something in the `rust-analyzer` crate, I feel sad because it's
102-
on the boundary between the two processes, and working there is slow. I usually
103-
just `cargo xtask install --server` and poke changes from my live environment.
104-
Note that this uses `--release`, which is usually faster overall, because
105-
loading stdlib into debug version of rust-analyzer takes a lot of time. To speed
106-
things up, sometimes I open a temporary hello-world project which has
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
109-
things rust-analyzer needs to do, and makes printf's more useful. Note that you
110-
should only use the `eprint!` family of macros for debugging: stdout is used for LSP
111-
communication, and `print!` would break it.
112-
113-
If I need to fix something simultaneously in the server and in the client, I
114-
feel even more sad. I don't have a specific workflow for this case.
115-
116-
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
118-
performance optimizations, or for bug minimization.
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 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 renaming 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 things to keep an eye on are 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 the `[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 the 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 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.
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-
## Crates.io Dependencies
170-
171-
We try to be very conservative with usage of crates.io dependencies.
172-
Don't use small "helper" crates (exception: `itertools` is allowed).
173-
If there's some general reusable bit of code you need, consider adding it to the `stdx` crate.
174-
175-
## Minimal Tests
176-
177-
Most tests in rust-analyzer start with a snippet of Rust code.
178-
This snippets should be minimal -- if you copy-paste a snippet of real code into the tests, make sure to remove everything which could be removed.
179-
There are many benefits to this:
180-
181-
* less to read or to scroll past
182-
* easier to understand what exactly is tested
183-
* less stuff printed during printf-debugging
184-
* less time to run test
185-
186-
It also makes sense to format snippets more compactly (for example, by placing enum defitions like `enum E { Foo, Bar }` on a single line),
187-
as long as they are still readable.
188-
189-
## Order of Imports
190-
191-
We separate import groups with blank lines
78+
After I am done with the fix, I use `cargo xtask install --client-code` to try the new extension for real.
19279

193-
```rust
194-
mod x;
195-
mod y;
80+
If I need to fix something in the `rust-analyzer` crate, I feel sad because it's on the boundary between the two processes, and working there is slow.
81+
I usually just `cargo xtask install --server` and poke changes from my live environment.
82+
Note that this uses `--release`, which is usually faster overall, because loading stdlib into debug version of rust-analyzer takes a lot of time.
83+
To speed things up, sometimes I open a temporary hello-world project which has `"rust-analyzer.withSysroot": false` in `.code/settings.json`.
84+
This flag causes rust-analyzer to skip loading the sysroot, which greatly reduces the amount of things rust-analyzer needs to do, and makes printf's more useful.
85+
Note that you should only use the `eprint!` family of macros for debugging: stdout is used for LSP communication, and `print!` would break it.
19686

197-
use std::{ ... }
198-
199-
use crate_foo::{ ... }
200-
use crate_bar::{ ... }
201-
202-
use crate::{}
203-
204-
use super::{} // but prefer `use crate::`
205-
```
206-
207-
## Import Style
208-
209-
Items from `hir` and `ast` should be used qualified:
210-
211-
```rust
212-
// Good
213-
use ra_syntax::ast;
214-
215-
fn frobnicate(func: hir::Function, strukt: ast::StructDef) {}
216-
217-
// Not as good
218-
use hir::Function;
219-
use ra_syntax::ast::StructDef;
220-
221-
fn frobnicate(func: Function, strukt: StructDef) {}
222-
```
223-
224-
Avoid local `use MyEnum::*` imports.
225-
226-
Prefer `use crate::foo::bar` to `use super::bar`.
227-
228-
## Order of Items
229-
230-
Optimize for the reader who sees the file for the first time, and wants to get the general idea about what's going on.
231-
People read things from top to bottom, so place most important things first.
232-
233-
Specifically, if all items except one are private, always put the non-private item on top.
234-
235-
Put `struct`s and `enum`s first, functions and impls last.
236-
237-
Do
238-
239-
```rust
240-
// Good
241-
struct Foo {
242-
bars: Vec<Bar>
243-
}
244-
245-
struct Bar;
246-
```
247-
248-
rather than
249-
250-
```rust
251-
// Not as good
252-
struct Bar;
253-
254-
struct Foo {
255-
bars: Vec<Bar>
256-
}
257-
```
87+
If I need to fix something simultaneously in the server and in the client, I feel even more sad.
88+
I don't have a specific workflow for this case.
25889

259-
## Variable Naming
90+
Additionally, I use `cargo run --release -p rust-analyzer -- analysis-stats path/to/some/rust/crate` to run a batch analysis.
91+
This is primarily useful for performance optimizations, or for bug minimization.
26092

261-
We generally use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)).
262-
The default name is a lowercased name of the type: `global_state: GlobalState`.
263-
Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (`db`, `ctx`, `acc`).
264-
The default name for "result of the function" local variable is `res`.
265-
266-
## Collection types
93+
## Parser Tests
26794

268-
We prefer `rustc_hash::FxHashMap` and `rustc_hash::FxHashSet` instead of the ones in `std::collections`.
269-
They use a hasher that's slightly faster and using them consistently will reduce code size by some small amount.
95+
Tests for the parser (`ra_parser`) live in the `ra_syntax` crate (see `test_data` directory).
96+
There are two kinds of tests:
27097

271-
## Preconditions
98+
* Manually written test cases in `parser/ok` and `parser/err`
99+
* "Inline" tests in `parser/inline` (these are generated) from comments in `ra_parser` crate.
272100

273-
Function preconditions should generally be expressed in types and provided by the caller (rather than checked by callee):
101+
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.
102+
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.
274103

275-
```rust
276-
// Good
277-
fn frbonicate(walrus: Walrus) {
278-
...
279-
}
104+
To update test data, run with `UPDATE_EXPECT` variable:
280105

281-
// Not as good
282-
fn frobnicate(walrus: Option<Walrus>) {
283-
let walrus = match walrus {
284-
Some(it) => it,
285-
None => return,
286-
};
287-
...
288-
}
106+
```bash
107+
env UPDATE_EXPECT=1 cargo qt
289108
```
290109

291-
## Premature Pessimization
292-
293-
While we don't specifically optimize code yet, avoid writing code which is slower than it needs to be.
294-
Don't allocate a `Vec` where an iterator would do, don't allocate strings needlessly.
110+
After adding a new inline test you need to run `cargo xtest codegen` and also update the test data as described above.
295111

296-
```rust
297-
// Good
298-
use itertools::Itertools;
112+
## TypeScript Tests
299113

300-
let (first_word, second_word) = match text.split_ascii_whitespace().collect_tuple() {
301-
Some(it) => it,
302-
None => return,
303-
}
114+
If you change files under `editors/code` and would like to run the tests and linter, install npm and run:
304115

305-
// Not as good
306-
let words = text.split_ascii_whitespace().collect::<Vec<_>>();
307-
if words.len() != 2 {
308-
return
309-
}
116+
```bash
117+
cd editors/code
118+
npm ci
119+
npm run lint
310120
```
311121

312-
## Documentation
313-
314-
For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines.
315-
If the line is too long, you want to split the sentence in two :-)
316-
317-
## Commit Style
122+
# Code organization
318123

319-
We don't have specific rules around git history hygiene.
320-
Maintaining clean git history is encouraged, but not enforced.
321-
We use rebase workflow, it's OK to rewrite history during PR review process.
124+
All Rust code lives in the `crates` top-level directory, and is organized as a single Cargo workspace.
125+
The `editors` top-level directory contains code for integrating with editors.
126+
Currently, it contains the plugin for VS Code (in TypeScript).
127+
The `docs` top-level directory contains both developer and user documentation.
322128

323-
Avoid @mentioning people in commit messages and pull request descriptions (they are added to commit message by bors), as such messages create a lot of duplicate notification traffic during rebases.
129+
We have some automation infra in Rust in the `xtask` package.
130+
It contains stuff like formatting checking, code generation and powers `cargo xtask install`.
131+
The latter syntax is achieved with the help of cargo aliases (see `.cargo` directory).
324132

325133
# Architecture Invariants
326134

@@ -355,35 +163,11 @@ The main IDE crate (`ra_ide`) uses "Plain Old Data" for the API.
355163
Rather than talking in definitions and references, it talks in Strings and textual offsets.
356164
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.
357165
The results are 100% Rust specific though.
166+
Shout outs to LSP developers for popularizing the idea that "UI" is a good place to draw a boundary at.
358167

359-
## Parser Tests
360-
361-
Tests for the parser (`ra_parser`) live in the `ra_syntax` crate (see `test_data` directory).
362-
There are two kinds of tests:
363-
364-
* Manually written test cases in `parser/ok` and `parser/err`
365-
* "Inline" tests in `parser/inline` (these are generated) from comments in `ra_parser` crate.
366-
367-
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.
368-
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.
369-
370-
To update test data, run with `UPDATE_EXPECT` variable:
371-
372-
```bash
373-
env UPDATE_EXPECT=1 cargo qt
374-
```
375-
376-
After adding a new inline test you need to run `cargo xtest codegen` and also update the test data as described above.
377-
378-
## TypeScript Tests
379-
380-
If you change files under `editors/code` and would like to run the tests and linter, install npm and run:
168+
# Code Style & Review Process
381169

382-
```bash
383-
cd editors/code
384-
npm ci
385-
npm run lint
386-
```
170+
Do see [./style.md](./style.md).
387171

388172
# Logging
389173

0 commit comments

Comments
 (0)