]> git.lizzy.rs Git - rust.git/blobdiff - docs/dev/README.md
Document import style
[rust.git] / docs / dev / README.md
index 3dc37e86ebaac7aabadab760f2f83f5672c11ea9..6f74d42236d428584efc8a6a88a9bd20f3db3925 100644 (file)
@@ -14,7 +14,7 @@ To learn more about how rust-analyzer works, see
 
 We also publish rustdoc docs to pages:
 
-https://rust-analyzer.github.io/rust-analyzer/ra_ide_api/index.html
+https://rust-analyzer.github.io/rust-analyzer/ra_ide/
 
 Various organizational and process issues are discussed in this document.
 
@@ -26,20 +26,11 @@ Discussion happens in this Zulip stream:
 
 https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0
 
-# Work List
-
-We have this "work list" paper document:
-
-https://paper.dropbox.com/doc/RLS-2.0-work-list--AZ3BgHKKCtqszbsi3gi6sjchAQ-42vbnxzuKq2lKwW0mkn8Y
-
-It shows what everyone is working on right now. If you want to (this is not
-mandatory), add yourself to the list!
-
 # Issue Labels
 
 * [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),
@@ -50,23 +41,25 @@ mandatory), add yourself to the list!
 
 # CI
 
-We use Travis for CI. Most of the things, including formatting, are checked by
+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. We use bors-ng to enforce the [not rocket
-science](https://graydon2.dreamwidth.org/1597.html) rule.
+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.
 
-You can run `cargo format-hook` to install git-hook to run rustfmt on commit.
+We use bors-ng to enforce the [not rocket science](https://graydon2.dreamwidth.org/1597.html) rule.
+
+You can run `cargo xtask install-pre-commit-hook` to install git-hook to run rustfmt on commit.
 
 # Code organization
 
 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 plugins for VS Code (in
-typescript) and Emacs (in elisp). The `docs` top-level directory contains both
-developer and user documentation.
+integrating with editors. Currently, it contains the plugin for VS Code (in
+typescript). The `docs` top-level directory contains both developer and user
+documentation.
 
-We have some automation infra in Rust in the `crates/tool` package. It contains
-stuff like formatting checking, code generation and powers `cargo install-code`.
+We have some automation infra in Rust in the `xtask` package. It contains
+stuff like formatting checking, code generation and powers `cargo xtask install`.
 The latter syntax is achieved with the help of cargo aliases (see `.cargo`
 directory).
 
@@ -81,21 +74,172 @@ relevant test and execute it (VS Code includes an action for running a single
 test).
 
 However, launching a VS Code instance with locally build language server is
-possible. There's even a VS Code task for this, so just <kbd>F5</kbd> should
-work (thanks, [@andrew-w-ross](https://github.com/andrew-w-ross)!).
+possible. There's **"Run Extension (Debug Build)"** launch configuration for this.
+
+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.
+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
+Code to sanity check that the thing works as I expect.
+
+If the problem concerns only the VS Code extension, I use **Run Installed Extension**
+launch configuration from `launch.json`. Notably, this uses the usual
+`rust-analyzer` binary from `PATH`. For this it is important to have the following
+in `setting.json` file:
+```json
+{
+    "rust-analyzer.serverPath": "rust-analyzer"
+}
+```
+After I am done with the fix, I use `cargo
+xtask install --client-code` to try the new extension for real.
+
+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. I usually
+just `cargo xtask install --server` and poke changes from my live environment.
+Note that this uses `--release`, which is usually faster overall, because
+loading stdlib into debug version of rust-analyzer takes a lot of time. To speed
+things up, sometimes I open a temporary hello-world project which has
+`"rust-analyzer.withSysroot": false` in `.code/settings.json`. 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. Note that you
+should only use `eprint!` family of macros for debugging: stdout is used for LSP
+communication, and `print!` would break it.
+
+If I need to fix something simultaneously in the server and in the client, I
+feel even more sad. I don't have a specific workflow for this case.
+
+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;
 
-I often just install development version with `cargo jinstall-lsp` and
-restart the host VS Code.
+use std::{ ... }
 
-See [./debugging.md](./debugging.md) for how to attach to rust-analyzer with
-debugger, and don't forget that rust-analyzer has useful `pd` snippet and `dbg`
-postfix completion for printf debugging :-)
+use crate_foo::{ ... }
+use crate_bar::{ ... }
 
-# Working With VS Code Extension
+use crate::{}
 
-To work on the VS Code extension, launch code inside `editors/code` and use `F5`
-to launch/debug. To automatically apply formatter and linter suggestions, use
-`npm run fix`.
+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 :-)
 
 # Logging
 
@@ -103,9 +247,8 @@ 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
-`flexi_logger` for logging frotend. By default, log goes to stderr (the same as
-with `env_logger`), but the stderr itself is processed by VS Code. To mirror
-logs to a `./log` directory, set `RA_LOG_DIR=1` environmental variable.
+`env_logger` for logging frontend. By default, log goes to stderr, but the
+stderr itself is processed by VS Code.
 
 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
@@ -115,7 +258,7 @@ To log all communication between the server and the client, there are two choice
 
 * you can log on the server side, by running something like
   ```
-  env RUST_LOG=gen_lsp_server=trace code .
+  env RA_LOG=gen_lsp_server=trace code .
   ```
 
 * you can log on the client side, by enabling `"rust-analyzer.trace.server":
@@ -129,16 +272,26 @@ There's also two VS Code commands which might be of interest:
 * `Rust Analyzer: Status` shows some memory-usage statistics. To take full
   advantage of it, you need to compile rust-analyzer with jemalloc support:
   ```
-  $ cargo install --path crates/ra_lsp_server --force --features jemalloc
+  $ cargo install --path crates/rust-analyzer --force --features jemalloc
   ```
 
-  There's an alias for this: `cargo jinstall-lsp`.
+  There's an alias for this: `cargo xtask install --server --jemalloc`.
 
 * `Rust Analyzer: Syntax Tree` shows syntax tree of the current file/selection.
 
+  You can hover over syntax nodes in the opened text file to see the appropriate
+  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
+  appropriate syntax node token.
+
+  ![demo](https://user-images.githubusercontent.com/36276403/78225773-6636a480-74d3-11ea-9d9f-1c9d42da03b0.png)
+
 # Profiling
 
-We have a built-in hierarchical profiler, you can enable it by using `RA_PROF` env-var:
+We have a built-in hierarchical profiler, you can enable it by using `RA_PROFILE` env-var:
 
 ```
 RA_PROFILE=*             // dump everything
@@ -146,17 +299,17 @@ RA_PROFILE=foo|bar|baz   // enabled only selected entries
 RA_PROFILE=*@3>10        // dump everything, up to depth 3, if it takes more than 10 ms
 ```
 
-In particular, I have `export RA_PROFILE='*>10' in my shell profile.
+In particular, I have `export RA_PROFILE='*>10'` in my shell profile.
 
 To measure time for from-scratch analysis, use something like this:
 
 ```
-$ cargo run --release -p ra_cli -- analysis-stats ../chalk/
+$ cargo run --release -p rust-analyzer -- analysis-stats ../chalk/
 ```
 
 For measuring time of incremental analysis, use either of these:
 
 ```
-$ cargo run --release -p ra_cli -- analysis-bench ../chalk/ --highlight ../chalk/chalk-engine/src/logic.rs
-$ cargo run --release -p ra_cli -- analysis-bench ../chalk/ --complete ../chalk/chalk-engine/src/logic.rs:94:0
+$ cargo run --release -p rust-analyzer -- analysis-bench ../chalk/ --highlight ../chalk/chalk-engine/src/logic.rs
+$ cargo run --release -p rust-analyzer -- analysis-bench ../chalk/ --complete ../chalk/chalk-engine/src/logic.rs:94:0
 ```