5 files changed, 162 insertions, 76 deletions

diff --git a/docs/dev/README.md b/docs/dev/README.md
index 57162a47d..d0e6d29d8 100644
--- a/docs/dev/README.md
+++ b/docs/dev/README.md
@@ -1,7 +1,7 @@
 # Contributing Quick Start
 
-Rust Analyzer is an ordinary Rust project, which is organized as a Cargo
-workspace, builds on stable and doesn't depend on C libraries. So, just
+Rust Analyzer is an ordinary Rust project, which is organized as a Cargo workspace, builds on stable and doesn't depend on C libraries.
+So, just
 
 ```
 $ cargo test
@@ -13,9 +13,8 @@ To learn more about how rust-analyzer works, see [./architecture.md](./architect
 It also explains the high-level layout of the source code.
 Do skim through that document.
 
-We also publish rustdoc docs to pages:
-
-https://rust-analyzer.github.io/rust-analyzer/ide/
+We also publish rustdoc docs to pages: https://rust-analyzer.github.io/rust-analyzer/ide/.
+Note though, that internal documentation is very incomplete.
 
 Various organizational and process issues are discussed in this document.
 
@@ -25,7 +24,7 @@ Rust Analyzer is a part of [RLS-2.0 working
 group](https://github.com/rust-lang/compiler-team/tree/6a769c13656c0a6959ebc09e7b1f7c09b86fb9c0/working-groups/rls-2.0).
 Discussion happens in this Zulip stream:
 
-https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0
+https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Frust-analyzer
 
 # Issue Labels
 
@@ -33,6 +32,8 @@ https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0
   are good issues to get into the project.
 * [E-has-instructions](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-has-instructions)
   issues have links to the code in question and tests.
+* [Broken Window](https://github.com/rust-analyzer/rust-analyzer/issues?q=is:issue+is:open+label:%22Broken+Window%22)
+  are issues which are not critical by themselves, but which should be fixed ASAP regardless, to avoid accumulation of technical debt.
 * [E-easy](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-easy),
   [E-medium](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-medium),
   [E-hard](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-hard),
@@ -49,21 +50,28 @@ https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0
   Also a kind of fun.
   These issues should generally include a link to a Zulip discussion thread.
 
-# CI
+# Code Style & Review Process
+
+Do see [./style.md](./style.md).
 
-We use GitHub Actions for CI. Most of the things, including formatting, are checked by
-`cargo test` so, if `cargo test` passes locally, that's a good sign that CI will
-be green as well. The only exception is that some long-running tests are skipped locally by default.
+# Cookbook
+
+## CI
+
+We use GitHub Actions for CI.
+Most of the things, including formatting, are checked by `cargo test`.
+If `cargo test` passes locally, that's a good sign that CI will be green as well.
+The only exception is that some long-running tests are skipped locally by default.
 Use `env RUN_SLOW_TESTS=1 cargo test` to run the full suite.
 
 We use bors-ng to enforce the [not rocket science](https://graydon2.dreamwidth.org/1597.html) rule.
 
-# Launching rust-analyzer
+## Launching rust-analyzer
 
 Debugging the language server can be tricky.
 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.
 
-For this reason, the best way to see how rust-analyzer works is to find a relevant test and execute it.
+For this reason, the best way to see how rust-analyzer works is to **find a relevant test and execute it**.
 VS Code & Emacs include an action for running a single test.
 
 Launching a VS Code instance with a locally built language server is also possible.
@@ -107,12 +115,7 @@ cd editors/code
 npm ci
 npm run lint
 ```
-
-# Code Style & Review Process
-
-Do see [./style.md](./style.md).
-
-# How to ...
+## How to ...
 
 * ... add an assist? [#7535](https://github.com/rust-analyzer/rust-analyzer/pull/7535)
 * ... add a new protocol extension? [#4569](https://github.com/rust-analyzer/rust-analyzer/pull/4569)
@@ -120,33 +123,30 @@ Do see [./style.md](./style.md).
 * ... add a new completion? [#6964](https://github.com/rust-analyzer/rust-analyzer/pull/6964)
 * ... allow new syntax in the parser? [#7338](https://github.com/rust-analyzer/rust-analyzer/pull/7338)
 
-# Logging
+## Logging
 
-Logging is done by both rust-analyzer and VS Code, so it might be tricky to
-figure out where logs go.
+Logging is done by both rust-analyzer and VS Code, so it might be tricky to figure out where logs go.
 
-Inside rust-analyzer, we use the standard `log` crate for logging, and
-`env_logger` for logging frontend. By default, log goes to stderr, but the
-stderr itself is processed by VS Code.
+Inside rust-analyzer, we use the standard `log` crate for logging, and `env_logger` for logging frontend.
+By default, log goes to stderr, but the stderr itself is processed by VS Code.
+`--log-file <PATH>` CLI argument allows logging to file.
 
-To see stderr in the running VS Code instance, go to the "Output" tab of the
-panel and select `rust-analyzer`. This shows `eprintln!` as well. Note that
-`stdout` is used for the actual protocol, so `println!` will break things.
+To see stderr in the running VS Code instance, go to the "Output" tab of the panel and select `rust-analyzer`.
+This shows `eprintln!` as well.
+Note that `stdout` is used for the actual protocol, so `println!` will break things.
 
 To log all communication between the server and the client, there are two choices:
 
-* you can log on the server side, by running something like
+* You can log on the server side, by running something like
   ```
   env RA_LOG=lsp_server=debug code .
   ```
+* You can log on the client side, by enabling `"rust-analyzer.trace.server": "verbose"` workspace setting.
+  These logs are shown in a separate tab in the output and could be used with LSP inspector.
+  Kudos to [@DJMcNab](https://github.com/DJMcNab) for setting this awesome infra up!
 
-* you can log on the client side, by enabling `"rust-analyzer.trace.server":
-  "verbose"` workspace setting. These logs are shown in a separate tab in the
-  output and could be used with LSP inspector. Kudos to
-  [@DJMcNab](https://github.com/DJMcNab) for setting this awesome infra up!
 
-
-There are also two VS Code commands which might be of interest:
+There are also several VS Code commands which might be of interest:
 
 * `Rust Analyzer: Status` shows some memory-usage statistics.
 
@@ -164,7 +164,7 @@ There are also two VS Code commands which might be of interest:
 
   ![demo](https://user-images.githubusercontent.com/36276403/78225773-6636a480-74d3-11ea-9d9f-1c9d42da03b0.png)
 
-# Profiling
+## Profiling
 
 We have a built-in hierarchical profiler, you can enable it by using `RA_PROFILE` env-var:
 
@@ -192,7 +192,9 @@ $ cargo run --release -p rust-analyzer -- analysis-bench ../chalk/ --highlight .
 $ cargo run --release -p rust-analyzer -- analysis-bench ../chalk/ --complete ../chalk/chalk-engine/src/logic.rs:94:0
 ```
 
-# Release Process
+Look for `fn benchmark_xxx` tests for a quick way to reproduce performance problems.
+
+## Release Process
 
 Release process is handled by `release`, `dist` and `promote` xtasks, `release` being the main one.
 
@@ -229,16 +231,18 @@ Make sure to remove the new changelog post created when running `cargo xtask rel
 We release "nightly" every night automatically and promote the latest nightly to "stable" manually, every week.
 We don't do "patch" releases, unless something truly egregious comes up.
 
-# Permissions
+## Permissions
 
 There are three sets of people with extra permissions:
 
-* rust-analyzer GitHub organization **admins** (which include current t-compiler leads).
+* rust-analyzer GitHub organization [**admins**](https://github.com/orgs/rust-analyzer/people?query=role:owner) (which include current t-compiler leads).
   Admins have full access to the org.
-* **review** team in the organization.
+* [**review**](https://github.com/orgs/rust-analyzer/teams/review) team in the organization.
   Reviewers have `r+` access to all of organization's repositories and publish rights on crates.io.
   They also have direct commit access, but all changes should via bors queue.
   It's ok to self-approve if you think you know what you are doing!
   bors should automatically sync the permissions.
-* **triage** team in the organization.
+* [**triage**](https://github.com/orgs/rust-analyzer/teams/triage) team in the organization.
   This team can label and close issues.
+
+Note that at the time being you need to be a member of the org yourself to view the links.
diff --git a/docs/dev/architecture.md b/docs/dev/architecture.md
index fb991133a..39edf9e19 100644
--- a/docs/dev/architecture.md
+++ b/docs/dev/architecture.md
@@ -42,7 +42,7 @@ The underlying engine makes sure that model is computed lazily (on-demand) and c
 ## Entry Points
 
 `crates/rust-analyzer/src/bin/main.rs` contains the main function which spawns LSP.
-This is *the* entry point, but it front-loads a lot of complexity, so its fine to just skim through it.
+This is *the* entry point, but it front-loads a lot of complexity, so it's fine to just skim through it.
 
 `crates/rust-analyzer/src/handlers.rs` implements all LSP requests and is a great place to start if you are already familiar with LSP.
 
@@ -67,7 +67,7 @@ They are handled by Rust code in the xtask directory.
 
 VS Code plugin.
 
-### `libs/`
+### `lib/`
 
 rust-analyzer independent libraries which we publish to crates.io.
 It's not heavily utilized at the moment.
@@ -139,7 +139,8 @@ If an AST method returns an `Option`, it *can* be `None` at runtime, even if thi
 ### `crates/base_db`
 
 We use the [salsa](https://github.com/salsa-rs/salsa) crate for incremental and on-demand computation.
-Roughly, you can think of salsa as a key-value store, but it can also compute derived values using specified functions. The `base_db` crate provides basic infrastructure for interacting with salsa.
+Roughly, you can think of salsa as a key-value store, but it can also compute derived values using specified functions.
+The `base_db` crate provides basic infrastructure for interacting with salsa.
 Crucially, it defines most of the "input" queries: facts supplied by the client of the analyzer.
 Reading the docs of the `base_db::input` module should be useful: everything else is strictly derived from those inputs.
 
@@ -221,7 +222,7 @@ Internally, `ide` is split across several crates. `ide_assists`, `ide_completion
 The `ide` contains a public API/façade, as well as implementation for a plethora of smaller features.
 
 **Architecture Invariant:** `ide` crate strives to provide a _perfect_ API.
-Although at the moment it has only one consumer, the LSP server, LSP *does not* influence it's API design.
+Although at the moment it has only one consumer, the LSP server, LSP *does not* influence its API design.
 Instead, we keep in mind a hypothetical _ideal_ client -- an IDE tailored specifically for rust, every nook and cranny of which is packed with Rust-specific goodies.
 
 ### `crates/rust-analyzer`
@@ -307,7 +308,7 @@ This sections talks about the things which are everywhere and nowhere in particu
 
 ### Code generation
 
-Some of the components of this repository are generated through automatic processes.
+Some components in this repository are generated through automatic processes.
 Generated code is updated automatically on `cargo test`.
 Generated code is generally committed to the git repository.
 
@@ -389,7 +390,7 @@ fn spam() {
 ```
 
 To specify input data, we use a single string literal in a special format, which can describe a set of rust files.
-See the `Fixture` type.
+See the `Fixture` its module for fixture examples and documentation.
 
 **Architecture Invariant:** all code invariants are tested by `#[test]` tests.
 There's no additional checks in CI, formatting and tidy tests are run with `cargo test`.
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index 8a6f9f06e..a4d92242b 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -1,8 +1,8 @@
 <!---
-lsp_ext.rs hash: e8a7502bd2b2c2f5
+lsp_ext.rs hash: b19ddc3ab8767af9
 
 If you need to change the above hash to make the test pass, please check if you
-need to adjust this doc as well and ping this  issue:
+need to adjust this doc as well and ping this issue:
 
   https://github.com/rust-analyzer/rust-analyzer/issues/4604
 
@@ -51,8 +51,8 @@ interface SnippetTextEdit extends TextEdit {
 
 ```typescript
 export interface TextDocumentEdit {
-        textDocument: OptionalVersionedTextDocumentIdentifier;
-        edits: (TextEdit | SnippetTextEdit)[];
+    textDocument: OptionalVersionedTextDocumentIdentifier;
+    edits: (TextEdit | SnippetTextEdit)[];
 }
 ```
 
@@ -145,9 +145,9 @@ mod foo;
 ### Unresolved Question
 
 * An alternative would be to use a more general "gotoSuper" request, which would work for super methods, super classes and super modules.
-  This is the approach IntelliJ Rust is takeing.
+  This is the approach IntelliJ Rust is taking.
   However, experience shows that super module (which generally has a feeling of navigation between files) should be separate.
-  If you want super module, but the cursor happens to be inside an overriden function, the behavior with single "gotoSuper" request is surprising.
+  If you want super module, but the cursor happens to be inside an overridden function, the behavior with single "gotoSuper" request is surprising.
 
 ## Join Lines
 
@@ -193,7 +193,7 @@ fn main() {
 ### Unresolved Question
 
 * What is the position of the cursor after `joinLines`?
-  Currently this is left to editor's discretion, but it might be useful to specify on the server via snippets.
+  Currently, this is left to editor's discretion, but it might be useful to specify on the server via snippets.
   However, it then becomes unclear how it works with multi cursor.
 
 ## On Enter
@@ -330,7 +330,7 @@ Moreover, it would be cool if editors didn't need to implement even basic langua
 
 ### Unresolved Question
 
-* Should we return a a nested brace structure, to allow paredit-like actions of jump *out* of the current brace pair?
+* Should we return a nested brace structure, to allow paredit-like actions of jump *out* of the current brace pair?
   This is how `SelectionRange` request works.
 * Alternatively, should we perhaps flag certain `SelectionRange`s as being brace pairs?
 
@@ -419,23 +419,37 @@ Returns internal status message, mostly for debugging purposes.
 
 Reloads project information (that is, re-executes `cargo metadata`).
 
-## Status Notification
+## Server Status
 
-**Experimental Client Capability:** `{ "statusNotification": boolean }`
+**Experimental Client Capability:** `{ "serverStatus": boolean }`
 
-**Method:** `rust-analyzer/status`
+**Method:** `experimental/serverStatus`
 
 **Notification:**
 
 ```typescript
-interface StatusParams {
-    status: "loading" | "readyPartial" | "ready" | "invalid" | "needsReload",
+interface ServerStatusParams {
+    /// `ok` means that the server is completely functional.
+    ///
+    /// `warning` means that the server is partially functional.
+    /// It can server requests, but some results might be wrong due to,
+    /// for example, some missing dependencies.
+    ///
+    /// `error` means that the server is not functional. For example,
+    /// there's a fatal build configuration problem.
+    health: "ok" | "warning" | "error",
+    /// Is there any pending background work which might change the status?
+    /// For example, are dependencies being downloaded?
+    quiescent: bool,
+    /// Explanatory message to show on hover.
+    message?: string,
 }
 ```
 
 This notification is sent from server to client.
-The client can use it to display persistent status to the user (in modline).
-For `needsReload` state, the client can provide a context-menu action to run `rust-analyzer/reloadWorkspace` request.
+The client can use it to display *persistent* status to the user (in modline).
+It is similar to the `showMessage`, but is intended for stares rather than point-in-time events.
+
 
 ## Syntax Tree
 
@@ -497,7 +511,7 @@ Expands macro call at a given position.
 This request is sent from client to server to render "inlay hints" -- virtual text inserted into editor to show things like inferred types.
 Generally, the client should re-query inlay hints after every modification.
 Note that we plan to move this request to `experimental/inlayHints`, as it is not really Rust-specific, but the current API is not necessary the right one.
-Upstream issue: https://github.com/microsoft/language-server-protocol/issues/956
+Upstream issues: https://github.com/microsoft/language-server-protocol/issues/956 , https://github.com/rust-analyzer/rust-analyzer/issues/2797
 
 **Request:**
 
@@ -602,12 +616,11 @@ interface TestInfo {
 
 This request is sent from client to server to move item under cursor or selection in some direction.
 
-**Method:** `experimental/moveItemUp`
-**Method:** `experimental/moveItemDown`
+**Method:** `experimental/moveItem`
 
 **Request:** `MoveItemParams`
 
-**Response:** `TextDocumentEdit | null`
+**Response:** `SnippetTextEdit[]`
 
 ```typescript
 export interface MoveItemParams {
diff --git a/docs/dev/style.md b/docs/dev/style.md
index e4a1672ca..078c478d4 100644
--- a/docs/dev/style.md
+++ b/docs/dev/style.md
@@ -53,11 +53,11 @@ https://www.tedinski.com/2018/02/06/system-boundaries.html
 ## Crates.io Dependencies
 
 We try to be very conservative with usage of crates.io dependencies.
-Don't use small "helper" crates (exception: `itertools` is allowed).
+Don't use small "helper" crates (exception: `itertools` and `either` are allowed).
 If there's some general reusable bit of code you need, consider adding it to the `stdx` crate.
+A useful exercise is to read Cargo.lock and see if some *transitive* dependencies do not make sense for rust-analyzer.
 
-**Rationale:** keep compile times low, create ecosystem pressure for faster
-compiles, reduce the number of things which might break.
+**Rationale:** keep compile times low, create ecosystem pressure for faster compiles, reduce the number of things which might break.
 
 ## Commit Style
 
@@ -152,6 +152,16 @@ Do not reuse marks between several tests.
 
 **Rationale:** marks provide an easy way to find the canonical test for each bit of code.
 This makes it much easier to understand.
+More than one mark per test / code branch doesn't add significantly to understanding.
+
+## `#[should_panic]`
+
+Do not use `#[should_panic]` tests.
+Instead, explicitly check for `None`, `Err`, etc.
+
+**Rationale:** `#[should_panic]` is a tool for library authors, to makes sure that API does not fail silently, when misused.
+`rust-analyzer` is not a library, we don't need to test for API misuse, and we have to handle any user input without panics.
+Panic messages in the logs from the `#[should_panic]` tests are confusing.
 
 ## Function Preconditions
 
@@ -330,7 +340,7 @@ When implementing `do_thing`, it might be very useful to create a context object
 
 ```rust
 pub fn do_thing(arg1: Arg1, arg2: Arg2) -> Res {
-    let mut ctx = Ctx { arg1, arg2 }
+    let mut ctx = Ctx { arg1, arg2 };
     ctx.run()
 }
 
@@ -586,7 +596,7 @@ use super::{}
 
 **Rationale:** consistency.
 Reading order is important for new contributors.
-Grouping by crate allows to spot unwanted dependencies easier.
+Grouping by crate allows spotting unwanted dependencies easier.
 
 ## Import Style
 
@@ -779,7 +789,7 @@ assert!(x < y);
 assert!(x > 0);
 
 // BAD
-assert!(x >= lo && x <= hi>);
+assert!(x >= lo && x <= hi);
 assert!(r1 < l2 || l1 > r2);
 assert!(y > x);
 assert!(0 > x);
@@ -806,9 +816,67 @@ if let Some(expected_type) = ctx.expected_type.as_ref() {
 }
 ```
 
-**Rational:** `match` is almost always more compact.
+**Rationale:** `match` is almost always more compact.
 The `else` branch can get a more precise pattern: `None` or `Err(_)` instead of `_`.
 
+## Helper Functions
+
+Avoid creating singe-use helper functions:
+
+```rust
+// GOOD
+let buf = {
+    let mut buf = get_empty_buf(&mut arena);
+    buf.add_item(item);
+    buf
+};
+
+// BAD
+
+let buf = prepare_buf(&mut arena, item);
+
+...
+
+fn prepare_buf(arena: &mut Arena, item: Item) -> ItemBuf {
+    let mut res = get_empty_buf(&mut arena);
+    res.add_item(item);
+    res
+}
+```
+
+Exception: if you want to make use of `return` or `?`.
+
+**Rationale:** single-use functions change frequently, adding or removing parameters adds churn.
+A block serves just as well to delineate a bit of logic, but has access to all the context.
+Re-using originally single-purpose function often leads to bad coupling.
+
+## Helper Variables
+
+Introduce helper variables freely, especially for multiline conditions:
+
+```rust
+// GOOD
+let rustfmt_not_installed =
+    captured_stderr.contains("not installed") || captured_stderr.contains("not available");
+
+match output.status.code() {
+    Some(1) if !rustfmt_not_installed => Ok(None),
+    _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
+};
+
+// BAD
+match output.status.code() {
+    Some(1)
+        if !captured_stderr.contains("not installed")
+           && !captured_stderr.contains("not available") => Ok(None),
+    _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
+};
+```
+
+**Rationale:** like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context.
+Extra variables help during debugging, they make it easy to print/view important intermediate results.
+Giving a name to a condition in `if` expression often improves clarity and leads to a nicer formatted code.
+
 ## Token names
 
 Use `T![foo]` instead of `SyntaxKind::FOO_KW`.
diff --git a/docs/dev/syntax.md b/docs/dev/syntax.md
index 737cc7a72..f7a0c09fc 100644
--- a/docs/dev/syntax.md
+++ b/docs/dev/syntax.md
@@ -145,7 +145,7 @@ Another alternative (used by swift and roslyn) is to explicitly divide the set o
 
 ```rust
 struct Token {
-    kind: NonTriviaTokenKind
+    kind: NonTriviaTokenKind,
     text: String,
     leading_trivia: Vec<TriviaToken>,
     trailing_trivia: Vec<TriviaToken>,
@@ -240,7 +240,7 @@ impl SyntaxNode {
             let child_offset = offset;
             offset += green_child.text_len;
             Arc::new(SyntaxData {
-                offset: child_offset;
+                offset: child_offset,
                 parent: Some(Arc::clone(self)),
                 green: Arc::clone(green_child),
             })
@@ -249,7 +249,7 @@ impl SyntaxNode {
 }
 
 impl PartialEq for SyntaxNode {
-    fn eq(&self, other: &SyntaxNode) {
+    fn eq(&self, other: &SyntaxNode) -> bool {
         self.offset == other.offset
             && Arc::ptr_eq(&self.green, &other.green)
     }
@@ -273,7 +273,7 @@ This is OK because trees traversals mostly (always, in case of rust-analyzer) ru
 The other thread can restore the `SyntaxNode` by traversing from the root green node and looking for a node with specified range.
 You can also use the similar trick to store a `SyntaxNode`.
 That is, a data structure that holds a `(GreenNode, Range<usize>)` will be `Sync`.
-However rust-analyzer goes even further.
+However, rust-analyzer goes even further.
 It treats trees as semi-transient and instead of storing a `GreenNode`, it generally stores just the id of the file from which the tree originated: `(FileId, Range<usize>)`.
 The `SyntaxNode` is the restored by reparsing the file and traversing it from root.
 With this trick, rust-analyzer holds only a small amount of trees in memory at the same time, which reduces memory usage.