Merge branch 'master' into keyword_completion

# Conflicts:
#	docs/user/generated_features.adoc

2 files changed, 299 insertions, 32 deletions

diff --git a/docs/dev/README.md b/docs/dev/README.md
index 65cc9fc12..ef5ffbf59 100644
--- a/docs/dev/README.md
+++ b/docs/dev/README.md
@@ -30,7 +30,7 @@ https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0
 
 * [good-first-issue](https://github.com/rust-analyzer/rust-analyzer/labels/good%20first%20issue)
   are good issues to get into the project.
-* [E-mentor](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-mentor)
+* [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.
 * [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),
@@ -55,7 +55,7 @@ You can run `cargo xtask install-pre-commit-hook` to install git-hook to run rus
 All Rust code lives in the `crates` top-level directory, and is organized as a
 single Cargo workspace. The `editors` top-level directory contains code for
 integrating with editors. Currently, it contains the plugin for VS Code (in
-typescript). The `docs` top-level directory contains both developer and user
+TypeScript). The `docs` top-level directory contains both developer and user
 documentation.
 
 We have some automation infra in Rust in the `xtask` package. It contains
@@ -79,8 +79,8 @@ possible. There's **"Run Extension (Debug Build)"** launch configuration for thi
 In general, I use one of the following workflows for fixing bugs and
 implementing features.
 
-If the problem concerns only internal parts of rust-analyzer (ie, I don't need
-to touch `rust-analyzer` crate or typescript code), there is a unit-test for it.
+If the problem concerns only internal parts of rust-analyzer (i.e. I don't need
+to touch `rust-analyzer` crate or TypeScript code), there is a unit-test for it.
 So, I use **Rust Analyzer: Run** action in VS Code to run this single test, and
 then just do printf-driven development/debugging. As a sanity check after I'm
 done, I use `cargo xtask install --server` and **Reload Window** action in VS
@@ -117,6 +117,203 @@ Additionally, I use `cargo run --release -p rust-analyzer -- analysis-stats
 path/to/some/rust/crate` to run a batch analysis. This is primarily useful for
 performance optimizations, or for bug minimization.
 
+# Code Style & Review Process
+
+Our approach to "clean code" is two fold:
+
+* We generally don't block PRs on style changes.
+* At the same time, all code in rust-analyzer is constantly refactored.
+
+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.
+Sending small cleanup PRs (like rename a single local variable) is encouraged.
+
+## Scale of Changes
+
+Everyone knows that it's better to send small & focused pull requests.
+The problem is, sometimes you *have* to, eg, rewrite the whole compiler, and that just doesn't fit into a set of isolated PRs.
+
+The main thing too keep an eye on is the boundaries between various components.
+There are three kinds of changes:
+
+1. Internals of a single component are changed.
+   Specifically, you don't change any `pub` items.
+   A good example here would be an addition of a new assist.
+
+2. API of a component is expanded.
+   Specifically, you add a new `pub` function which wasn't there before.
+   A good example here would be expansion of assist API, for example, to implement lazy assists or assists groups.
+
+3. A new dependency between components is introduced.
+   Specifically, you add a `pub use` reexport from another crate or you add a new line to `[dependencies]` section of `Cargo.toml`.
+   A good example here would be adding reference search capability to the assists crates.
+
+For the first group, the change is generally merged as long as:
+
+* it works for the happy case,
+* it has tests,
+* it doesn't panic for unhappy case.
+
+For the second group, the change would be subjected to quite a bit of scrutiny and iteration.
+The new API needs to be right (or at least easy to change later).
+The actual implementation doesn't matter that much.
+It's very important to minimize the amount of changed lines of code for changes of the second kind.
+Often, you start doing change of the first kind, only to realise that you need to elevate to a change of the second kind.
+In this case, we'll probably ask you to split API changes into a separate PR.
+
+Changes of the third group should be pretty rare, so we don't specify any specific process for them.
+That said, adding an innocent-looking `pub use` is a very simple way to break encapsulation, keep an eye on it!
+
+Note: if you enjoyed this abstract hand-waving about boundaries, you might appreciate
+https://www.tedinski.com/2018/02/06/system-boundaries.html
+
+## Order of Imports
+
+We separate import groups with blank lines
+
+```rust
+mod x;
+mod y;
+
+use std::{ ... }
+
+use crate_foo::{ ... }
+use crate_bar::{ ... }
+
+use crate::{}
+
+use super::{} // but prefer `use crate::`
+```
+
+## Import Style
+
+Items from `hir` and `ast` should be used qualified:
+
+```rust
+// Good
+use ra_syntax::ast;
+
+fn frobnicate(func: hir::Function, strukt: ast::StructDef) {}
+
+// Not as good
+use hir::Function;
+use ra_syntax::ast::StructDef;
+
+fn frobnicate(func: Function, strukt: StructDef) {}
+```
+
+Avoid local `use MyEnum::*` imports.
+
+Prefer `use crate::foo::bar` to `use super::bar`.
+
+## Order of Items
+
+Optimize for the reader who sees the file for the first time, and wants to get the general idea about what's going on.
+People read things from top to bottom, so place most important things first.
+
+Specifically, if all items except one are private, always put the non-private item on top.
+
+Put `struct`s and `enum`s first, functions and impls last.
+
+Do
+
+```rust
+// Good
+struct Foo {
+  bars: Vec<Bar>
+}
+
+struct Bar;
+```
+
+rather than
+
+```rust
+// Not as good
+struct Bar;
+
+struct Foo {
+  bars: Vec<Bar>
+}
+```
+
+## Documentation
+
+For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines.
+If the line is too long, you want to split the sentence in two :-)
+
+## Preconditions
+
+Function preconditions should generally be expressed in types and provided by the caller (rather than checked by callee):
+
+```rust
+// Good
+fn frbonicate(walrus: Walrus) {
+  ...
+}
+
+// Not as good
+fn frobnicate(walrus: Option<Walrus>) {
+  let walrus = match walrus {
+    Some(it) => it,
+    None => return,
+  };
+  ...
+}
+```
+
+## Commit Style
+
+We don't have specific rules around git history hygiene.
+Maintaining clean git history is encouraged, but not enforced.
+We use rebase workflow, it's OK to rewrite history during PR review process.
+
+Avoid @mentioning people in commit messages, as such messages create a lot of duplicate notification traffic during rebases.
+
+# Architecture Invariants
+
+This section tries to document high-level design constraints, which are not
+always obvious from the low-level code.
+
+## Incomplete syntax trees
+
+Syntax trees are by design incomplete and do not enforce well-formedness.
+If ast method returns an `Option`, it *can* be `None` at runtime, even if this is forbidden by the grammar.
+
+## LSP independence
+
+rust-analyzer is independent from LSP.
+It provides features for a hypothetical perfect Rust-specific IDE client.
+Internal representations are lowered to LSP in the `rust-analyzer` crate (the only crate which is allowed to use LSP types).
+
+## IDE/Compiler split
+
+There's a semi-hard split between "compiler" and "IDE", at the `ra_hir` crate.
+Compiler derives new facts about source code.
+It explicitly acknowledges that not all info is available (i.e. you can't look at types during name resolution).
+
+IDE assumes that all information is available at all times.
+
+IDE should use only types from `ra_hir`, and should not depend on the underling compiler types.
+`ra_hir` is a facade.
+
+## IDE API
+
+The main IDE crate (`ra_ide`) uses "Plain Old Data" for the API.
+Rather than talking in definitions and references, it talks in Strings and textual offsets.
+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.
+The results are 100% Rust specific though.
+
+## Parser Tests
+
+Test for parser (`ra_parser`) live in `ra_syntax` crate (see `test_data` direcotory).
+There are two kinds of tests:
+
+* Manually written test cases in `parser/ok` and `parser/err`
+* "Inline" tests in `parser/inline` (these are generated) from comments in `ra_parser` crate.
+
+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.
+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.
+
 # Logging
 
 Logging is done by both rust-analyzer and VS Code, so it might be tricky to
@@ -159,8 +356,8 @@ There's also two VS Code commands which might be of interest:
   rust code that it refers to and the rust editor will also highlight the proper
   text range.
 
-  If you press <kbd>Ctrl</kbd> (i.e. trigger goto definition) in the inspected
-  Rust source file the syntax tree read-only editor should scroll to and select the
+  If you trigger Go to Definition in the inspected Rust source file,
+  the syntax tree read-only editor should scroll to and select the
   appropriate syntax node token.
 
   ![demo](https://user-images.githubusercontent.com/36276403/78225773-6636a480-74d3-11ea-9d9f-1c9d42da03b0.png)
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index c57a93f12..a0847dad3 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -97,6 +97,30 @@ Invoking code action at this position will yield two code actions for importing
 * Is a fixed two-level structure enough?
 * Should we devise a general way to encode custom interaction protocols for GUI refactorings?
 
+## Lazy assists with `ResolveCodeAction`
+
+**Issue:** https://github.com/microsoft/language-server-protocol/issues/787
+
+**Client Capability** `{ "resolveCodeAction": boolean }`
+
+If this capability is set, the assists will be computed lazily. Thus `CodeAction` returned from the server will only contain `id` but not `edit` or `command` fields. The only exclusion from the rule is the diagnostic edits.
+
+After the client got the id, it should then call `experimental/resolveCodeAction` command on the server and provide the following payload:
+
+```typescript
+interface ResolveCodeActionParams {
+    id: string;
+    codeActionParams: lc.CodeActionParams;
+}
+```
+
+As a result of the command call the client will get the respective workspace edit (`lc.WorkspaceEdit`).
+
+### Unresolved Questions
+
+* Apply smarter filtering for ids?
+* Upon `resolveCodeAction` command only call the assits which should be resolved and not all of them?
+
 ## Parent Module
 
 **Issue:** https://github.com/microsoft/language-server-protocol/issues/1002
@@ -311,6 +335,50 @@ Moreover, it would be cool if editors didn't need to implement even basic langua
   This is how `SelectionRange` request works.
 * Alternatively, should we perhaps flag certain `SelectionRange`s as being brace pairs?
 
+## Runnables
+
+**Issue:** https://github.com/microsoft/language-server-protocol/issues/944
+
+**Server Capability:** `{ "runnables": { "kinds": string[] } }`
+
+This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`).
+
+**Method:** `experimental/runnables`
+
+**Request:**
+
+```typescript
+interface RunnablesParams {
+    textDocument: TextDocumentIdentifier;
+    /// If null, compute runnables for the whole file.
+    position?: Position;
+}
+```
+
+**Response:** `Runnable[]`
+
+```typescript
+interface Runnable {
+    label: string;
+    /// If this Runnable is associated with a specific function/module, etc, the location of this item
+    location?: LocationLink;
+    /// Running things is necessary technology specific, `kind` needs to be advertised via server capabilities,
+    // the type of `args` is specific to `kind`. The actual running is handled by the client.
+    kind: string;
+    args: any;
+}
+```
+
+rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look like this:
+
+```typescript
+{
+    workspaceRoot?: string;
+    cargoArgs: string[];
+    executableArgs: string[];
+}
+```
+
 ## Analyzer Status
 
 **Method:** `rust-analyzer/analyzerStatus`
@@ -400,38 +468,40 @@ interface InlayHint {
 }
 ```
 
-## Runnables
-
-**Method:** `rust-analyzer/runnables`
+## Hover Actions
 
-This request is send from client to server to get the list of things that can be run (tests, binaries, `cargo check -p`).
-Note that we plan to move this request to `experimental/runnables`, 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/944
+**Client Capability:** `{ "hoverActions": boolean }`
 
-**Request:**
+If this capability is set, `Hover` request returned from the server might contain an additional field, `actions`:
 
 ```typescript
-interface RunnablesParams {
-    textDocument: TextDocumentIdentifier;
-    /// If null, compute runnables for the whole file.
-    position?: Position;
+interface Hover {
+    ...
+    actions?: CommandLinkGroup[];
 }
-```
 
-**Response:** `Runnable[]`
+interface CommandLink extends Command {
+    /**
+     * A tooltip for the command, when represented in the UI.
+     */
+    tooltip?: string;
+}
 
-```typescript
-interface Runnable {
-    /// The range this runnable is applicable for.
-    range: lc.Range;
-    /// The label to show in the UI.
-    label: string;
-    /// The following fields describe a process to spawn.
-    bin: string;
-    args: string[];
-    /// Args for cargo after `--`.
-    extraArgs: string[];
-    env: { [key: string]: string };
-    cwd: string | null;
+interface CommandLinkGroup {
+    title?: string;
+    commands: CommandLink[];
 }
 ```
+
+Such actions on the client side are appended to a hover bottom as command links:
+```
+  +-----------------------------+
+  | Hover content               |
+  |                             |
+  +-----------------------------+
+  | _Action1_ | _Action2_       |  <- first group, no TITLE
+  +-----------------------------+
+  | TITLE _Action1_ | _Action2_ |  <- second group
+  +-----------------------------+
+  ...
+```
\ No newline at end of file