5 files changed, 122 insertions, 45 deletions

diff --git a/docs/dev/README.md b/docs/dev/README.md
index eab21a765..16b23adc6 100644
--- a/docs/dev/README.md
+++ b/docs/dev/README.md
@@ -24,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
 
@@ -32,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),
@@ -206,20 +208,26 @@ Release process is handled by `release`, `dist` and `promote` xtasks, `release`
 
 Additionally, it assumes that remote for `rust-analyzer` is called `upstream` (I use `origin` to point to my fork).
 
+`release` calls the GitHub API calls to scrape pull request comments and categorize them in the changelog.
+This step uses the `curl` and `jq` applications, which need to be available in `PATH`.
+Finally, you need to obtain a GitHub personal access token and set the `GITHUB_TOKEN` environment variable.
+
 Release steps:
 
-1. Inside rust-analyzer, run `cargo xtask release`. This will:
+1. Set the `GITHUB_TOKEN` environment variable.
+2. Inside rust-analyzer, run `cargo xtask release`. This will:
    * checkout the `release` branch
    * reset it to `upstream/nightly`
    * push it to `upstream`. This triggers GitHub Actions which:
      * runs `cargo xtask dist` to package binaries and VS Code extension
      * makes a GitHub release
      * pushes VS Code extension to the marketplace
-   * create new changelog in `rust-analyzer.github.io`
-2. While the release is in progress, fill in the changelog
-3. Commit & push the changelog
-4. Tweet
-5. Inside `rust-analyzer`, run `cargo xtask promote` -- this will create a PR to rust-lang/rust updating rust-analyzer's submodule.
+   * call the GitHub API for PR details
+   * create a new changelog in `rust-analyzer.github.io`
+3. While the release is in progress, fill in the changelog
+4. Commit & push the changelog
+5. Tweet
+6. Inside `rust-analyzer`, run `cargo xtask promote` -- this will create a PR to rust-lang/rust updating rust-analyzer's submodule.
    Self-approve the PR.
 
 If the GitHub Actions release fails because of a transient problem like a timeout, you can re-run the job from the Actions console.
@@ -227,18 +235,27 @@ If it fails because of something that needs to be fixed, remove the release tag
 Make sure to remove the new changelog post created when running `cargo xtask release` a second time.
 
 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.
+To do a patch release, cherry-pick the fix on top of the current `release` branch and push the branch.
+There's no need to write a changelog for a patch release, it's OK to include the notes about the fix into the next weekly one.
+Note: we tag releases by dates, releasing a patch release on the same day should work (by overwriting a tag), but I am not 100% sure.
 
 ## 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.
+  Feel free to request a review or assign any PR to a reviewer with the relevant expertise to bring the work to their attention.
+  Don't feel pressured to review assigned PRs though.
+  If you don't feel like reviewing for whatever reason, someone else will pick the review up!
+* [**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..f0f981802 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: 28a9d5a24b7ca396
 
 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
 
@@ -46,13 +46,14 @@ If this capability is set, `WorkspaceEdit`s returned from `codeAction` requests
 ```typescript
 interface SnippetTextEdit extends TextEdit {
     insertTextFormat?: InsertTextFormat;
+    annotationId?: ChangeAnnotationIdentifier;
 }
 ```
 
 ```typescript
 export interface TextDocumentEdit {
-        textDocument: OptionalVersionedTextDocumentIdentifier;
-        edits: (TextEdit | SnippetTextEdit)[];
+    textDocument: OptionalVersionedTextDocumentIdentifier;
+    edits: (TextEdit | SnippetTextEdit)[];
 }
 ```
 
@@ -145,9 +146,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 +194,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 +331,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 +420,42 @@ 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:** `{ "serverStatusNotification": 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 answer correctly to most 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. The server might
+    /// still give correct answers to simple requests, but most results
+    /// will be incomplete or wrong.
+    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.
+
+Note that this functionality is intended primarily to inform the end user about the state of the server.
+In particular, it's valid for the client to completely ignore this extension.
+Clients are discouraged from but are allowed to use the `health` status to decide if it's worth sending a request to the server.
 
 ## Syntax Tree
 
@@ -497,7 +517,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 +622,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 c594946be..6ab60b50e 100644
--- a/docs/dev/style.md
+++ b/docs/dev/style.md
@@ -53,9 +53,9 @@ 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 of the *transitive* dependencies do not make sense for rust-analyzer.
+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.
 
@@ -83,8 +83,19 @@ This makes it easier to prepare a changelog.
 
 If the change adds a new user-visible functionality, consider recording a GIF with [peek](https://github.com/phw/peek) and pasting it into the PR description.
 
+To make writing the release notes easier, you can mark a pull request as a feature, fix, internal change, or minor.
+Minor changes are excluded from the release notes, while the other types are distributed in their corresponding sections.
+There are two ways to mark this:
+
+* use a `feat: `, `feature: `, `fix: `, `internal: ` or `minor: ` prefix in the PR title
+* write `changelog [feature|fix|internal|skip] [description]` in a comment or in the PR description; the description is optional, and will replace the title if included.
+
+These comments don't have to be added by the PR author.
+Editing a comment or the PR description or title is also fine, as long as it happens before the release.
+
 **Rationale:** clean history is potentially useful, but rarely used.
 But many users read changelogs.
+Including a description and GIF suitable for the changelog means less work for the maintainers on the release day.
 
 ## Clippy
 
@@ -152,6 +163,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 +351,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 +607,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 +800,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);
@@ -842,7 +863,26 @@ Re-using originally single-purpose function often leads to bad coupling.
 
 ## Helper Variables
 
-Introduce helper variables freely, especially for multiline conditions.
+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.
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.