From 2980ba1fde50a6fc8863750b9dd7f09e3c1227ce Mon Sep 17 00:00:00 2001 From: robojumper Date: Mon, 4 May 2020 13:29:09 +0200 Subject: [PATCH 01/46] Support build.rs cargo:rustc-cfg --- crates/ra_cfg/src/lib.rs | 9 ++ .../ra_project_model/src/cargo_workspace.rs | 10 +- crates/ra_project_model/src/lib.rs | 7 + .../rust-analyzer/tests/heavy_tests/main.rs | 135 +++++++++++++++++- 4 files changed, 158 insertions(+), 3 deletions(-) diff --git a/crates/ra_cfg/src/lib.rs b/crates/ra_cfg/src/lib.rs index 51d953f6e89..697a0458147 100644 --- a/crates/ra_cfg/src/lib.rs +++ b/crates/ra_cfg/src/lib.rs @@ -53,4 +53,13 @@ impl CfgOptions { pub fn insert_features(&mut self, iter: impl IntoIterator) { iter.into_iter().for_each(|feat| self.insert_key_value("feature".into(), feat)); } + + /// Shortcut to set cfgs + pub fn insert_cfgs(&mut self, iter: impl IntoIterator) { + iter.into_iter().for_each(|cfg| match cfg.find('=') { + Some(split) => self + .insert_key_value(cfg[0..split].into(), cfg[split + 1..].trim_matches('"').into()), + None => self.insert_atom(cfg), + }); + } } diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index 362ee30fe04..afbd301646b 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -83,6 +83,7 @@ pub struct PackageData { pub dependencies: Vec, pub edition: Edition, pub features: Vec, + pub cfgs: Vec, pub out_dir: Option, pub proc_macro_dylib_path: Option, } @@ -165,10 +166,12 @@ impl CargoWorkspace { })?; let mut out_dir_by_id = FxHashMap::default(); + let mut cfgs = FxHashMap::default(); let mut proc_macro_dylib_paths = FxHashMap::default(); if cargo_features.load_out_dirs_from_check { let resources = load_extern_resources(cargo_toml, cargo_features)?; out_dir_by_id = resources.out_dirs; + cfgs = resources.cfgs; proc_macro_dylib_paths = resources.proc_dylib_paths; } @@ -194,6 +197,7 @@ impl CargoWorkspace { edition, dependencies: Vec::new(), features: Vec::new(), + cfgs: cfgs.get(&id).cloned().unwrap_or_default(), out_dir: out_dir_by_id.get(&id).cloned(), proc_macro_dylib_path: proc_macro_dylib_paths.get(&id).cloned(), }); @@ -275,6 +279,7 @@ impl CargoWorkspace { pub struct ExternResources { out_dirs: FxHashMap, proc_dylib_paths: FxHashMap, + cfgs: FxHashMap>, } pub fn load_extern_resources( @@ -300,8 +305,9 @@ pub fn load_extern_resources( for message in cargo_metadata::parse_messages(output.stdout.as_slice()) { if let Ok(message) = message { match message { - Message::BuildScriptExecuted(BuildScript { package_id, out_dir, .. }) => { - res.out_dirs.insert(package_id, out_dir); + Message::BuildScriptExecuted(BuildScript { package_id, out_dir, cfgs, .. }) => { + res.out_dirs.insert(package_id.clone(), out_dir); + res.cfgs.insert(package_id, cfgs); } Message::CompilerArtifact(message) => { diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 731cbd29181..2d5d61b611f 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -399,6 +399,13 @@ impl ProjectWorkspace { let cfg_options = { let mut opts = default_cfg_options.clone(); opts.insert_features(cargo[pkg].features.iter().map(Into::into)); + opts.insert_cfgs( + cargo[pkg] + .cfgs + .iter() + .filter_map(|c| c.to_str()) + .map(Into::into), + ); opts }; let mut env = Env::default(); diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index a218da76d6c..e94fbce3a1d 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -9,7 +9,8 @@ use lsp_types::{ }; use rust_analyzer::req::{ CodeActionParams, CodeActionRequest, Completion, CompletionParams, DidOpenTextDocument, - Formatting, GotoDefinition, HoverRequest, OnEnter, Runnables, RunnablesParams, + Formatting, GotoDefinition, GotoTypeDefinition, HoverRequest, OnEnter, Runnables, + RunnablesParams, }; use serde_json::json; use tempfile::TempDir; @@ -707,3 +708,135 @@ pub fn foo(_input: TokenStream) -> TokenStream { let value = res.get("contents").unwrap().get("value").unwrap().to_string(); assert_eq!(value, r#""```rust\nfoo::Bar\nfn bar()\n```""#) } + +#[test] +fn build_rs_cfgs() { + if skip_slow_tests() { + return; + } + + let server = Project::with_fixture( + r###" +//- Cargo.toml +[package] +name = "foo" +version = "0.0.0" + +//- build.rs + +fn main() { + println!("cargo:rustc-cfg=atom_cfg"); + println!("cargo:rustc-cfg=featlike=\"set\""); + println!("cargo:rerun-if-changed=build.rs"); +} +//- src/main.rs +#[cfg(atom_cfg)] +struct A; + +#[cfg(bad_atom_cfg)] +struct A; + +#[cfg(featlike = "set")] +struct B; + +#[cfg(featlike = "not_set")] +struct B; + +fn main() { + let va = A; + let vb = B; +} +"###, + ) + .with_config(|config| { + config.cargo.load_out_dirs_from_check = true; + }) + .server(); + server.wait_until_workspace_is_loaded(); + server.request::( + GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams::new( + server.doc_id("src/main.rs"), + Position::new(13, 9), + ), + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + json!([{ + "originSelectionRange": { + "end": { + "character": 10, + "line": 13 + }, + "start": { + "character": 8, + "line":13 + } + }, + "targetRange": { + "end": { + "character": 9, + "line": 1 + }, + "start": { + "character": 0, + "line":0 + } + }, + "targetSelectionRange": { + "end": { + "character": 8, + "line": 1 + }, + "start": { + "character": 7, + "line": 1 + } + }, + "targetUri": "file:///[..]src/main.rs" + }]), + ); + server.request::( + GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams::new( + server.doc_id("src/main.rs"), + Position::new(14, 9), + ), + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + json!([{ + "originSelectionRange": { + "end": { + "character": 10, + "line": 14 + }, + "start": { + "character": 8, + "line":14 + } + }, + "targetRange": { + "end": { + "character": 9, + "line": 7 + }, + "start": { + "character": 0, + "line":6 + } + }, + "targetSelectionRange": { + "end": { + "character": 8, + "line": 7 + }, + "start": { + "character": 7, + "line": 7 + } + }, + "targetUri": "file:///[..]src/main.rs" + }]), + ); +} From 0bf02f5ccac99c91f10ef46bb06ff2ea316c382c Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 5 May 2020 14:29:07 +0200 Subject: [PATCH 02/46] do not truncate display for hover #4311 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_ide/src/hover.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs index 54d31885822..df8451af258 100644 --- a/crates/ra_ide/src/hover.rs +++ b/crates/ra_ide/src/hover.rs @@ -208,7 +208,7 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option Date: Tue, 5 May 2020 14:53:18 +0200 Subject: [PATCH 03/46] Merge heavy tests --- .../rust-analyzer/tests/heavy_tests/main.rs | 239 ++++++++---------- 1 file changed, 105 insertions(+), 134 deletions(-) diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index e94fbce3a1d..2e52a29a8ee 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -575,7 +575,7 @@ version = \"0.0.0\" } #[test] -fn resolve_include_concat_env() { +fn out_dirs_check() { if skip_slow_tests() { return; } @@ -598,11 +598,28 @@ fn main() { r#"pub fn message() -> &'static str { "Hello, World!" }"#, ) .unwrap(); + println!("cargo:rustc-cfg=atom_cfg"); + println!("cargo:rustc-cfg=featlike=\"set\""); println!("cargo:rerun-if-changed=build.rs"); } //- src/main.rs include!(concat!(env!("OUT_DIR"), "/hello.rs")); +#[cfg(atom_cfg)] +struct A; +#[cfg(bad_atom_cfg)] +struct A; +#[cfg(featlike = "set")] +struct B; +#[cfg(featlike = "not_set")] +struct B; + +fn main() { + let va = A; + let vb = B; + message(); +} + fn main() { message(); } "###, ) @@ -614,12 +631,98 @@ fn main() { message(); } let res = server.send_request::(GotoDefinitionParams { text_document_position_params: TextDocumentPositionParams::new( server.doc_id("src/main.rs"), - Position::new(2, 15), + Position::new(14, 8), ), work_done_progress_params: Default::default(), partial_result_params: Default::default(), }); assert!(format!("{}", res).contains("hello.rs")); + server.request::( + GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams::new( + server.doc_id("src/main.rs"), + Position::new(12, 9), + ), + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + json!([{ + "originSelectionRange": { + "end": { + "character": 10, + "line": 12 + }, + "start": { + "character": 8, + "line": 12 + } + }, + "targetRange": { + "end": { + "character": 9, + "line": 3 + }, + "start": { + "character": 0, + "line": 2 + } + }, + "targetSelectionRange": { + "end": { + "character": 8, + "line": 3 + }, + "start": { + "character": 7, + "line": 3 + } + }, + "targetUri": "file:///[..]src/main.rs" + }]), + ); + server.request::( + GotoDefinitionParams { + text_document_position_params: TextDocumentPositionParams::new( + server.doc_id("src/main.rs"), + Position::new(13, 9), + ), + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + json!([{ + "originSelectionRange": { + "end": { + "character": 10, + "line": 13 + }, + "start": { + "character": 8, + "line":13 + } + }, + "targetRange": { + "end": { + "character": 9, + "line": 7 + }, + "start": { + "character": 0, + "line":6 + } + }, + "targetSelectionRange": { + "end": { + "character": 8, + "line": 7 + }, + "start": { + "character": 7, + "line": 7 + } + }, + "targetUri": "file:///[..]src/main.rs" + }]), + ); } #[test] @@ -708,135 +811,3 @@ pub fn foo(_input: TokenStream) -> TokenStream { let value = res.get("contents").unwrap().get("value").unwrap().to_string(); assert_eq!(value, r#""```rust\nfoo::Bar\nfn bar()\n```""#) } - -#[test] -fn build_rs_cfgs() { - if skip_slow_tests() { - return; - } - - let server = Project::with_fixture( - r###" -//- Cargo.toml -[package] -name = "foo" -version = "0.0.0" - -//- build.rs - -fn main() { - println!("cargo:rustc-cfg=atom_cfg"); - println!("cargo:rustc-cfg=featlike=\"set\""); - println!("cargo:rerun-if-changed=build.rs"); -} -//- src/main.rs -#[cfg(atom_cfg)] -struct A; - -#[cfg(bad_atom_cfg)] -struct A; - -#[cfg(featlike = "set")] -struct B; - -#[cfg(featlike = "not_set")] -struct B; - -fn main() { - let va = A; - let vb = B; -} -"###, - ) - .with_config(|config| { - config.cargo.load_out_dirs_from_check = true; - }) - .server(); - server.wait_until_workspace_is_loaded(); - server.request::( - GotoDefinitionParams { - text_document_position_params: TextDocumentPositionParams::new( - server.doc_id("src/main.rs"), - Position::new(13, 9), - ), - work_done_progress_params: Default::default(), - partial_result_params: Default::default(), - }, - json!([{ - "originSelectionRange": { - "end": { - "character": 10, - "line": 13 - }, - "start": { - "character": 8, - "line":13 - } - }, - "targetRange": { - "end": { - "character": 9, - "line": 1 - }, - "start": { - "character": 0, - "line":0 - } - }, - "targetSelectionRange": { - "end": { - "character": 8, - "line": 1 - }, - "start": { - "character": 7, - "line": 1 - } - }, - "targetUri": "file:///[..]src/main.rs" - }]), - ); - server.request::( - GotoDefinitionParams { - text_document_position_params: TextDocumentPositionParams::new( - server.doc_id("src/main.rs"), - Position::new(14, 9), - ), - work_done_progress_params: Default::default(), - partial_result_params: Default::default(), - }, - json!([{ - "originSelectionRange": { - "end": { - "character": 10, - "line": 14 - }, - "start": { - "character": 8, - "line":14 - } - }, - "targetRange": { - "end": { - "character": 9, - "line": 7 - }, - "start": { - "character": 0, - "line":6 - } - }, - "targetSelectionRange": { - "end": { - "character": 8, - "line": 7 - }, - "start": { - "character": 7, - "line": 7 - } - }, - "targetUri": "file:///[..]src/main.rs" - }]), - ); -} From f2dd233ddc60b647fe9c32ea2d712224005ae99e Mon Sep 17 00:00:00 2001 From: robojumper Date: Tue, 5 May 2020 14:53:52 +0200 Subject: [PATCH 04/46] Assume cargo_metadata uses String for cfgs soon --- crates/ra_project_model/src/cargo_workspace.rs | 11 ++++++++--- crates/ra_project_model/src/lib.rs | 8 +------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index afbd301646b..bf83adc42a3 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -83,7 +83,7 @@ pub struct PackageData { pub dependencies: Vec, pub edition: Edition, pub features: Vec, - pub cfgs: Vec, + pub cfgs: Vec, pub out_dir: Option, pub proc_macro_dylib_path: Option, } @@ -279,7 +279,7 @@ impl CargoWorkspace { pub struct ExternResources { out_dirs: FxHashMap, proc_dylib_paths: FxHashMap, - cfgs: FxHashMap>, + cfgs: FxHashMap>, } pub fn load_extern_resources( @@ -307,7 +307,12 @@ pub fn load_extern_resources( match message { Message::BuildScriptExecuted(BuildScript { package_id, out_dir, cfgs, .. }) => { res.out_dirs.insert(package_id.clone(), out_dir); - res.cfgs.insert(package_id, cfgs); + res.cfgs.insert( + package_id, + // FIXME: Current `cargo_metadata` uses `PathBuf` instead of `String`, + // change when https://github.com/oli-obk/cargo_metadata/pulls/112 reaches crates.io + cfgs.iter().filter_map(|c| c.to_str().map(|s| s.to_owned())).collect(), + ); } Message::CompilerArtifact(message) => { diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 2d5d61b611f..7231da221a4 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -399,13 +399,7 @@ impl ProjectWorkspace { let cfg_options = { let mut opts = default_cfg_options.clone(); opts.insert_features(cargo[pkg].features.iter().map(Into::into)); - opts.insert_cfgs( - cargo[pkg] - .cfgs - .iter() - .filter_map(|c| c.to_str()) - .map(Into::into), - ); + opts.insert_cfgs(cargo[pkg].cfgs.iter().map(Into::into)); opts }; let mut env = Env::default(); From 2b06041692dcdbc1c49292fd64fdc67da312c1ca Mon Sep 17 00:00:00 2001 From: "Daniel M. Capella" Date: Tue, 5 May 2020 18:23:32 -0400 Subject: [PATCH 05/46] Update Arch Linux and ALE install instructions Package has been added to the Arch repos: https://www.archlinux.org/packages/community/x86_64/rust-analyzer/ ALE merged rust-analyzer support: https://github.com/dense-analysis/ale/commit/70005134e5b2d40d176ee5b851ac64a296b22201 --- docs/user/readme.adoc | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/docs/user/readme.adoc b/docs/user/readme.adoc index 69f5b13d6b8..2f2b1d37a45 100644 --- a/docs/user/readme.adoc +++ b/docs/user/readme.adoc @@ -139,17 +139,16 @@ If your editor can't find the binary even though the binary is on your `$PATH`, ==== Arch Linux -The `rust-analyzer` binary can be installed from AUR (Arch User Repository): +The `rust-analyzer` binary can be installed from the repos or AUR (Arch User Repository): -- https://aur.archlinux.org/packages/rust-analyzer-bin[`rust-analyzer-bin`] (binary from GitHub releases) -- https://aur.archlinux.org/packages/rust-analyzer[`rust-analyzer`] (built from latest tagged source) -- https://aur.archlinux.org/packages/rust-analyzer-git[`rust-analyzer-git`] (latest git version) +- https://www.archlinux.org/packages/community/x86_64/rust-analyzer/[`rust-analyzer`] (built from latest tagged source) +- https://aur.archlinux.org/packages/rust-analyzer-git[`rust-analyzer-git`] (latest Git version) -Install it with AUR helper of your choice, for example: +Install it with pacman, for example: [source,bash] ---- -$ yay -S rust-analyzer-bin +$ pacman -S rust-analyzer ---- === Emacs @@ -187,7 +186,7 @@ The are several LSP client implementations for vim or neovim: 1. Install LanguageClient-neovim by following the instructions https://github.com/autozimu/LanguageClient-neovim[here] - * The github project wiki has extra tips on configuration + * The GitHub project wiki has extra tips on configuration 2. Configure by adding this to your vim/neovim config file (replacing the existing Rust-specific line if it exists): + @@ -220,17 +219,11 @@ let g:ycm_language_server = ==== ALE -To add the LSP server to https://github.com/dense-analysis/ale[ale]: +To use the LSP server in https://github.com/dense-analysis/ale[ale]: [source,vim] ---- -call ale#linter#Define('rust', { -\ 'name': 'rust-analyzer', -\ 'lsp': 'stdio', -\ 'executable': 'rust-analyzer', -\ 'command': '%e', -\ 'project_root': '.', -\}) +let g:ale_linters = {'rust': ['analyzer']} ---- ==== nvim-lsp From ffaef1b7aeb61984992e231d9af20f39486403ea Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Tue, 5 May 2020 11:59:41 -0700 Subject: [PATCH 06/46] ra_project_model: look for Cargo in more places See #3118 --- Cargo.lock | 77 ++++++++++++++++++- crates/ra_project_model/Cargo.toml | 2 + .../ra_project_model/src/cargo_workspace.rs | 59 +++++++++++--- 3 files changed, 128 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d50a766fe5..b2d4569b386 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,12 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33954243bd79057c2de7338850b85983a44588021f8a5fee574a8888c6de4344" +[[package]] +name = "arrayref" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4c527152e37cf757a3f78aae5a06fbeefdb07ccc535c980a3208ee3060dd544" + [[package]] name = "arrayvec" version = "0.5.1" @@ -66,6 +72,12 @@ dependencies = [ "libc", ] +[[package]] +name = "base64" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b41b7ea54a0c9d92199de89e20e58d49f02f8e699814ef3fdf266f6f748d15c7" + [[package]] name = "base64" version = "0.12.0" @@ -78,6 +90,17 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "blake2b_simd" +version = "0.5.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d8fb2d74254a3a0b5cac33ac9f8ed0e44aa50378d9dbb2e5d83bd21ed1dc2c8a" +dependencies = [ + "arrayref", + "arrayvec", + "constant_time_eq", +] + [[package]] name = "bstr" version = "0.2.12" @@ -212,6 +235,12 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "constant_time_eq" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" + [[package]] name = "crossbeam" version = "0.7.3" @@ -289,6 +318,28 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198" +[[package]] +name = "dirs" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13aea89a5c93364a98e9b37b2fa237effbb694d5cfe01c5b70941f7eb087d5e3" +dependencies = [ + "cfg-if", + "dirs-sys", +] + +[[package]] +name = "dirs-sys" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "afa0b23de8fd801745c471deffa6e12d248f962c9fd4b4c33787b055599bde7b" +dependencies = [ + "cfg-if", + "libc", + "redox_users", + "winapi 0.3.8", +] + [[package]] name = "drop_bomb" version = "0.1.4" @@ -659,7 +710,7 @@ version = "0.74.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0e6a2b8837d27b29deb3f3e6dc1c6d2f57947677f9be1024e482ec5b59525" dependencies = [ - "base64", + "base64 0.12.0", "bitflags", "serde", "serde_json", @@ -1158,6 +1209,7 @@ version = "0.1.0" dependencies = [ "anyhow", "cargo_metadata", + "dirs", "log", "ra_arena", "ra_cfg", @@ -1298,6 +1350,17 @@ version = "0.1.56" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84" +[[package]] +name = "redox_users" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09b23093265f8d200fa7b4c2c76297f47e681c655f6f1285a8780d6a022f7431" +dependencies = [ + "getrandom", + "redox_syscall", + "rust-argon2", +] + [[package]] name = "regex" version = "1.3.7" @@ -1382,6 +1445,18 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "rust-argon2" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bc8af4bda8e1ff4932523b94d3dd20ee30a87232323eda55903ffd71d2fb017" +dependencies = [ + "base64 0.11.0", + "blake2b_simd", + "constant_time_eq", + "crossbeam-utils", +] + [[package]] name = "rustc-ap-rustc_lexer" version = "656.0.0" diff --git a/crates/ra_project_model/Cargo.toml b/crates/ra_project_model/Cargo.toml index 5e651fe70d9..91958ed947c 100644 --- a/crates/ra_project_model/Cargo.toml +++ b/crates/ra_project_model/Cargo.toml @@ -22,3 +22,5 @@ serde = { version = "1.0.106", features = ["derive"] } serde_json = "1.0.48" anyhow = "1.0.26" + +dirs = "2.0" diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index 59f46a2a05e..a15a8c68eda 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -8,7 +8,7 @@ use std::{ process::Command, }; -use anyhow::{Context, Result}; +use anyhow::{Context, Error, Result}; use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId}; use ra_arena::{Arena, Idx}; use ra_db::Edition; @@ -145,12 +145,8 @@ impl CargoWorkspace { cargo_toml: &Path, cargo_features: &CargoConfig, ) -> Result { - let _ = Command::new(cargo_binary()) - .arg("--version") - .output() - .context("failed to run `cargo --version`, is `cargo` in PATH?")?; - let mut meta = MetadataCommand::new(); + meta.cargo_path(cargo_binary()?); meta.manifest_path(cargo_toml); if cargo_features.all_features { meta.features(CargoOpt::AllFeatures); @@ -288,7 +284,7 @@ pub fn load_extern_resources( cargo_toml: &Path, cargo_features: &CargoConfig, ) -> Result { - let mut cmd = Command::new(cargo_binary()); + let mut cmd = Command::new(cargo_binary()?); cmd.args(&["check", "--message-format=json", "--manifest-path"]).arg(cargo_toml); if cargo_features.all_features { cmd.arg("--all-features"); @@ -337,6 +333,51 @@ fn is_dylib(path: &Path) -> bool { } } -fn cargo_binary() -> String { - env::var("CARGO").unwrap_or_else(|_| "cargo".to_string()) +/// Return a `String` to use for executable `cargo`. +/// +/// E.g., this may just be `cargo` if that gives a valid Cargo executable; or it +/// may be a full path to a valid Cargo. +fn cargo_binary() -> Result { + // The current implementation checks three places for a `cargo` to use: + // 1) $CARGO environment variable (erroring if this is set but not a usable Cargo) + // 2) `cargo` + // 3) `~/.cargo/bin/cargo` + if let Ok(path) = env::var("CARGO") { + if is_valid_cargo(&path) { + Ok(path) + } else { + Err(Error::msg("`CARGO` environment variable points to something that's not a valid Cargo executable")) + } + } else { + let final_path: Option = if is_valid_cargo("cargo") { + Some("cargo".to_owned()) + } else { + if let Some(mut path) = dirs::home_dir() { + path.push(".cargo"); + path.push("bin"); + path.push("cargo"); + if is_valid_cargo(&path) { + Some(path.into_os_string().into_string().expect("Invalid Unicode in path")) + } else { + None + } + } else { + None + } + }; + final_path.ok_or( + // This error message may also be caused by $PATH or $CARGO not being set correctly for VSCode, + // even if they are set correctly in a terminal. + // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode + // to inherit environment variables including $PATH and $CARGO from that terminal; but + // launching VSCode from Dock does not inherit environment variables from a terminal. + // For more discussion, see #3118. + Error::msg("Failed to find `cargo` executable. Make sure `cargo` is in `$PATH`, or set `$CARGO` to point to a valid Cargo executable.") + ) + } +} + +/// Does the given `Path` point to a usable `Cargo`? +fn is_valid_cargo(p: impl AsRef) -> bool { + Command::new(p.as_ref()).arg("--version").output().is_ok() } From 5aa1bba107ef434e61c3136120b9478a307d67a9 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Tue, 5 May 2020 13:44:43 -0700 Subject: [PATCH 07/46] more generic, find rustc as well --- .../ra_project_model/src/cargo_workspace.rs | 57 ++--------------- .../ra_project_model/src/find_executables.rs | 63 +++++++++++++++++++ crates/ra_project_model/src/lib.rs | 1 + crates/ra_project_model/src/sysroot.rs | 4 +- 4 files changed, 71 insertions(+), 54 deletions(-) create mode 100644 crates/ra_project_model/src/find_executables.rs diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index a15a8c68eda..c7350d1e021 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -1,14 +1,14 @@ //! FIXME: write short doc here use std::{ - env, ffi::OsStr, ops, path::{Path, PathBuf}, process::Command, }; -use anyhow::{Context, Error, Result}; +use super::find_executables::get_path_for_executable; +use anyhow::{Context, Result}; use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId}; use ra_arena::{Arena, Idx}; use ra_db::Edition; @@ -146,7 +146,7 @@ impl CargoWorkspace { cargo_features: &CargoConfig, ) -> Result { let mut meta = MetadataCommand::new(); - meta.cargo_path(cargo_binary()?); + meta.cargo_path(get_path_for_executable("cargo")?); meta.manifest_path(cargo_toml); if cargo_features.all_features { meta.features(CargoOpt::AllFeatures); @@ -284,7 +284,7 @@ pub fn load_extern_resources( cargo_toml: &Path, cargo_features: &CargoConfig, ) -> Result { - let mut cmd = Command::new(cargo_binary()?); + let mut cmd = Command::new(get_path_for_executable("cargo")?); cmd.args(&["check", "--message-format=json", "--manifest-path"]).arg(cargo_toml); if cargo_features.all_features { cmd.arg("--all-features"); @@ -332,52 +332,3 @@ fn is_dylib(path: &Path) -> bool { Some(ext) => matches!(ext.as_str(), "dll" | "dylib" | "so"), } } - -/// Return a `String` to use for executable `cargo`. -/// -/// E.g., this may just be `cargo` if that gives a valid Cargo executable; or it -/// may be a full path to a valid Cargo. -fn cargo_binary() -> Result { - // The current implementation checks three places for a `cargo` to use: - // 1) $CARGO environment variable (erroring if this is set but not a usable Cargo) - // 2) `cargo` - // 3) `~/.cargo/bin/cargo` - if let Ok(path) = env::var("CARGO") { - if is_valid_cargo(&path) { - Ok(path) - } else { - Err(Error::msg("`CARGO` environment variable points to something that's not a valid Cargo executable")) - } - } else { - let final_path: Option = if is_valid_cargo("cargo") { - Some("cargo".to_owned()) - } else { - if let Some(mut path) = dirs::home_dir() { - path.push(".cargo"); - path.push("bin"); - path.push("cargo"); - if is_valid_cargo(&path) { - Some(path.into_os_string().into_string().expect("Invalid Unicode in path")) - } else { - None - } - } else { - None - } - }; - final_path.ok_or( - // This error message may also be caused by $PATH or $CARGO not being set correctly for VSCode, - // even if they are set correctly in a terminal. - // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode - // to inherit environment variables including $PATH and $CARGO from that terminal; but - // launching VSCode from Dock does not inherit environment variables from a terminal. - // For more discussion, see #3118. - Error::msg("Failed to find `cargo` executable. Make sure `cargo` is in `$PATH`, or set `$CARGO` to point to a valid Cargo executable.") - ) - } -} - -/// Does the given `Path` point to a usable `Cargo`? -fn is_valid_cargo(p: impl AsRef) -> bool { - Command::new(p.as_ref()).arg("--version").output().is_ok() -} diff --git a/crates/ra_project_model/src/find_executables.rs b/crates/ra_project_model/src/find_executables.rs new file mode 100644 index 00000000000..9b020d3dafc --- /dev/null +++ b/crates/ra_project_model/src/find_executables.rs @@ -0,0 +1,63 @@ +use anyhow::{Error, Result}; +use std::env; +use std::path::Path; +use std::process::Command; + +/// Return a `String` to use for the given executable. +/// +/// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that +/// gives a valid Cargo executable; or it may return a full path to a valid +/// Cargo. +pub fn get_path_for_executable(executable_name: impl AsRef) -> Result { + // The current implementation checks three places for an executable to use: + // 1) Appropriate environment variable (erroring if this is set but not a usable executable) + // example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc + // 2) `` + // example: for cargo, this tries just `cargo`, which will succeed if `cargo` in on the $PATH + // 3) `~/.cargo/bin/` + // example: for cargo, this tries ~/.cargo/bin/cargo + // It seems that this is a reasonable place to try for cargo, rustc, and rustup + let executable_name = executable_name.as_ref(); + let env_var = executable_name.to_ascii_uppercase(); + if let Ok(path) = env::var(&env_var) { + if is_valid_executable(&path) { + Ok(path) + } else { + Err(Error::msg(format!("`{}` environment variable points to something that's not a valid executable", env_var))) + } + } else { + let final_path: Option = if is_valid_executable(executable_name) { + Some(executable_name.to_owned()) + } else { + if let Some(mut path) = dirs::home_dir() { + path.push(".cargo"); + path.push("bin"); + path.push(executable_name); + if is_valid_executable(&path) { + Some(path.into_os_string().into_string().expect("Invalid Unicode in path")) + } else { + None + } + } else { + None + } + }; + final_path.ok_or( + // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly + // for VSCode, even if they are set correctly in a terminal. + // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode + // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; + // but launching VSCode from Dock does not inherit environment variables from a terminal. + // For more discussion, see #3118. + Error::msg(format!("Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", executable_name, executable_name, env_var)) + ) + } +} + +/// Does the given `Path` point to a usable executable? +/// +/// (assumes the executable takes a `--version` switch and writes to stdout, +/// which is true for `cargo`, `rustc`, and `rustup`) +fn is_valid_executable(p: impl AsRef) -> bool { + Command::new(p.as_ref()).arg("--version").output().is_ok() +} diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index c2b33c1dcaf..5028b6b6df9 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -1,6 +1,7 @@ //! FIXME: write short doc here mod cargo_workspace; +mod find_executables; mod json_project; mod sysroot; diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 55ff5ad80bd..8d68032b2ab 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -1,5 +1,6 @@ //! FIXME: write short doc here +use super::find_executables::get_path_for_executable; use anyhow::{bail, Context, Result}; use std::{ env, ops, @@ -114,7 +115,8 @@ fn get_or_install_rust_src(cargo_toml: &Path) -> Result { if let Ok(path) = env::var("RUST_SRC_PATH") { return Ok(path.into()); } - let rustc_output = run_command_in_cargo_dir(cargo_toml, "rustc", &["--print", "sysroot"])?; + let rustc = get_path_for_executable("rustc")?; + let rustc_output = run_command_in_cargo_dir(cargo_toml, &rustc, &["--print", "sysroot"])?; let stdout = String::from_utf8(rustc_output.stdout)?; let sysroot_path = Path::new(stdout.trim()); let src_path = sysroot_path.join("lib/rustlib/src/rust/src"); From 303b444dbb66019fc916dd350e54f7675aa3007f Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Tue, 5 May 2020 14:07:10 -0700 Subject: [PATCH 08/46] pull function out into new crate ra_env; use in ra_flycheck as well --- Cargo.lock | 11 ++++++++++- crates/ra_env/Cargo.toml | 9 +++++++++ .../src/find_executables.rs => ra_env/src/lib.rs} | 2 +- crates/ra_flycheck/Cargo.toml | 1 + crates/ra_flycheck/src/lib.rs | 8 ++------ crates/ra_project_model/Cargo.toml | 5 ++--- crates/ra_project_model/src/cargo_workspace.rs | 2 +- crates/ra_project_model/src/lib.rs | 4 ++-- crates/ra_project_model/src/sysroot.rs | 5 +++-- 9 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 crates/ra_env/Cargo.toml rename crates/{ra_project_model/src/find_executables.rs => ra_env/src/lib.rs} (98%) diff --git a/Cargo.lock b/Cargo.lock index b2d4569b386..28424b7d4da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -999,6 +999,14 @@ dependencies = [ "test_utils", ] +[[package]] +name = "ra_env" +version = "0.1.0" +dependencies = [ + "anyhow", + "dirs", +] + [[package]] name = "ra_flycheck" version = "0.1.0" @@ -1009,6 +1017,7 @@ dependencies = [ "jod-thread", "log", "lsp-types", + "ra_env", "serde_json", ] @@ -1209,11 +1218,11 @@ version = "0.1.0" dependencies = [ "anyhow", "cargo_metadata", - "dirs", "log", "ra_arena", "ra_cfg", "ra_db", + "ra_env", "ra_proc_macro", "rustc-hash", "serde", diff --git a/crates/ra_env/Cargo.toml b/crates/ra_env/Cargo.toml new file mode 100644 index 00000000000..7fed446a701 --- /dev/null +++ b/crates/ra_env/Cargo.toml @@ -0,0 +1,9 @@ +[package] +edition = "2018" +name = "ra_env" +version = "0.1.0" +authors = ["rust-analyzer developers"] + +[dependencies] +anyhow = "1.0.26" +dirs = "2.0" diff --git a/crates/ra_project_model/src/find_executables.rs b/crates/ra_env/src/lib.rs similarity index 98% rename from crates/ra_project_model/src/find_executables.rs rename to crates/ra_env/src/lib.rs index 9b020d3dafc..cb9fbf80c98 100644 --- a/crates/ra_project_model/src/find_executables.rs +++ b/crates/ra_env/src/lib.rs @@ -13,7 +13,7 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result` - // example: for cargo, this tries just `cargo`, which will succeed if `cargo` in on the $PATH + // example: for cargo, this tries just `cargo`, which will succeed if `cargo` is on the $PATH // 3) `~/.cargo/bin/` // example: for cargo, this tries ~/.cargo/bin/cargo // It seems that this is a reasonable place to try for cargo, rustc, and rustup diff --git a/crates/ra_flycheck/Cargo.toml b/crates/ra_flycheck/Cargo.toml index 3d5093264e8..d0f7fb2dcf2 100644 --- a/crates/ra_flycheck/Cargo.toml +++ b/crates/ra_flycheck/Cargo.toml @@ -14,6 +14,7 @@ log = "0.4.8" cargo_metadata = "0.9.1" serde_json = "1.0.48" jod-thread = "0.1.1" +ra_env = { path = "../ra_env" } [dev-dependencies] insta = "0.16.0" diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index f27252949bd..d8b727b0ead 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -4,7 +4,6 @@ mod conv; use std::{ - env, io::{self, BufRead, BufReader}, path::PathBuf, process::{Command, Stdio}, @@ -17,6 +16,7 @@ use lsp_types::{ CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, }; +use ra_env::get_path_for_executable; use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; @@ -216,7 +216,7 @@ impl FlycheckThread { let mut cmd = match &self.config { FlycheckConfig::CargoCommand { command, all_targets, all_features, extra_args } => { - let mut cmd = Command::new(cargo_binary()); + let mut cmd = Command::new(get_path_for_executable("cargo").unwrap()); cmd.arg(command); cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]); cmd.arg(self.workspace_root.join("Cargo.toml")); @@ -337,7 +337,3 @@ fn run_cargo( Ok(()) } - -fn cargo_binary() -> String { - env::var("CARGO").unwrap_or_else(|_| "cargo".to_string()) -} diff --git a/crates/ra_project_model/Cargo.toml b/crates/ra_project_model/Cargo.toml index 91958ed947c..62647846898 100644 --- a/crates/ra_project_model/Cargo.toml +++ b/crates/ra_project_model/Cargo.toml @@ -14,13 +14,12 @@ rustc-hash = "1.1.0" cargo_metadata = "0.9.1" ra_arena = { path = "../ra_arena" } -ra_db = { path = "../ra_db" } ra_cfg = { path = "../ra_cfg" } +ra_db = { path = "../ra_db" } +ra_env = { path = "../ra_env" } ra_proc_macro = { path = "../ra_proc_macro" } serde = { version = "1.0.106", features = ["derive"] } serde_json = "1.0.48" anyhow = "1.0.26" - -dirs = "2.0" diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index c7350d1e021..4027f020f3b 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -7,11 +7,11 @@ use std::{ process::Command, }; -use super::find_executables::get_path_for_executable; use anyhow::{Context, Result}; use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId}; use ra_arena::{Arena, Idx}; use ra_db::Edition; +use ra_env::get_path_for_executable; use rustc_hash::FxHashMap; /// `CargoWorkspace` represents the logical structure of, well, a Cargo diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 5028b6b6df9..e4b86f1e20f 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -1,7 +1,6 @@ //! FIXME: write short doc here mod cargo_workspace; -mod find_executables; mod json_project; mod sysroot; @@ -15,6 +14,7 @@ use std::{ use anyhow::{bail, Context, Result}; use ra_cfg::CfgOptions; use ra_db::{CrateGraph, CrateName, Edition, Env, ExternSource, ExternSourceId, FileId}; +use ra_env::get_path_for_executable; use rustc_hash::FxHashMap; use serde_json::from_reader; @@ -559,7 +559,7 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions { match (|| -> Result { // `cfg(test)` and `cfg(debug_assertion)` are handled outside, so we suppress them here. - let mut cmd = Command::new("rustc"); + let mut cmd = Command::new(get_path_for_executable("rustc")?); cmd.args(&["--print", "cfg", "-O"]); if let Some(target) = target { cmd.args(&["--target", target.as_str()]); diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 8d68032b2ab..516e0472dab 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -1,6 +1,5 @@ //! FIXME: write short doc here -use super::find_executables::get_path_for_executable; use anyhow::{bail, Context, Result}; use std::{ env, ops, @@ -9,6 +8,7 @@ use std::{ }; use ra_arena::{Arena, Idx}; +use ra_env::get_path_for_executable; #[derive(Default, Debug, Clone)] pub struct Sysroot { @@ -122,7 +122,8 @@ fn get_or_install_rust_src(cargo_toml: &Path) -> Result { let src_path = sysroot_path.join("lib/rustlib/src/rust/src"); if !src_path.exists() { - run_command_in_cargo_dir(cargo_toml, "rustup", &["component", "add", "rust-src"])?; + let rustup = get_path_for_executable("rustup")?; + run_command_in_cargo_dir(cargo_toml, &rustup, &["component", "add", "rust-src"])?; } if !src_path.exists() { bail!( From 7e60264ba0dc33110559390868c7c966f0ab2e64 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Tue, 5 May 2020 14:07:52 -0700 Subject: [PATCH 09/46] cargo fmt --- crates/ra_env/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ra_env/src/lib.rs b/crates/ra_env/src/lib.rs index cb9fbf80c98..df20f783d8d 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_env/src/lib.rs @@ -23,7 +23,10 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result = if is_valid_executable(executable_name) { From 3e603a8fdd207f9ad5a2ad2898350f54d5bc2fb8 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Tue, 5 May 2020 14:41:47 -0700 Subject: [PATCH 10/46] add module-level docs so that tests pass --- crates/ra_env/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ra_env/src/lib.rs b/crates/ra_env/src/lib.rs index df20f783d8d..c7b49e9979c 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_env/src/lib.rs @@ -1,3 +1,7 @@ +//! This crate contains a single public function +//! [`get_path_for_executable`](fn.get_path_for_executable.html). +//! See docs there for more information. + use anyhow::{Error, Result}; use std::env; use std::path::Path; From a78dd06951dffcc6ff69aec21a2d8224c12f5026 Mon Sep 17 00:00:00 2001 From: veetaha Date: Wed, 6 May 2020 01:22:02 +0300 Subject: [PATCH 11/46] Preliminary refactoring of cargo.ts --- editors/code/src/cargo.ts | 45 ++++++++------------------ editors/code/src/commands/runnables.ts | 7 ++-- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/editors/code/src/cargo.ts b/editors/code/src/cargo.ts index a328ba9bd01..613aa82da9a 100644 --- a/editors/code/src/cargo.ts +++ b/editors/code/src/cargo.ts @@ -10,17 +10,9 @@ interface CompilationArtifact { } export class Cargo { - rootFolder: string; - env?: Record; - output: OutputChannel; + constructor(readonly rootFolder: string, readonly output: OutputChannel) { } - public constructor(cargoTomlFolder: string, output: OutputChannel, env: Record | undefined = undefined) { - this.rootFolder = cargoTomlFolder; - this.output = output; - this.env = env; - } - - public async artifactsFromArgs(cargoArgs: string[]): Promise { + private async artifactsFromArgs(cargoArgs: string[]): Promise { const artifacts: CompilationArtifact[] = []; try { @@ -37,17 +29,13 @@ export class Cargo { isTest: message.profile.test }); } - } - else if (message.reason === 'compiler-message') { + } else if (message.reason === 'compiler-message') { this.output.append(message.message.rendered); } }, - stderr => { - this.output.append(stderr); - } + stderr => this.output.append(stderr), ); - } - catch (err) { + } catch (err) { this.output.show(true); throw new Error(`Cargo invocation has failed: ${err}`); } @@ -55,9 +43,8 @@ export class Cargo { return artifacts; } - public async executableFromArgs(args: string[]): Promise { - const cargoArgs = [...args]; // to remain args unchanged - cargoArgs.push("--message-format=json"); + async executableFromArgs(args: readonly string[]): Promise { + const cargoArgs = [...args, "--message-format=json"]; const artifacts = await this.artifactsFromArgs(cargoArgs); @@ -70,24 +57,20 @@ export class Cargo { return artifacts[0].fileName; } - runCargo( + private runCargo( cargoArgs: string[], onStdoutJson: (obj: any) => void, onStderrString: (data: string) => void ): Promise { - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const cargo = cp.spawn('cargo', cargoArgs, { stdio: ['ignore', 'pipe', 'pipe'], - cwd: this.rootFolder, - env: this.env, + cwd: this.rootFolder }); - cargo.on('error', err => { - reject(new Error(`could not launch cargo: ${err}`)); - }); - cargo.stderr.on('data', chunk => { - onStderrString(chunk.toString()); - }); + cargo.on('error', err => reject(new Error(`could not launch cargo: ${err}`))); + + cargo.stderr.on('data', chunk => onStderrString(chunk.toString())); const rl = readline.createInterface({ input: cargo.stdout }); rl.on('line', line => { @@ -103,4 +86,4 @@ export class Cargo { }); }); } -} \ No newline at end of file +} diff --git a/editors/code/src/commands/runnables.ts b/editors/code/src/commands/runnables.ts index d77e8188c75..2ed150e25f2 100644 --- a/editors/code/src/commands/runnables.ts +++ b/editors/code/src/commands/runnables.ts @@ -119,8 +119,11 @@ export function debugSingle(ctx: Ctx): Cmd { } if (!debugEngine) { - vscode.window.showErrorMessage(`Install [CodeLLDB](https://marketplace.visualstudio.com/items?itemName=${lldbId})` - + ` or [MS C++ tools](https://marketplace.visualstudio.com/items?itemName=${cpptoolsId}) extension for debugging.`); + vscode.window.showErrorMessage( + `Install [CodeLLDB](https://marketplace.visualstudio.com/items?itemName=${lldbId}) ` + + `or [MS C++ tools](https://marketplace.visualstudio.com/items?itemName=${cpptoolsId}) ` + + `extension for debugging.` + ); return; } From c9b395be2bfcd67e045c1031143b7e8c27a6d3fb Mon Sep 17 00:00:00 2001 From: veetaha Date: Wed, 6 May 2020 01:42:04 +0300 Subject: [PATCH 12/46] Fix cargo not found on macos bug at vscode extension side --- editors/code/src/cargo.ts | 36 +++++++++++++++++++++++++++++++++++- editors/code/src/main.ts | 8 ++------ editors/code/src/util.ts | 11 +++++++++++ 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/editors/code/src/cargo.ts b/editors/code/src/cargo.ts index 613aa82da9a..2a2c2e0e1bb 100644 --- a/editors/code/src/cargo.ts +++ b/editors/code/src/cargo.ts @@ -1,6 +1,9 @@ import * as cp from 'child_process'; +import * as os from 'os'; +import * as path from 'path'; import * as readline from 'readline'; import { OutputChannel } from 'vscode'; +import { isValidExecutable } from './util'; interface CompilationArtifact { fileName: string; @@ -63,7 +66,14 @@ export class Cargo { onStderrString: (data: string) => void ): Promise { return new Promise((resolve, reject) => { - const cargo = cp.spawn('cargo', cargoArgs, { + let cargoPath; + try { + cargoPath = getCargoPathOrFail(); + } catch (err) { + return reject(err); + } + + const cargo = cp.spawn(cargoPath, cargoArgs, { stdio: ['ignore', 'pipe', 'pipe'], cwd: this.rootFolder }); @@ -87,3 +97,27 @@ export class Cargo { }); } } + +// Mirrors `ra_env::get_path_for_executable` implementation +function getCargoPathOrFail(): string { + const envVar = process.env.CARGO; + const executableName = "cargo"; + + if (envVar) { + if (isValidExecutable(envVar)) return envVar; + + throw new Error(`\`${envVar}\` environment variable points to something that's not a valid executable`); + } + + if (isValidExecutable(executableName)) return executableName; + + const standardLocation = path.join(os.homedir(), '.cargo', 'bin', executableName); + + if (isValidExecutable(standardLocation)) return standardLocation; + + throw new Error( + `Failed to find \`${executableName}\` executable. ` + + `Make sure \`${executableName}\` is in \`$PATH\`, ` + + `or set \`${envVar}\` to point to a valid executable.` + ); +} diff --git a/editors/code/src/main.ts b/editors/code/src/main.ts index efd56a84b52..9b020d0019a 100644 --- a/editors/code/src/main.ts +++ b/editors/code/src/main.ts @@ -8,10 +8,9 @@ import { activateInlayHints } from './inlay_hints'; import { activateStatusDisplay } from './status_display'; import { Ctx } from './ctx'; import { Config, NIGHTLY_TAG } from './config'; -import { log, assert } from './util'; +import { log, assert, isValidExecutable } from './util'; import { PersistentState } from './persistent_state'; import { fetchRelease, download } from './net'; -import { spawnSync } from 'child_process'; import { activateTaskProvider } from './tasks'; let ctx: Ctx | undefined; @@ -179,10 +178,7 @@ async function bootstrapServer(config: Config, state: PersistentState): Promise< log.debug("Using server binary at", path); - const res = spawnSync(path, ["--version"], { encoding: 'utf8' }); - log.debug("Checked binary availability via --version", res); - log.debug(res, "--version output:", res.output); - if (res.status !== 0) { + if (!isValidExecutable(path)) { throw new Error(`Failed to execute ${path} --version`); } diff --git a/editors/code/src/util.ts b/editors/code/src/util.ts index 6f91f81d63e..127a9e91124 100644 --- a/editors/code/src/util.ts +++ b/editors/code/src/util.ts @@ -1,6 +1,7 @@ import * as lc from "vscode-languageclient"; import * as vscode from "vscode"; import { strict as nativeAssert } from "assert"; +import { spawnSync } from "child_process"; export function assert(condition: boolean, explanation: string): asserts condition { try { @@ -82,3 +83,13 @@ export function isRustDocument(document: vscode.TextDocument): document is RustD export function isRustEditor(editor: vscode.TextEditor): editor is RustEditor { return isRustDocument(editor.document); } + +export function isValidExecutable(path: string): boolean { + log.debug("Checking availability of a binary at", path); + + const res = spawnSync(path, ["--version"], { encoding: 'utf8' }); + + log.debug(res, "--version output:", res.output); + + return res.status === 0; +} From 1b76b4281e90292922455a9192f82a2b6b80d279 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Tue, 5 May 2020 16:09:39 -0700 Subject: [PATCH 13/46] simplify some code using early returns --- crates/ra_env/src/lib.rs | 41 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/crates/ra_env/src/lib.rs b/crates/ra_env/src/lib.rs index c7b49e9979c..8d6aa926894 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_env/src/lib.rs @@ -33,31 +33,24 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result = if is_valid_executable(executable_name) { - Some(executable_name.to_owned()) - } else { - if let Some(mut path) = dirs::home_dir() { - path.push(".cargo"); - path.push("bin"); - path.push(executable_name); - if is_valid_executable(&path) { - Some(path.into_os_string().into_string().expect("Invalid Unicode in path")) - } else { - None - } - } else { - None + if is_valid_executable(executable_name) { + return Ok(executable_name.to_owned()); + } + if let Some(mut path) = dirs::home_dir() { + path.push(".cargo"); + path.push("bin"); + path.push(executable_name); + if is_valid_executable(&path) { + return Ok(path.into_os_string().into_string().expect("Invalid Unicode in path")); } - }; - final_path.ok_or( - // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly - // for VSCode, even if they are set correctly in a terminal. - // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode - // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; - // but launching VSCode from Dock does not inherit environment variables from a terminal. - // For more discussion, see #3118. - Error::msg(format!("Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", executable_name, executable_name, env_var)) - ) + } + // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly + // for VSCode, even if they are set correctly in a terminal. + // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode + // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; + // but launching VSCode from Dock does not inherit environment variables from a terminal. + // For more discussion, see #3118. + Err(Error::msg(format!("Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", executable_name, executable_name, env_var))) } } From 550c62949829c16861aefd6c1aea240709343ac8 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 6 May 2020 11:08:50 +0200 Subject: [PATCH 14/46] do not truncate display for hover Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_ide/src/hover.rs | 43 +++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs index df8451af258..d52f22157ff 100644 --- a/crates/ra_ide/src/hover.rs +++ b/crates/ra_ide/src/hover.rs @@ -143,7 +143,7 @@ fn hover_text_from_name_kind(db: &RootDatabase, def: Definition) -> Option from_def_source(db, it, mod_path), ModuleDef::BuiltinType(it) => Some(it.to_string()), }, - Definition::Local(it) => Some(rust_code_markup(&it.ty(db).display_truncated(db, None))), + Definition::Local(it) => Some(rust_code_markup(&it.ty(db).display(db))), Definition::TypeParam(_) | Definition::SelfType(_) => { // FIXME: Hover for generic param None @@ -279,6 +279,47 @@ mod tests { assert_eq!(trim_markup_opt(hover.info.first()), Some("u32")); } + #[test] + fn hover_shows_long_type_of_an_expression() { + check_hover_result( + r#" + //- /main.rs + struct Scan { + a: A, + b: B, + c: C, + } + + struct FakeIter { + inner: I, + } + + struct OtherStruct { + i: T, + } + + enum FakeOption { + Some(T), + None, + } + + fn scan(a: A, b: B, c: C) -> FakeIter, B, C>> { + FakeIter { inner: Scan { a, b, c } } + } + + fn main() { + let num: i32 = 55; + let closure = |memo: &mut u32, value: &u32, _another: &mut u32| -> FakeOption { + FakeOption::Some(*memo + value) + }; + let number = 5u32; + let mut iter<|> = scan(OtherStruct { i: num }, closure, number); + } + "#, + &["FakeIter>, |&mut u32, &u32, &mut u32| -> FakeOption, u32>>"], + ); + } + #[test] fn hover_shows_fn_signature() { // Single file with result From 1ec953f11744c708ba74c238737eac8e96a1c7b1 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 6 May 2020 11:33:43 +0200 Subject: [PATCH 15/46] do not truncate display for hover Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_ide/src/hover.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs index d52f22157ff..06d4f1c6395 100644 --- a/crates/ra_ide/src/hover.rs +++ b/crates/ra_ide/src/hover.rs @@ -446,7 +446,7 @@ mod tests { } #[test] - fn hover_omits_default_generic_types() { + fn hover_default_generic_types() { check_hover_result( r#" //- /main.rs @@ -458,7 +458,7 @@ struct Test { fn main() { let zz<|> = Test { t: 23, k: 33 }; }"#, - &["Test"], + &["Test"], ); } From fdd4df97ba5ce1f59abf9e945052fc6f3e077c3a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 15:26:40 +0200 Subject: [PATCH 16/46] Use SourceChange for assists --- crates/ra_assists/src/assist_ctx.rs | 30 ++++++++++++------- crates/ra_assists/src/lib.rs | 16 +++------- crates/ra_assists/src/tests.rs | 22 +++++++------- crates/ra_ide/src/assists.rs | 42 --------------------------- crates/ra_ide/src/lib.rs | 31 +++++++++++++++----- crates/ra_ide_db/src/source_change.rs | 6 ++-- 6 files changed, 60 insertions(+), 87 deletions(-) delete mode 100644 crates/ra_ide/src/assists.rs diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index cbf1963b721..77275156c80 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -1,8 +1,11 @@ //! This module defines `AssistCtx` -- the API surface that is exposed to assists. use hir::Semantics; -use ra_db::FileRange; +use ra_db::{FileId, FileRange}; use ra_fmt::{leading_indent, reindent}; -use ra_ide_db::RootDatabase; +use ra_ide_db::{ + source_change::{SingleFileChange, SourceChange}, + RootDatabase, +}; use ra_syntax::{ algo::{self, find_covering_element, find_node_at_offset, SyntaxRewriter}, AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize, @@ -10,7 +13,7 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; -use crate::{AssistAction, AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist}; +use crate::{AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist}; #[derive(Clone, Debug)] pub(crate) struct Assist(pub(crate) Vec); @@ -19,7 +22,7 @@ pub(crate) struct Assist(pub(crate) Vec); pub(crate) struct AssistInfo { pub(crate) label: AssistLabel, pub(crate) group_label: Option, - pub(crate) action: Option, + pub(crate) action: Option, } impl AssistInfo { @@ -27,7 +30,7 @@ impl AssistInfo { AssistInfo { label, group_label: None, action: None } } - fn resolved(self, action: AssistAction) -> AssistInfo { + fn resolved(self, action: SourceChange) -> AssistInfo { AssistInfo { action: Some(action), ..self } } @@ -98,13 +101,13 @@ impl<'a> AssistCtx<'a> { f: impl FnOnce(&mut ActionBuilder), ) -> Option { let label = AssistLabel::new(id, label.into(), None, target); - + let change_label = label.label.clone(); let mut info = AssistInfo::new(label); if self.should_compute_edit { let action = { let mut edit = ActionBuilder::new(&self); f(&mut edit); - edit.build() + edit.build(change_label, self.frange.file_id) }; info = info.resolved(action) }; @@ -157,13 +160,13 @@ impl<'a> AssistGroup<'a> { f: impl FnOnce(&mut ActionBuilder), ) { let label = AssistLabel::new(id, label.into(), Some(self.group.clone()), target); - + let change_label = label.label.clone(); let mut info = AssistInfo::new(label).with_group(self.group.clone()); if self.ctx.should_compute_edit { let action = { let mut edit = ActionBuilder::new(&self.ctx); f(&mut edit); - edit.build() + edit.build(change_label, self.ctx.frange.file_id) }; info = info.resolved(action) }; @@ -255,11 +258,16 @@ impl<'a, 'b> ActionBuilder<'a, 'b> { self.file = assist_file } - fn build(self) -> AssistAction { + fn build(self, change_label: String, current_file: FileId) -> SourceChange { let edit = self.edit.finish(); if edit.is_empty() && self.cursor_position.is_none() { panic!("Only call `add_assist` if the assist can be applied") } - AssistAction { edit, cursor_position: self.cursor_position, file: self.file } + let file = match self.file { + AssistFile::CurrentFile => current_file, + AssistFile::TargetFile(it) => it, + }; + SingleFileChange { label: change_label, edit, cursor_position: self.cursor_position } + .into_source_change(file) } } diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index f4f37614ffe..8cd8f89c42b 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -19,9 +19,8 @@ pub mod ast_transform; use hir::Semantics; use ra_db::{FileId, FileRange}; -use ra_ide_db::RootDatabase; -use ra_syntax::{TextRange, TextSize}; -use ra_text_edit::TextEdit; +use ra_ide_db::{source_change::SourceChange, RootDatabase}; +use ra_syntax::TextRange; pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; @@ -57,21 +56,14 @@ impl AssistLabel { } } -#[derive(Debug, Clone)] -pub struct AssistAction { - pub edit: TextEdit, - pub cursor_position: Option, - pub file: AssistFile, -} - #[derive(Debug, Clone)] pub struct ResolvedAssist { pub label: AssistLabel, - pub action: AssistAction, + pub action: SourceChange, } #[derive(Debug, Clone, Copy)] -pub enum AssistFile { +enum AssistFile { CurrentFile, TargetFile(FileId), } diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index dd9026df616..572462bfc47 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -11,7 +11,7 @@ use test_utils::{ RangeOrOffset, }; -use crate::{handlers::Handler, resolved_assists, AssistCtx, AssistFile}; +use crate::{handlers::Handler, resolved_assists, AssistCtx}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -41,7 +41,7 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { let (db, file_id) = crate::tests::with_single_file(&before); let frange = FileRange { file_id, range: selection.into() }; - let assist = resolved_assists(&db, frange) + let mut assist = resolved_assists(&db, frange) .into_iter() .find(|assist| assist.label.id.0 == assist_id) .unwrap_or_else(|| { @@ -57,8 +57,9 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { }); let actual = { + let change = assist.action.source_file_edits.pop().unwrap(); let mut actual = before.clone(); - assist.action.edit.apply(&mut actual); + change.edit.apply(&mut actual); actual }; assert_eq_text!(after, &actual); @@ -93,26 +94,23 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) { match (assist(assist_ctx), expected) { (Some(assist), ExpectedResult::After(after)) => { - let action = assist.0[0].action.clone().unwrap(); + let mut action = assist.0[0].action.clone().unwrap(); + let change = action.source_file_edits.pop().unwrap(); - let mut actual = if let AssistFile::TargetFile(file_id) = action.file { - db.file_text(file_id).as_ref().to_owned() - } else { - text_without_caret - }; - action.edit.apply(&mut actual); + let mut actual = db.file_text(change.file_id).as_ref().to_owned(); + change.edit.apply(&mut actual); match action.cursor_position { None => { if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset { - let off = action + let off = change .edit .apply_to_offset(before_cursor_pos) .expect("cursor position is affected by the edit"); actual = add_cursor(&actual, off) } } - Some(off) => actual = add_cursor(&actual, off), + Some(off) => actual = add_cursor(&actual, off.offset), }; assert_eq_text!(after, &actual); diff --git a/crates/ra_ide/src/assists.rs b/crates/ra_ide/src/assists.rs deleted file mode 100644 index 389339a0344..00000000000 --- a/crates/ra_ide/src/assists.rs +++ /dev/null @@ -1,42 +0,0 @@ -//! FIXME: write short doc here - -use ra_assists::{resolved_assists, AssistAction}; -use ra_db::{FilePosition, FileRange}; -use ra_ide_db::RootDatabase; - -use crate::{FileId, SourceChange, SourceFileEdit}; - -pub use ra_assists::AssistId; - -#[derive(Debug)] -pub struct Assist { - pub id: AssistId, - pub label: String, - pub group_label: Option, - pub source_change: SourceChange, -} - -pub(crate) fn assists(db: &RootDatabase, frange: FileRange) -> Vec { - resolved_assists(db, frange) - .into_iter() - .map(|assist| { - let file_id = frange.file_id; - Assist { - id: assist.label.id, - label: assist.label.label.clone(), - group_label: assist.label.group.map(|it| it.0), - source_change: action_to_edit(assist.action, file_id, assist.label.label.clone()), - } - }) - .collect() -} - -fn action_to_edit(action: AssistAction, file_id: FileId, label: String) -> SourceChange { - let file_id = match action.file { - ra_assists::AssistFile::TargetFile(it) => it, - _ => file_id, - }; - let file_edit = SourceFileEdit { file_id, edit: action.edit }; - SourceChange::source_file_edit(label, file_edit) - .with_cursor_opt(action.cursor_position.map(|offset| FilePosition { offset, file_id })) -} diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 4ed02f60efb..614029de46a 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -31,7 +31,6 @@ mod syntax_highlighting; mod parent_module; mod references; mod impls; -mod assists; mod diagnostics; mod syntax_tree; mod folding_ranges; @@ -64,7 +63,6 @@ use ra_syntax::{SourceFile, TextRange, TextSize}; use crate::display::ToNav; pub use crate::{ - assists::{Assist, AssistId}, call_hierarchy::CallItem, completion::{ CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat, @@ -84,6 +82,7 @@ pub use crate::{ }; pub use hir::Documentation; +pub use ra_assists::AssistId; pub use ra_db::{ Canceled, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRootId, }; @@ -134,10 +133,12 @@ pub struct AnalysisHost { db: RootDatabase, } -impl Default for AnalysisHost { - fn default() -> AnalysisHost { - AnalysisHost::new(None) - } +#[derive(Debug)] +pub struct Assist { + pub id: AssistId, + pub label: String, + pub group_label: Option, + pub source_change: SourceChange, } impl AnalysisHost { @@ -187,6 +188,12 @@ impl AnalysisHost { } } +impl Default for AnalysisHost { + fn default() -> AnalysisHost { + AnalysisHost::new(None) + } +} + /// Analysis is a snapshot of a world state at a moment in time. It is the main /// entry point for asking semantic information about the world. When the world /// state is advanced using `AnalysisHost::apply_change` method, all existing @@ -464,7 +471,17 @@ impl Analysis { /// Computes assists (aka code actions aka intentions) for the given /// position. pub fn assists(&self, frange: FileRange) -> Cancelable> { - self.with_db(|db| assists::assists(db, frange)) + self.with_db(|db| { + ra_assists::resolved_assists(db, frange) + .into_iter() + .map(|assist| Assist { + id: assist.label.id, + label: assist.label.label, + group_label: assist.label.group.map(|it| it.0), + source_change: assist.action, + }) + .collect() + }) } /// Computes the set of diagnostics for the given file. diff --git a/crates/ra_ide_db/src/source_change.rs b/crates/ra_ide_db/src/source_change.rs index 4dd01b31220..af81a91a4a5 100644 --- a/crates/ra_ide_db/src/source_change.rs +++ b/crates/ra_ide_db/src/source_change.rs @@ -6,7 +6,7 @@ use ra_db::{FileId, FilePosition, RelativePathBuf, SourceRootId}; use ra_text_edit::{TextEdit, TextSize}; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct SourceChange { /// For display in the undo log in the editor pub label: String, @@ -90,13 +90,13 @@ impl SourceChange { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct SourceFileEdit { pub file_id: FileId, pub edit: TextEdit, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum FileSystemEdit { CreateFile { source_root: SourceRootId, path: RelativePathBuf }, MoveFile { src: FileId, dst_source_root: SourceRootId, dst_path: RelativePathBuf }, From 0970c3454baa0164df81a9ff665430d486687331 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 16:01:47 +0200 Subject: [PATCH 17/46] Rename --- crates/ra_assists/src/assist_ctx.rs | 18 +++++++++--------- crates/ra_assists/src/lib.rs | 2 +- crates/ra_assists/src/tests.rs | 4 ++-- crates/ra_ide/src/lib.rs | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 77275156c80..af6756d17bd 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -22,16 +22,16 @@ pub(crate) struct Assist(pub(crate) Vec); pub(crate) struct AssistInfo { pub(crate) label: AssistLabel, pub(crate) group_label: Option, - pub(crate) action: Option, + pub(crate) source_change: Option, } impl AssistInfo { fn new(label: AssistLabel) -> AssistInfo { - AssistInfo { label, group_label: None, action: None } + AssistInfo { label, group_label: None, source_change: None } } - fn resolved(self, action: SourceChange) -> AssistInfo { - AssistInfo { action: Some(action), ..self } + fn resolved(self, source_change: SourceChange) -> AssistInfo { + AssistInfo { source_change: Some(source_change), ..self } } fn with_group(self, group_label: GroupLabel) -> AssistInfo { @@ -40,7 +40,7 @@ impl AssistInfo { pub(crate) fn into_resolved(self) -> Option { let label = self.label; - self.action.map(|action| ResolvedAssist { label, action }) + self.source_change.map(|source_change| ResolvedAssist { label, source_change }) } } @@ -104,12 +104,12 @@ impl<'a> AssistCtx<'a> { let change_label = label.label.clone(); let mut info = AssistInfo::new(label); if self.should_compute_edit { - let action = { + let source_change = { let mut edit = ActionBuilder::new(&self); f(&mut edit); edit.build(change_label, self.frange.file_id) }; - info = info.resolved(action) + info = info.resolved(source_change) }; Some(Assist(vec![info])) @@ -163,12 +163,12 @@ impl<'a> AssistGroup<'a> { let change_label = label.label.clone(); let mut info = AssistInfo::new(label).with_group(self.group.clone()); if self.ctx.should_compute_edit { - let action = { + let source_change = { let mut edit = ActionBuilder::new(&self.ctx); f(&mut edit); edit.build(change_label, self.ctx.frange.file_id) }; - info = info.resolved(action) + info = info.resolved(source_change) }; self.assists.push(info) diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 8cd8f89c42b..6e81ea8eec5 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -59,7 +59,7 @@ impl AssistLabel { #[derive(Debug, Clone)] pub struct ResolvedAssist { pub label: AssistLabel, - pub action: SourceChange, + pub source_change: SourceChange, } #[derive(Debug, Clone, Copy)] diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index 572462bfc47..17e3ece9f60 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -57,7 +57,7 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { }); let actual = { - let change = assist.action.source_file_edits.pop().unwrap(); + let change = assist.source_change.source_file_edits.pop().unwrap(); let mut actual = before.clone(); change.edit.apply(&mut actual); actual @@ -94,7 +94,7 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) { match (assist(assist_ctx), expected) { (Some(assist), ExpectedResult::After(after)) => { - let mut action = assist.0[0].action.clone().unwrap(); + let mut action = assist.0[0].source_change.clone().unwrap(); let change = action.source_file_edits.pop().unwrap(); let mut actual = db.file_text(change.file_id).as_ref().to_owned(); diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 614029de46a..737f8710919 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -478,7 +478,7 @@ impl Analysis { id: assist.label.id, label: assist.label.label, group_label: assist.label.group.map(|it| it.0), - source_change: assist.action, + source_change: assist.source_change, }) .collect() }) From 4d50709a96f92f3927b3ac59110d593b49c53008 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 16:05:43 +0200 Subject: [PATCH 18/46] Minor --- crates/ra_ide/src/completion/completion_item.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ra_ide/src/completion/completion_item.rs b/crates/ra_ide/src/completion/completion_item.rs index 383b23ac44e..6021f7279f6 100644 --- a/crates/ra_ide/src/completion/completion_item.rs +++ b/crates/ra_ide/src/completion/completion_item.rs @@ -2,11 +2,12 @@ use std::fmt; -use super::completion_config::SnippetCap; use hir::Documentation; use ra_syntax::TextRange; use ra_text_edit::TextEdit; +use crate::completion::completion_config::SnippetCap; + /// `CompletionItem` describes a single completion variant in the editor pop-up. /// It is basically a POD with various properties. To construct a /// `CompletionItem`, use `new` method and the `Builder` struct. From bd9f1f7eb78843ddd91d259a04e988b0681a5db4 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Wed, 6 May 2020 17:17:35 +0300 Subject: [PATCH 19/46] Fix rename of enum variant visible from module --- crates/ra_hir/src/code_model.rs | 7 +++ crates/ra_hir_def/src/adt.rs | 4 +- crates/ra_ide/src/references/rename.rs | 62 ++++++++++++++++++++++++++ crates/ra_ide_db/src/defs.rs | 5 ++- 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 5f480c3040d..7eba0b23363 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -474,6 +474,13 @@ impl EnumVariant { } } +impl HasVisibility for EnumVariant { + fn visibility(&self, db: &dyn HirDatabase) -> Visibility { + let visibility = &db.enum_data(self.parent.id).visibility; + visibility.resolve(db.upcast(), &self.parent.id.resolver(db.upcast())) + } +} + /// A Data Type #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Adt { diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 2bc34d449f2..0fda4d6c975 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -33,6 +33,7 @@ pub struct StructData { #[derive(Debug, Clone, PartialEq, Eq)] pub struct EnumData { pub name: Name, + pub visibility: RawVisibility, pub variants: Arena, } @@ -91,7 +92,8 @@ impl EnumData { let name = src.value.name().map_or_else(Name::missing, |n| n.as_name()); let mut trace = Trace::new_for_arena(); lower_enum(db, &mut trace, &src, e.lookup(db).container.module(db)); - Arc::new(EnumData { name, variants: trace.into_arena() }) + let visibility = RawVisibility::from_ast(db, src.with_value(src.value.visibility())); + Arc::new(EnumData { name, visibility, variants: trace.into_arena() }) } pub fn variant(&self, name: &Name) -> Option { diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs index 0398d53bc92..2cbb82c1a49 100644 --- a/crates/ra_ide/src/references/rename.rs +++ b/crates/ra_ide/src/references/rename.rs @@ -712,6 +712,68 @@ mod tests { "###); } + #[test] + fn test_enum_variant_from_module_1() { + test_rename( + r#" + mod foo { + pub enum Foo { + Bar<|>, + } + } + + fn func(f: foo::Foo) { + match f { + foo::Foo::Bar => {} + } + } + "#, + "Baz", + r#" + mod foo { + pub enum Foo { + Baz, + } + } + + fn func(f: foo::Foo) { + match f { + foo::Foo::Baz => {} + } + } + "#, + ); + } + + #[test] + fn test_enum_variant_from_module_2() { + test_rename( + r#" + mod foo { + pub struct Foo { + pub bar<|>: uint, + } + } + + fn foo(f: foo::Foo) { + let _ = f.bar; + } + "#, + "baz", + r#" + mod foo { + pub struct Foo { + pub baz: uint, + } + } + + fn foo(f: foo::Foo) { + let _ = f.baz; + } + "#, + ); + } + fn test_rename(text: &str, new_name: &str, expected: &str) { let (analysis, position) = single_file_with_position(text); let source_change = analysis.rename(position, new_name).unwrap(); diff --git a/crates/ra_ide_db/src/defs.rs b/crates/ra_ide_db/src/defs.rs index 40d0e77b5f4..2e2850efbd7 100644 --- a/crates/ra_ide_db/src/defs.rs +++ b/crates/ra_ide_db/src/defs.rs @@ -47,7 +47,10 @@ impl Definition { match self { Definition::Macro(_) => None, Definition::Field(sf) => Some(sf.visibility(db)), - Definition::ModuleDef(def) => module?.visibility_of(db, def), + Definition::ModuleDef(def) => match def { + ModuleDef::EnumVariant(id) => Some(id.visibility(db)), + _ => module?.visibility_of(db, def), + }, Definition::SelfType(_) => None, Definition::Local(_) => None, Definition::TypeParam(_) => None, From 020ca6695f4d58f651984c4fbe2227d891896bb3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 16:39:11 +0200 Subject: [PATCH 20/46] Simplify --- crates/ra_assists/src/assist_ctx.rs | 22 ++++++++----------- .../ra_assists/src/handlers/add_function.rs | 14 ++++++------ crates/ra_assists/src/lib.rs | 14 +----------- 3 files changed, 17 insertions(+), 33 deletions(-) diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index af6756d17bd..871671de2bf 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -13,7 +13,7 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; -use crate::{AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist}; +use crate::{AssistId, AssistLabel, GroupLabel, ResolvedAssist}; #[derive(Clone, Debug)] pub(crate) struct Assist(pub(crate) Vec); @@ -107,7 +107,7 @@ impl<'a> AssistCtx<'a> { let source_change = { let mut edit = ActionBuilder::new(&self); f(&mut edit); - edit.build(change_label, self.frange.file_id) + edit.build(change_label) }; info = info.resolved(source_change) }; @@ -166,7 +166,7 @@ impl<'a> AssistGroup<'a> { let source_change = { let mut edit = ActionBuilder::new(&self.ctx); f(&mut edit); - edit.build(change_label, self.ctx.frange.file_id) + edit.build(change_label) }; info = info.resolved(source_change) }; @@ -186,7 +186,7 @@ impl<'a> AssistGroup<'a> { pub(crate) struct ActionBuilder<'a, 'b> { edit: TextEditBuilder, cursor_position: Option, - file: AssistFile, + file: FileId, ctx: &'a AssistCtx<'b>, } @@ -195,7 +195,7 @@ impl<'a, 'b> ActionBuilder<'a, 'b> { Self { edit: TextEditBuilder::default(), cursor_position: None, - file: AssistFile::default(), + file: ctx.frange.file_id, ctx, } } @@ -254,20 +254,16 @@ impl<'a, 'b> ActionBuilder<'a, 'b> { algo::diff(&node, &new).into_text_edit(&mut self.edit) } - pub(crate) fn set_file(&mut self, assist_file: AssistFile) { - self.file = assist_file + pub(crate) fn set_file(&mut self, assist_file: FileId) { + self.file = assist_file; } - fn build(self, change_label: String, current_file: FileId) -> SourceChange { + fn build(self, change_label: String) -> SourceChange { let edit = self.edit.finish(); if edit.is_empty() && self.cursor_position.is_none() { panic!("Only call `add_assist` if the assist can be applied") } - let file = match self.file { - AssistFile::CurrentFile => current_file, - AssistFile::TargetFile(it) => it, - }; SingleFileChange { label: change_label, edit, cursor_position: self.cursor_position } - .into_source_change(file) + .into_source_change(self.file) } } diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index 1d9d4e638fc..27807966506 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -3,9 +3,10 @@ use ra_syntax::{ SyntaxKind, SyntaxNode, TextSize, }; -use crate::{Assist, AssistCtx, AssistFile, AssistId}; +use crate::{Assist, AssistCtx, AssistId}; use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner}; use hir::HirDisplay; +use ra_db::FileId; use rustc_hash::{FxHashMap, FxHashSet}; // Assist: add_function @@ -70,7 +71,7 @@ struct FunctionTemplate { insert_offset: TextSize, cursor_offset: TextSize, fn_def: ast::SourceFile, - file: AssistFile, + file: FileId, } struct FunctionBuilder { @@ -78,7 +79,7 @@ struct FunctionBuilder { fn_name: ast::Name, type_params: Option, params: ast::ParamList, - file: AssistFile, + file: FileId, needs_pub: bool, } @@ -92,7 +93,7 @@ impl FunctionBuilder { target_module: Option>, ) -> Option { let needs_pub = target_module.is_some(); - let mut file = AssistFile::default(); + let mut file = ctx.frange.file_id; let target = if let Some(target_module) = target_module { let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?; file = in_file; @@ -253,9 +254,8 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option, -) -> Option<(AssistFile, GeneratedFunctionTarget)> { +) -> Option<(FileId, GeneratedFunctionTarget)> { let file = module.file_id.original_file(db); - let assist_file = AssistFile::TargetFile(file); let assist_item = match module.value { hir::ModuleSource::SourceFile(it) => { if let Some(last_item) = it.items().last() { @@ -272,7 +272,7 @@ fn next_space_for_fn_in_module( } } }; - Some((assist_file, assist_item)) + Some((file, assist_item)) } #[cfg(test)] diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 6e81ea8eec5..13ea45ec7cc 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -18,7 +18,7 @@ pub mod utils; pub mod ast_transform; use hir::Semantics; -use ra_db::{FileId, FileRange}; +use ra_db::FileRange; use ra_ide_db::{source_change::SourceChange, RootDatabase}; use ra_syntax::TextRange; @@ -62,18 +62,6 @@ pub struct ResolvedAssist { pub source_change: SourceChange, } -#[derive(Debug, Clone, Copy)] -enum AssistFile { - CurrentFile, - TargetFile(FileId), -} - -impl Default for AssistFile { - fn default() -> Self { - Self::CurrentFile - } -} - /// Return all the assists applicable at the given position. /// /// Assists are returned in the "unresolved" state, that is only labels are From 7c94fa7d01f264e5268ab5f57519f071c00a6579 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 6 May 2020 17:45:47 +0300 Subject: [PATCH 21/46] Fix usefulness check for never type --- crates/ra_hir_ty/src/_match.rs | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 779e7857458..149f6504241 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -573,14 +573,20 @@ pub(crate) fn is_useful( matrix: &Matrix, v: &PatStack, ) -> MatchCheckResult { - // Handle the special case of enums with no variants. In that case, no match - // arm is useful. - if let Ty::Apply(ApplicationTy { ctor: TypeCtor::Adt(AdtId::EnumId(enum_id)), .. }) = - cx.infer[cx.match_expr].strip_references() - { - if cx.db.enum_data(*enum_id).variants.is_empty() { + // Handle two special cases: + // - enum with no variants + // - `!` type + // In those cases, no match arm is useful. + match cx.infer[cx.match_expr].strip_references() { + Ty::Apply(ApplicationTy { ctor: TypeCtor::Adt(AdtId::EnumId(enum_id)), .. }) => { + if cx.db.enum_data(*enum_id).variants.is_empty() { + return Ok(Usefulness::NotUseful); + } + } + Ty::Apply(ApplicationTy { ctor: TypeCtor::Never, .. }) => { return Ok(Usefulness::NotUseful); } + _ => (), } if v.is_empty() { @@ -1917,6 +1923,17 @@ mod tests { check_no_diagnostic(content); } + #[test] + fn type_never() { + let content = r" + fn test_fn(never: !) { + match never {} + } + "; + + check_no_diagnostic(content); + } + #[test] fn enum_never_ref() { let content = r" From 9a4553b833e8fa57a361d934c3498efa485d27c9 Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Wed, 6 May 2020 11:22:24 -0400 Subject: [PATCH 22/46] package.json: Minor configuration spelling fix --- editors/code/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editors/code/package.json b/editors/code/package.json index eeb3d3513fe..8849615c880 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -405,7 +405,7 @@ "ms-vscode.cpptools" ], "default": "auto", - "description": "Preffered debug engine.", + "description": "Preferred debug engine.", "markdownEnumDescriptions": [ "First try to use [CodeLLDB](https://marketplace.visualstudio.com/items?itemName=vadimcn.vscode-lldb), if it's not installed try to use [MS C++ tools](https://marketplace.visualstudio.com/items?itemName=ms-vscode.cpptools).", "Use [CodeLLDB](https://marketplace.visualstudio.com/items?itemName=vadimcn.vscode-lldb)", From 51c02ab84f6b88ba39e2d0a3ed22bea51114b05a Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 5 May 2020 19:02:45 +0200 Subject: [PATCH 23/46] add Ok wrapping Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../handlers/change_return_type_to_result.rs | 971 ++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 + crates/ra_assists/src/tests/generated.rs | 13 + docs/user/assists.md | 12 + 4 files changed, 998 insertions(+) create mode 100644 crates/ra_assists/src/handlers/change_return_type_to_result.rs diff --git a/crates/ra_assists/src/handlers/change_return_type_to_result.rs b/crates/ra_assists/src/handlers/change_return_type_to_result.rs new file mode 100644 index 00000000000..1e8d986cdbc --- /dev/null +++ b/crates/ra_assists/src/handlers/change_return_type_to_result.rs @@ -0,0 +1,971 @@ +use ra_syntax::{ + ast, AstNode, + SyntaxKind::{COMMENT, WHITESPACE}, + SyntaxNode, TextSize, +}; + +use crate::{Assist, AssistCtx, AssistId}; +use ast::{BlockExpr, Expr, LoopBodyOwner}; + +// Assist: change_return_type_to_result +// +// Change the function's return type to Result. +// +// ``` +// fn foo() -> i32<|> { 42i32 } +// ``` +// -> +// ``` +// fn foo() -> Result { Ok(42i32) } +// ``` +pub(crate) fn change_return_type_to_result(ctx: AssistCtx) -> Option { + let fn_def = ctx.find_node_at_offset::(); + let fn_def = &mut fn_def?; + let ret_type = &fn_def.ret_type()?.type_ref()?; + if ret_type.syntax().text().to_string().starts_with("Result<") { + return None; + } + + let block_expr = &fn_def.body()?; + let cursor_in_ret_type = + fn_def.ret_type()?.syntax().text_range().contains_range(ctx.frange.range); + if !cursor_in_ret_type { + return None; + } + + ctx.add_assist( + AssistId("change_return_type_to_result"), + "Change return type to Result", + ret_type.syntax().text_range(), + |edit| { + let mut tail_return_expr_collector = TailReturnCollector::new(); + tail_return_expr_collector.collect_jump_exprs(block_expr, false); + tail_return_expr_collector.collect_tail_exprs(block_expr); + + for ret_expr_arg in tail_return_expr_collector.exprs_to_wrap { + edit.replace_node_and_indent(&ret_expr_arg, format!("Ok({})", ret_expr_arg)); + } + edit.replace_node_and_indent(ret_type.syntax(), format!("Result<{}, >", ret_type)); + + if let Some(node_start) = result_insertion_offset(&ret_type) { + edit.set_cursor(node_start + TextSize::of(&format!("Result<{}, ", ret_type))); + } + }, + ) +} + +struct TailReturnCollector { + exprs_to_wrap: Vec, +} + +impl TailReturnCollector { + fn new() -> Self { + Self { exprs_to_wrap: vec![] } + } + /// Collect all`return` expression + fn collect_jump_exprs(&mut self, block_expr: &BlockExpr, collect_break: bool) { + let statements = block_expr.statements(); + for stmt in statements { + let expr = match &stmt { + ast::Stmt::ExprStmt(stmt) => stmt.expr(), + ast::Stmt::LetStmt(stmt) => stmt.initializer(), + }; + if let Some(expr) = &expr { + self.handle_exprs(expr, collect_break); + } + } + + // Browse tail expressions for each block + if let Some(expr) = block_expr.expr() { + if let Some(last_exprs) = get_tail_expr_from_block(&expr) { + for last_expr in last_exprs { + let last_expr = match last_expr { + NodeType::Node(expr) | NodeType::Leaf(expr) => expr, + }; + + if let Some(last_expr) = Expr::cast(last_expr.clone()) { + self.handle_exprs(&last_expr, collect_break); + } else if let Some(expr_stmt) = ast::Stmt::cast(last_expr) { + let expr_stmt = match &expr_stmt { + ast::Stmt::ExprStmt(stmt) => stmt.expr(), + ast::Stmt::LetStmt(stmt) => stmt.initializer(), + }; + if let Some(expr) = &expr_stmt { + self.handle_exprs(expr, collect_break); + } + } + } + } + } + } + + fn handle_exprs(&mut self, expr: &Expr, collect_break: bool) { + match expr { + Expr::BlockExpr(block_expr) => { + self.collect_jump_exprs(&block_expr, collect_break); + } + Expr::ReturnExpr(ret_expr) => { + if let Some(ret_expr_arg) = &ret_expr.expr() { + self.exprs_to_wrap.push(ret_expr_arg.syntax().clone()); + } + } + Expr::BreakExpr(break_expr) if collect_break => { + if let Some(break_expr_arg) = &break_expr.expr() { + self.exprs_to_wrap.push(break_expr_arg.syntax().clone()); + } + } + Expr::IfExpr(if_expr) => { + for block in if_expr.blocks() { + self.collect_jump_exprs(&block, collect_break); + } + } + Expr::LoopExpr(loop_expr) => { + if let Some(block_expr) = loop_expr.loop_body() { + self.collect_jump_exprs(&block_expr, collect_break); + } + } + Expr::ForExpr(for_expr) => { + if let Some(block_expr) = for_expr.loop_body() { + self.collect_jump_exprs(&block_expr, collect_break); + } + } + Expr::WhileExpr(while_expr) => { + if let Some(block_expr) = while_expr.loop_body() { + self.collect_jump_exprs(&block_expr, collect_break); + } + } + Expr::MatchExpr(match_expr) => { + if let Some(arm_list) = match_expr.match_arm_list() { + arm_list.arms().filter_map(|match_arm| match_arm.expr()).for_each(|expr| { + self.handle_exprs(&expr, collect_break); + }); + } + } + _ => {} + } + } + + fn collect_tail_exprs(&mut self, block: &BlockExpr) { + if let Some(expr) = block.expr() { + self.handle_exprs(&expr, true); + self.fetch_tail_exprs(&expr); + } + } + + fn fetch_tail_exprs(&mut self, expr: &Expr) { + if let Some(exprs) = get_tail_expr_from_block(expr) { + for node_type in &exprs { + match node_type { + NodeType::Leaf(expr) => { + self.exprs_to_wrap.push(expr.clone()); + } + NodeType::Node(expr) => match &Expr::cast(expr.clone()) { + Some(last_expr) => { + self.fetch_tail_exprs(last_expr); + } + None => { + self.exprs_to_wrap.push(expr.clone()); + } + }, + } + } + } + } +} + +#[derive(Debug)] +enum NodeType { + Leaf(SyntaxNode), + Node(SyntaxNode), +} + +/// Get a tail expression inside a block +fn get_tail_expr_from_block(expr: &Expr) -> Option> { + match expr { + Expr::IfExpr(if_expr) => { + let mut nodes = vec![]; + for block in if_expr.blocks() { + if let Some(block_expr) = block.expr() { + if let Some(tail_exprs) = get_tail_expr_from_block(&block_expr) { + nodes.extend(tail_exprs); + } + } else if let Some(last_expr) = block.syntax().last_child() { + nodes.push(NodeType::Node(last_expr)); + } else { + nodes.push(NodeType::Node(block.syntax().clone())); + } + } + Some(nodes) + } + Expr::LoopExpr(loop_expr) => { + loop_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) + } + Expr::ForExpr(for_expr) => { + for_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) + } + Expr::WhileExpr(while_expr) => { + while_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) + } + Expr::BlockExpr(block_expr) => { + block_expr.expr().map(|lc| vec![NodeType::Node(lc.syntax().clone())]) + } + Expr::MatchExpr(match_expr) => { + let arm_list = match_expr.match_arm_list()?; + let arms: Vec = arm_list + .arms() + .filter_map(|match_arm| match_arm.expr()) + .map(|expr| match expr { + Expr::ReturnExpr(ret_expr) => NodeType::Node(ret_expr.syntax().clone()), + Expr::BreakExpr(break_expr) => NodeType::Node(break_expr.syntax().clone()), + _ => match expr.syntax().last_child() { + Some(last_expr) => NodeType::Node(last_expr), + None => NodeType::Node(expr.syntax().clone()), + }, + }) + .collect(); + + Some(arms) + } + Expr::BreakExpr(expr) => expr.expr().map(|e| vec![NodeType::Leaf(e.syntax().clone())]), + Expr::ReturnExpr(ret_expr) => Some(vec![NodeType::Node(ret_expr.syntax().clone())]), + Expr::CallExpr(call_expr) => Some(vec![NodeType::Leaf(call_expr.syntax().clone())]), + Expr::Literal(lit_expr) => Some(vec![NodeType::Leaf(lit_expr.syntax().clone())]), + Expr::TupleExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::ArrayExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::ParenExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::PathExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::Label(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::RecordLit(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::IndexExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::MethodCallExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::AwaitExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::CastExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::RefExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::PrefixExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::RangeExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::BinExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::MacroCall(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + Expr::BoxExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]), + _ => None, + } +} + +fn result_insertion_offset(ret_type: &ast::TypeRef) -> Option { + let non_ws_child = ret_type + .syntax() + .children_with_tokens() + .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?; + Some(non_ws_child.text_range().start()) +} + +#[cfg(test)] +mod tests { + + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn change_return_type_to_result_simple() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i3<|>2 { + let test = "test"; + return 42i32; + }"#, + r#"fn foo() -> Result> { + let test = "test"; + return Ok(42i32); + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_return_type() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let test = "test"; + return 42i32; + }"#, + r#"fn foo() -> Result> { + let test = "test"; + return Ok(42i32); + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_return_type_bad_cursor() { + check_assist_not_applicable( + change_return_type_to_result, + r#"fn foo() -> i32 { + let test = "test";<|> + return 42i32; + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_cursor() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> <|>i32 { + let test = "test"; + return 42i32; + }"#, + r#"fn foo() -> Result> { + let test = "test"; + return Ok(42i32); + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_tail() { + check_assist( + change_return_type_to_result, + r#"fn foo() -><|> i32 { + let test = "test"; + 42i32 + }"#, + r#"fn foo() -> Result> { + let test = "test"; + Ok(42i32) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_tail_only() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + 42i32 + }"#, + r#"fn foo() -> Result> { + Ok(42i32) + }"#, + ); + } + #[test] + fn change_return_type_to_result_simple_with_tail_block_like() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + if true { + 42i32 + } else { + 24i32 + } + }"#, + r#"fn foo() -> Result> { + if true { + Ok(42i32) + } else { + Ok(24i32) + } + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_nested_if() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + if true { + if false { + 1 + } else { + 2 + } + } else { + 24i32 + } + }"#, + r#"fn foo() -> Result> { + if true { + if false { + Ok(1) + } else { + Ok(2) + } + } else { + Ok(24i32) + } + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_await() { + check_assist( + change_return_type_to_result, + r#"async fn foo() -> i<|>32 { + if true { + if false { + 1.await + } else { + 2.await + } + } else { + 24i32.await + } + }"#, + r#"async fn foo() -> Result> { + if true { + if false { + Ok(1.await) + } else { + Ok(2.await) + } + } else { + Ok(24i32.await) + } + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_array() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> [i32;<|> 3] { + [1, 2, 3] + }"#, + r#"fn foo() -> Result<[i32; 3], <|>> { + Ok([1, 2, 3]) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_cast() { + check_assist( + change_return_type_to_result, + r#"fn foo() -<|>> i32 { + if true { + if false { + 1 as i32 + } else { + 2 as i32 + } + } else { + 24 as i32 + } + }"#, + r#"fn foo() -> Result> { + if true { + if false { + Ok(1 as i32) + } else { + Ok(2 as i32) + } + } else { + Ok(24 as i32) + } + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_tail_block_like_match() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let my_var = 5; + match my_var { + 5 => 42i32, + _ => 24i32, + } + }"#, + r#"fn foo() -> Result> { + let my_var = 5; + match my_var { + 5 => Ok(42i32), + _ => Ok(24i32), + } + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_loop_with_tail() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let my_var = 5; + loop { + println!("test"); + 5 + } + + my_var + }"#, + r#"fn foo() -> Result> { + let my_var = 5; + loop { + println!("test"); + 5 + } + + Ok(my_var) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_loop_in_let_stmt() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let my_var = let x = loop { + break 1; + }; + + my_var + }"#, + r#"fn foo() -> Result> { + let my_var = let x = loop { + break 1; + }; + + Ok(my_var) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_tail_block_like_match_return_expr() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let my_var = 5; + let res = match my_var { + 5 => 42i32, + _ => return 24i32, + }; + + res + }"#, + r#"fn foo() -> Result> { + let my_var = 5; + let res = match my_var { + 5 => 42i32, + _ => return Ok(24i32), + }; + + Ok(res) + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let my_var = 5; + let res = if my_var == 5 { + 42i32 + } else { + return 24i32; + }; + + res + }"#, + r#"fn foo() -> Result> { + let my_var = 5; + let res = if my_var == 5 { + 42i32 + } else { + return Ok(24i32); + }; + + Ok(res) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_tail_block_like_match_deeper() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let my_var = 5; + match my_var { + 5 => { + if true { + 42i32 + } else { + 25i32 + } + }, + _ => { + let test = "test"; + if test == "test" { + return bar(); + } + 53i32 + }, + } + }"#, + r#"fn foo() -> Result> { + let my_var = 5; + match my_var { + 5 => { + if true { + Ok(42i32) + } else { + Ok(25i32) + } + }, + _ => { + let test = "test"; + if test == "test" { + return Ok(bar()); + } + Ok(53i32) + }, + } + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_tail_block_like_early_return() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i<|>32 { + let test = "test"; + if test == "test" { + return 24i32; + } + 53i32 + }"#, + r#"fn foo() -> Result> { + let test = "test"; + if test == "test" { + return Ok(24i32); + } + Ok(53i32) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_closure() { + check_assist( + change_return_type_to_result, + r#"fn foo(the_field: u32) -><|> u32 { + let true_closure = || { + return true; + }; + if the_field < 5 { + let mut i = 0; + + + if true_closure() { + return 99; + } else { + return 0; + } + } + + the_field + }"#, + r#"fn foo(the_field: u32) -> Result> { + let true_closure = || { + return true; + }; + if the_field < 5 { + let mut i = 0; + + + if true_closure() { + return Ok(99); + } else { + return Ok(0); + } + } + + Ok(the_field) + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo(the_field: u32) -> u32<|> { + let true_closure = || { + return true; + }; + if the_field < 5 { + let mut i = 0; + + + if true_closure() { + return 99; + } else { + return 0; + } + } + let t = None; + + t.unwrap_or_else(|| the_field) + }"#, + r#"fn foo(the_field: u32) -> Result> { + let true_closure = || { + return true; + }; + if the_field < 5 { + let mut i = 0; + + + if true_closure() { + return Ok(99); + } else { + return Ok(0); + } + } + let t = None; + + Ok(t.unwrap_or_else(|| the_field)) + }"#, + ); + } + + #[test] + fn change_return_type_to_result_simple_with_weird_forms() { + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let test = "test"; + if test == "test" { + return 24i32; + } + let mut i = 0; + loop { + if i == 1 { + break 55; + } + i += 1; + } + }"#, + r#"fn foo() -> Result> { + let test = "test"; + if test == "test" { + return Ok(24i32); + } + let mut i = 0; + loop { + if i == 1 { + break Ok(55); + } + i += 1; + } + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo() -> i32<|> { + let test = "test"; + if test == "test" { + return 24i32; + } + let mut i = 0; + loop { + loop { + if i == 1 { + break 55; + } + i += 1; + } + } + }"#, + r#"fn foo() -> Result> { + let test = "test"; + if test == "test" { + return Ok(24i32); + } + let mut i = 0; + loop { + loop { + if i == 1 { + break Ok(55); + } + i += 1; + } + } + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo() -> i3<|>2 { + let test = "test"; + let other = 5; + if test == "test" { + let res = match other { + 5 => 43, + _ => return 56, + }; + } + let mut i = 0; + loop { + loop { + if i == 1 { + break 55; + } + i += 1; + } + } + }"#, + r#"fn foo() -> Result> { + let test = "test"; + let other = 5; + if test == "test" { + let res = match other { + 5 => 43, + _ => return Ok(56), + }; + } + let mut i = 0; + loop { + loop { + if i == 1 { + break Ok(55); + } + i += 1; + } + } + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo(the_field: u32) -> u32<|> { + if the_field < 5 { + let mut i = 0; + loop { + if i > 5 { + return 55u32; + } + i += 3; + } + + match i { + 5 => return 99, + _ => return 0, + }; + } + + the_field + }"#, + r#"fn foo(the_field: u32) -> Result> { + if the_field < 5 { + let mut i = 0; + loop { + if i > 5 { + return Ok(55u32); + } + i += 3; + } + + match i { + 5 => return Ok(99), + _ => return Ok(0), + }; + } + + Ok(the_field) + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo(the_field: u32) -> u3<|>2 { + if the_field < 5 { + let mut i = 0; + + match i { + 5 => return 99, + _ => return 0, + } + } + + the_field + }"#, + r#"fn foo(the_field: u32) -> Result> { + if the_field < 5 { + let mut i = 0; + + match i { + 5 => return Ok(99), + _ => return Ok(0), + } + } + + Ok(the_field) + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo(the_field: u32) -> u32<|> { + if the_field < 5 { + let mut i = 0; + + if i == 5 { + return 99 + } else { + return 0 + } + } + + the_field + }"#, + r#"fn foo(the_field: u32) -> Result> { + if the_field < 5 { + let mut i = 0; + + if i == 5 { + return Ok(99) + } else { + return Ok(0) + } + } + + Ok(the_field) + }"#, + ); + + check_assist( + change_return_type_to_result, + r#"fn foo(the_field: u32) -> <|>u32 { + if the_field < 5 { + let mut i = 0; + + if i == 5 { + return 99; + } else { + return 0; + } + } + + the_field + }"#, + r#"fn foo(the_field: u32) -> Result> { + if the_field < 5 { + let mut i = 0; + + if i == 5 { + return Ok(99); + } else { + return Ok(0); + } + } + + Ok(the_field) + }"#, + ); + } +} diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 13ea45ec7cc..0473fd8c200 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -129,6 +129,7 @@ mod handlers { mod replace_qualified_name_with_use; mod replace_unwrap_with_match; mod split_import; + mod change_return_type_to_result; mod add_from_impl_for_enum; mod reorder_fields; mod unwrap_block; @@ -145,6 +146,7 @@ mod handlers { add_new::add_new, apply_demorgan::apply_demorgan, auto_import::auto_import, + change_return_type_to_result::change_return_type_to_result, change_visibility::change_visibility, early_return::convert_to_guarded_return, fill_match_arms::fill_match_arms, diff --git a/crates/ra_assists/src/tests/generated.rs b/crates/ra_assists/src/tests/generated.rs index 7d35fa2846d..972dbd251d6 100644 --- a/crates/ra_assists/src/tests/generated.rs +++ b/crates/ra_assists/src/tests/generated.rs @@ -249,6 +249,19 @@ pub mod std { pub mod collections { pub struct HashMap { } } } ) } +#[test] +fn doctest_change_return_type_to_result() { + check_doc_test( + "change_return_type_to_result", + r#####" +fn foo() -> i32<|> { 42i32 } +"#####, + r#####" +fn foo() -> Result { Ok(42i32) } +"#####, + ) +} + #[test] fn doctest_change_visibility() { check_doc_test( diff --git a/docs/user/assists.md b/docs/user/assists.md index ee515949e9d..692fd4f52ba 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -241,6 +241,18 @@ fn main() { } ``` +## `change_return_type_to_result` + +Change the function's return type to Result. + +```rust +// BEFORE +fn foo() -> i32┃ { 42i32 } + +// AFTER +fn foo() -> Result { Ok(42i32) } +``` + ## `change_visibility` Adds or changes existing visibility specifier. From 8a5d14453e464edeb8613df2fdca742bfe2a72f5 Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Tue, 5 May 2020 23:36:35 +0200 Subject: [PATCH 24/46] Add fixture doc comment --- crates/ra_db/src/fixture.rs | 46 ++++++++++++++++++++++++++++++++++++- xtask/tests/tidy.rs | 1 - 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/crates/ra_db/src/fixture.rs b/crates/ra_db/src/fixture.rs index 8248684eeac..51d4c493e3e 100644 --- a/crates/ra_db/src/fixture.rs +++ b/crates/ra_db/src/fixture.rs @@ -1,4 +1,48 @@ -//! FIXME: write short doc here +//! Fixtures are strings containing rust source code with optional metadata. +//! A fixture without metadata is parsed into a single source file. +//! Use this to test functionality local to one file. +//! +//! Example: +//! ``` +//! r#" +//! fn main() { +//! println!("Hello World") +//! } +//! "# +//! ``` +//! +//! Metadata can be added to a fixture after a `//-` comment. +//! The basic form is specifying filenames, +//! which is also how to define multiple files in a single test fixture +//! +//! Example: +//! ``` +//! " +//! //- /main.rs +//! mod foo; +//! fn main() { +//! foo::bar(); +//! } +//! +//! //- /foo.rs +//! pub fn bar() {} +//! " +//! ``` +//! +//! Metadata allows specifying all settings and variables +//! that are available in a real rust project: +//! - crate names via `crate:cratename` +//! - dependencies via `deps:dep1,dep2` +//! - configuration settings via `cfg:dbg=false,opt_level=2` +//! - environment variables via `env:PATH=/bin,RUST_LOG=debug` +//! +//! Example: +//! ``` +//! " +//! //- /lib.rs crate:foo deps:bar,baz cfg:foo=a,bar=b env:OUTDIR=path/to,OTHER=foo +//! fn insert_source_code_here() {} +//! " +//! ``` use std::str::FromStr; use std::sync::Arc; diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index c4eac1bc4e1..b8e8860ba1e 100644 --- a/xtask/tests/tidy.rs +++ b/xtask/tests/tidy.rs @@ -136,7 +136,6 @@ impl TidyDocs { } let whitelist = [ - "ra_db", "ra_hir", "ra_hir_expand", "ra_ide", From 86fa80e5b3860ee2446dcf29355c314d6dc4365a Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Tue, 5 May 2020 23:36:51 +0200 Subject: [PATCH 25/46] Allow fixture strings with unindented first line This allows fixtures like "//- /lib.rs ... //- /foo.rs ... " --- crates/test_utils/src/lib.rs | 104 ++++++++++++++++++++++++++++++++--- 1 file changed, 95 insertions(+), 9 deletions(-) diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs index b1365444a8f..b13e13af2aa 100644 --- a/crates/test_utils/src/lib.rs +++ b/crates/test_utils/src/lib.rs @@ -155,7 +155,7 @@ pub fn add_cursor(text: &str, offset: TextSize) -> String { res } -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq)] pub struct FixtureEntry { pub meta: String, pub text: String, @@ -170,19 +170,26 @@ pub struct FixtureEntry { /// // - other meta /// ``` pub fn parse_fixture(fixture: &str) -> Vec { - let margin = fixture - .lines() - .filter(|it| it.trim_start().starts_with("//-")) - .map(|it| it.len() - it.trim_start().len()) - .next() - .expect("empty fixture"); + let fixture = indent_first_line(fixture); + let margin = fixture_margin(&fixture); let mut lines = fixture .split('\n') // don't use `.lines` to not drop `\r\n` - .filter_map(|line| { + .enumerate() + .filter_map(|(ix, line)| { if line.len() >= margin { assert!(line[..margin].trim().is_empty()); - Some(&line[margin..]) + let line_content = &line[margin..]; + if !line_content.starts_with("//-") { + assert!( + !line_content.contains("//-"), + r#"Metadata line {} has invalid indentation. All metadata lines need to have the same indentation. +The offending line: {:?}"#, + ix, + line + ); + } + Some(line_content) } else { assert!(line.trim().is_empty()); None @@ -202,6 +209,85 @@ pub fn parse_fixture(fixture: &str) -> Vec { res } +/// Adjusts the indentation of the first line to the minimum indentation of the rest of the lines. +/// This allows fixtures to start off in a different indentation, e.g. to align the first line with +/// the other lines visually: +/// ``` +/// let fixture = "//- /lib.rs +/// mod foo; +/// //- /foo.rs +/// fn bar() {} +/// "; +/// assert_eq!(fixture_margin(fixture), +/// " //- /lib.rs +/// mod foo; +/// //- /foo.rs +/// fn bar() {} +/// ") +/// ``` +fn indent_first_line(fixture: &str) -> String { + if fixture.is_empty() { + return String::new(); + } + let mut lines = fixture.lines(); + let first_line = lines.next().unwrap(); + if first_line.contains("//-") { + let rest = lines.collect::>().join("\n"); + let fixed_margin = fixture_margin(&rest); + let fixed_indent = fixed_margin - indent_len(first_line); + format!("\n{}{}\n{}", " ".repeat(fixed_indent), first_line, rest) + } else { + fixture.to_owned() + } +} + +fn fixture_margin(fixture: &str) -> usize { + fixture + .lines() + .filter(|it| it.trim_start().starts_with("//-")) + .map(indent_len) + .next() + .expect("empty fixture") +} + +fn indent_len(s: &str) -> usize { + s.len() - s.trim_start().len() +} + +#[test] +#[should_panic] +fn parse_fixture_checks_further_indented_metadata() { + parse_fixture( + r" + //- /lib.rs + mod bar; + + fn foo() {} + //- /bar.rs + pub fn baz() {} + ", + ); +} + +#[test] +fn parse_fixture_can_handle_unindented_first_line() { + let fixture = "//- /lib.rs + mod foo; + //- /foo.rs + struct Bar; +"; + assert_eq!( + parse_fixture(fixture), + parse_fixture( + "//- /lib.rs +mod foo; +//- /foo.rs +struct Bar; +" + ) + ) +} + /// Same as `parse_fixture`, except it allow empty fixture pub fn parse_single_fixture(fixture: &str) -> Option { if !fixture.lines().any(|it| it.trim_start().starts_with("//-")) { From e0b63855b1874ccc71a49c933f73c62c95a92d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Wed, 6 May 2020 19:53:14 +0300 Subject: [PATCH 26/46] Fix Windows server path CC @Coder-256. --- docs/user/readme.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/readme.adoc b/docs/user/readme.adoc index 69f5b13d6b8..301e9a49cba 100644 --- a/docs/user/readme.adoc +++ b/docs/user/readme.adoc @@ -61,7 +61,7 @@ The server binary is stored in: * Linux: `~/.config/Code/User/globalStorage/matklad.rust-analyzer` * macOS: `~/Library/Application Support/Code/User/globalStorage/matklad.rust-analyzer` -* Windows: `%APPDATA%\Code\User\globalStorage` +* Windows: `%APPDATA%\Code\User\globalStorage\matklad.rust-analyzer` Note that we only support the latest version of VS Code. From 71369f5c595a48cda4c4d005bc8872a18efacfbe Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 19:03:17 +0200 Subject: [PATCH 27/46] Better mapping to TextMate scopes for keywords https://github.com/microsoft/vscode/issues/94367#issuecomment-608629883 --- editors/code/package.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/editors/code/package.json b/editors/code/package.json index 8849615c880..12a08ba4001 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -592,6 +592,12 @@ "keyword.unsafe": [ "keyword.other.unsafe" ], + "keyword": [ + "keyword" + ], + "keyword.controlFlow": [ + "keyword.control" + ], "variable.constant": [ "entity.name.constant" ] From 44b01ccff3d993daae237c75d466050711d06268 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Wed, 6 May 2020 12:39:11 -0700 Subject: [PATCH 28/46] return a PathBuf instead of String --- crates/ra_env/src/lib.rs | 12 ++++++------ crates/ra_project_model/src/sysroot.rs | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/ra_env/src/lib.rs b/crates/ra_env/src/lib.rs index 8d6aa926894..a1c4239be01 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_env/src/lib.rs @@ -4,15 +4,15 @@ use anyhow::{Error, Result}; use std::env; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -/// Return a `String` to use for the given executable. +/// Return a `PathBuf` to use for the given executable. /// /// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that /// gives a valid Cargo executable; or it may return a full path to a valid /// Cargo. -pub fn get_path_for_executable(executable_name: impl AsRef) -> Result { +pub fn get_path_for_executable(executable_name: impl AsRef) -> Result { // The current implementation checks three places for an executable to use: // 1) Appropriate environment variable (erroring if this is set but not a usable executable) // example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc @@ -25,7 +25,7 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result) -> Result String { format!("{} {}", program, args.join(" ")) } -fn run_command_in_cargo_dir(cargo_toml: &Path, program: &str, args: &[&str]) -> Result { +fn run_command_in_cargo_dir(cargo_toml: impl AsRef, program: impl AsRef, args: &[&str]) -> Result { + let program = program.as_ref().as_os_str().to_str().expect("Invalid Unicode in path"); let output = Command::new(program) - .current_dir(cargo_toml.parent().unwrap()) + .current_dir(cargo_toml.as_ref().parent().unwrap()) .args(args) .output() .context(format!("{} failed", create_command_text(program, args)))?; From 5d4648884baf591fe8f53e8ceae6d559564e9797 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Wed, 6 May 2020 12:47:13 -0700 Subject: [PATCH 29/46] cargo fmt --- crates/ra_project_model/src/sysroot.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index ed374f241ca..11c26ad8911 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -89,7 +89,11 @@ fn create_command_text(program: &str, args: &[&str]) -> String { format!("{} {}", program, args.join(" ")) } -fn run_command_in_cargo_dir(cargo_toml: impl AsRef, program: impl AsRef, args: &[&str]) -> Result { +fn run_command_in_cargo_dir( + cargo_toml: impl AsRef, + program: impl AsRef, + args: &[&str], +) -> Result { let program = program.as_ref().as_os_str().to_str().expect("Invalid Unicode in path"); let output = Command::new(program) .current_dir(cargo_toml.as_ref().parent().unwrap()) From 227929f9ddd3a5dfdab8a5552f457fd6184d3eb2 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Wed, 6 May 2020 13:28:32 -0700 Subject: [PATCH 30/46] simplify by using bail! macro --- crates/ra_env/src/lib.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/ra_env/src/lib.rs b/crates/ra_env/src/lib.rs index a1c4239be01..4f94bffdee1 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_env/src/lib.rs @@ -2,7 +2,7 @@ //! [`get_path_for_executable`](fn.get_path_for_executable.html). //! See docs there for more information. -use anyhow::{Error, Result}; +use anyhow::{bail, Result}; use std::env; use std::path::{Path, PathBuf}; use std::process::Command; @@ -27,10 +27,10 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result) -> Result Date: Thu, 7 May 2020 14:29:01 +0200 Subject: [PATCH 31/46] Fix panic in FunctionSignature --- crates/ra_ide/src/display/function_signature.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/ra_ide/src/display/function_signature.rs b/crates/ra_ide/src/display/function_signature.rs index db3907fe64e..f16d4227648 100644 --- a/crates/ra_ide/src/display/function_signature.rs +++ b/crates/ra_ide/src/display/function_signature.rs @@ -1,5 +1,7 @@ //! FIXME: write short doc here +// FIXME: this modules relies on strings and AST way too much, and it should be +// rewritten (matklad 2020-05-07) use std::{ convert::From, fmt::{self, Display}, @@ -202,7 +204,11 @@ impl From<&'_ ast::FnDef> for FunctionSignature { res.extend(param_list.params().map(|param| param.syntax().text().to_string())); res_types.extend(param_list.params().map(|param| { - param.syntax().text().to_string().split(':').nth(1).unwrap()[1..].to_string() + let param_text = param.syntax().text().to_string(); + match param_text.split(':').nth(1) { + Some(it) => it[1..].to_string(), + None => param_text, + } })); } (has_self_param, res, res_types) From 210f0cbd27b0621c47e15c74bbb16ab47a642999 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 7 May 2020 16:30:20 +0300 Subject: [PATCH 32/46] Remove HasVisibility implementation --- crates/ra_hir/src/code_model.rs | 7 ------- crates/ra_hir_def/src/adt.rs | 4 +--- crates/ra_ide_db/src/defs.rs | 7 +++++-- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 7eba0b23363..5f480c3040d 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -474,13 +474,6 @@ impl EnumVariant { } } -impl HasVisibility for EnumVariant { - fn visibility(&self, db: &dyn HirDatabase) -> Visibility { - let visibility = &db.enum_data(self.parent.id).visibility; - visibility.resolve(db.upcast(), &self.parent.id.resolver(db.upcast())) - } -} - /// A Data Type #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Adt { diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 0fda4d6c975..2bc34d449f2 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -33,7 +33,6 @@ pub struct StructData { #[derive(Debug, Clone, PartialEq, Eq)] pub struct EnumData { pub name: Name, - pub visibility: RawVisibility, pub variants: Arena, } @@ -92,8 +91,7 @@ impl EnumData { let name = src.value.name().map_or_else(Name::missing, |n| n.as_name()); let mut trace = Trace::new_for_arena(); lower_enum(db, &mut trace, &src, e.lookup(db).container.module(db)); - let visibility = RawVisibility::from_ast(db, src.with_value(src.value.visibility())); - Arc::new(EnumData { name, visibility, variants: trace.into_arena() }) + Arc::new(EnumData { name, variants: trace.into_arena() }) } pub fn variant(&self, name: &Name) -> Option { diff --git a/crates/ra_ide_db/src/defs.rs b/crates/ra_ide_db/src/defs.rs index 2e2850efbd7..f990e3bb97d 100644 --- a/crates/ra_ide_db/src/defs.rs +++ b/crates/ra_ide_db/src/defs.rs @@ -6,7 +6,7 @@ // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06). use hir::{ - Field, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution, + Adt, Field, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution, Semantics, TypeParam, Visibility, }; use ra_prof::profile; @@ -48,7 +48,10 @@ impl Definition { Definition::Macro(_) => None, Definition::Field(sf) => Some(sf.visibility(db)), Definition::ModuleDef(def) => match def { - ModuleDef::EnumVariant(id) => Some(id.visibility(db)), + ModuleDef::EnumVariant(id) => { + let parent = id.parent_enum(db); + module?.visibility_of(db, &ModuleDef::Adt(Adt::Enum(parent))) + } _ => module?.visibility_of(db, def), }, Definition::SelfType(_) => None, From 4867968d22899395e6551f22641b3617e995140c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 18:45:35 +0200 Subject: [PATCH 33/46] Refactor assists API to be more convenient for adding new assists It now duplicates completion API in its shape. --- .../src/{assist_ctx.rs => assist_context.rs} | 263 ++++++++---------- .../src/handlers/add_custom_impl.rs | 9 +- crates/ra_assists/src/handlers/add_derive.rs | 9 +- .../src/handlers/add_explicit_type.rs | 6 +- .../src/handlers/add_from_impl_for_enum.rs | 10 +- .../ra_assists/src/handlers/add_function.rs | 23 +- crates/ra_assists/src/handlers/add_impl.rs | 61 ++-- .../src/handlers/add_missing_impl_members.rs | 32 ++- crates/ra_assists/src/handlers/add_new.rs | 8 +- .../ra_assists/src/handlers/apply_demorgan.rs | 6 +- crates/ra_assists/src/handlers/auto_import.rs | 41 +-- .../handlers/change_return_type_to_result.rs | 10 +- .../src/handlers/change_visibility.rs | 31 +-- .../ra_assists/src/handlers/early_return.rs | 171 ++++++------ .../src/handlers/fill_match_arms.rs | 6 +- .../ra_assists/src/handlers/flip_binexpr.rs | 6 +- crates/ra_assists/src/handlers/flip_comma.rs | 6 +- .../src/handlers/flip_trait_bound.rs | 6 +- .../src/handlers/inline_local_variable.rs | 31 +-- .../src/handlers/introduce_variable.rs | 6 +- crates/ra_assists/src/handlers/invert_if.rs | 36 +-- .../ra_assists/src/handlers/merge_imports.rs | 13 +- .../src/handlers/merge_match_arms.rs | 6 +- crates/ra_assists/src/handlers/move_bounds.rs | 51 ++-- crates/ra_assists/src/handlers/move_guard.rs | 10 +- crates/ra_assists/src/handlers/raw_string.rs | 18 +- crates/ra_assists/src/handlers/remove_dbg.rs | 6 +- crates/ra_assists/src/handlers/remove_mut.rs | 6 +- .../ra_assists/src/handlers/reorder_fields.rs | 27 +- .../src/handlers/replace_if_let_with_match.rs | 51 ++-- .../src/handlers/replace_let_with_if_let.rs | 12 +- .../replace_qualified_name_with_use.rs | 19 +- .../src/handlers/replace_unwrap_with_match.rs | 41 ++- .../ra_assists/src/handlers/split_import.rs | 6 +- .../ra_assists/src/handlers/unwrap_block.rs | 6 +- crates/ra_assists/src/lib.rs | 42 ++- crates/ra_assists/src/tests.rs | 21 +- crates/ra_assists/src/utils/insert_use.rs | 10 +- 38 files changed, 525 insertions(+), 597 deletions(-) rename crates/ra_assists/src/{assist_ctx.rs => assist_context.rs} (54%) diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_context.rs similarity index 54% rename from crates/ra_assists/src/assist_ctx.rs rename to crates/ra_assists/src/assist_context.rs index 871671de2bf..203ad127386 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -1,4 +1,6 @@ -//! This module defines `AssistCtx` -- the API surface that is exposed to assists. +//! See `AssistContext` + +use algo::find_covering_element; use hir::Semantics; use ra_db::{FileId, FileRange}; use ra_fmt::{leading_indent, reindent}; @@ -7,7 +9,7 @@ use ra_ide_db::{ RootDatabase, }; use ra_syntax::{ - algo::{self, find_covering_element, find_node_at_offset, SyntaxRewriter}, + algo::{self, find_node_at_offset, SyntaxRewriter}, AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, }; @@ -15,46 +17,17 @@ use ra_text_edit::TextEditBuilder; use crate::{AssistId, AssistLabel, GroupLabel, ResolvedAssist}; -#[derive(Clone, Debug)] -pub(crate) struct Assist(pub(crate) Vec); - -#[derive(Clone, Debug)] -pub(crate) struct AssistInfo { - pub(crate) label: AssistLabel, - pub(crate) group_label: Option, - pub(crate) source_change: Option, -} - -impl AssistInfo { - fn new(label: AssistLabel) -> AssistInfo { - AssistInfo { label, group_label: None, source_change: None } - } - - fn resolved(self, source_change: SourceChange) -> AssistInfo { - AssistInfo { source_change: Some(source_change), ..self } - } - - fn with_group(self, group_label: GroupLabel) -> AssistInfo { - AssistInfo { group_label: Some(group_label), ..self } - } - - pub(crate) fn into_resolved(self) -> Option { - let label = self.label; - self.source_change.map(|source_change| ResolvedAssist { label, source_change }) - } -} - -/// `AssistCtx` allows to apply an assist or check if it could be applied. +/// `AssistContext` allows to apply an assist or check if it could be applied. /// -/// Assists use a somewhat over-engineered approach, given the current needs. The -/// assists workflow consists of two phases. In the first phase, a user asks for -/// the list of available assists. In the second phase, the user picks a +/// Assists use a somewhat over-engineered approach, given the current needs. +/// The assists workflow consists of two phases. In the first phase, a user asks +/// for the list of available assists. In the second phase, the user picks a /// particular assist and it gets applied. /// /// There are two peculiarities here: /// -/// * first, we ideally avoid computing more things then necessary to answer -/// "is assist applicable" in the first phase. +/// * first, we ideally avoid computing more things then necessary to answer "is +/// assist applicable" in the first phase. /// * second, when we are applying assist, we don't have a guarantee that there /// weren't any changes between the point when user asked for assists and when /// they applied a particular assist. So, when applying assist, we need to do @@ -63,152 +36,158 @@ impl AssistInfo { /// To avoid repeating the same code twice for both "check" and "apply" /// functions, we use an approach reminiscent of that of Django's function based /// views dealing with forms. Each assist receives a runtime parameter, -/// `should_compute_edit`. It first check if an edit is applicable (potentially -/// computing info required to compute the actual edit). If it is applicable, -/// and `should_compute_edit` is `true`, it then computes the actual edit. +/// `resolve`. It first check if an edit is applicable (potentially computing +/// info required to compute the actual edit). If it is applicable, and +/// `resolve` is `true`, it then computes the actual edit. /// /// So, to implement the original assists workflow, we can first apply each edit -/// with `should_compute_edit = false`, and then applying the selected edit -/// again, with `should_compute_edit = true` this time. +/// with `resolve = false`, and then applying the selected edit again, with +/// `resolve = true` this time. /// /// Note, however, that we don't actually use such two-phase logic at the /// moment, because the LSP API is pretty awkward in this place, and it's much /// easier to just compute the edit eagerly :-) -#[derive(Clone)] -pub(crate) struct AssistCtx<'a> { - pub(crate) sema: &'a Semantics<'a, RootDatabase>, - pub(crate) db: &'a RootDatabase, +pub(crate) struct AssistContext<'a> { + pub(crate) sema: Semantics<'a, RootDatabase>, + pub(super) db: &'a RootDatabase, pub(crate) frange: FileRange, source_file: SourceFile, - should_compute_edit: bool, } -impl<'a> AssistCtx<'a> { - pub fn new( - sema: &'a Semantics<'a, RootDatabase>, - frange: FileRange, - should_compute_edit: bool, - ) -> AssistCtx<'a> { +impl<'a> AssistContext<'a> { + pub fn new(sema: Semantics<'a, RootDatabase>, frange: FileRange) -> AssistContext<'a> { let source_file = sema.parse(frange.file_id); - AssistCtx { sema, db: sema.db, frange, source_file, should_compute_edit } + let db = sema.db; + AssistContext { sema, db, frange, source_file } } - pub(crate) fn add_assist( - self, - id: AssistId, - label: impl Into, - target: TextRange, - f: impl FnOnce(&mut ActionBuilder), - ) -> Option { - let label = AssistLabel::new(id, label.into(), None, target); - let change_label = label.label.clone(); - let mut info = AssistInfo::new(label); - if self.should_compute_edit { - let source_change = { - let mut edit = ActionBuilder::new(&self); - f(&mut edit); - edit.build(change_label) - }; - info = info.resolved(source_change) - }; - - Some(Assist(vec![info])) - } - - pub(crate) fn add_assist_group(self, group_name: impl Into) -> AssistGroup<'a> { - let group = GroupLabel(group_name.into()); - AssistGroup { ctx: self, group, assists: Vec::new() } + // NB, this ignores active selection. + pub(crate) fn offset(&self) -> TextSize { + self.frange.range.start() } pub(crate) fn token_at_offset(&self) -> TokenAtOffset { - self.source_file.syntax().token_at_offset(self.frange.range.start()) + self.source_file.syntax().token_at_offset(self.offset()) } - pub(crate) fn find_token_at_offset(&self, kind: SyntaxKind) -> Option { self.token_at_offset().find(|it| it.kind() == kind) } - pub(crate) fn find_node_at_offset(&self) -> Option { - find_node_at_offset(self.source_file.syntax(), self.frange.range.start()) + find_node_at_offset(self.source_file.syntax(), self.offset()) } - pub(crate) fn find_node_at_offset_with_descend(&self) -> Option { self.sema .find_node_at_offset_with_descend(self.source_file.syntax(), self.frange.range.start()) } - pub(crate) fn covering_element(&self) -> SyntaxElement { find_covering_element(self.source_file.syntax(), self.frange.range) } + // FIXME: remove pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement { find_covering_element(self.source_file.syntax(), range) } } -pub(crate) struct AssistGroup<'a> { - ctx: AssistCtx<'a>, - group: GroupLabel, - assists: Vec, +pub(crate) struct Assists { + resolve: bool, + file: FileId, + buf: Vec<(AssistLabel, Option)>, } -impl<'a> AssistGroup<'a> { - pub(crate) fn add_assist( +impl Assists { + pub(crate) fn new_resolved(ctx: &AssistContext) -> Assists { + Assists { resolve: true, file: ctx.frange.file_id, buf: Vec::new() } + } + pub(crate) fn new_unresolved(ctx: &AssistContext) -> Assists { + Assists { resolve: false, file: ctx.frange.file_id, buf: Vec::new() } + } + + pub(crate) fn finish_unresolved(self) -> Vec { + assert!(!self.resolve); + self.finish() + .into_iter() + .map(|(label, edit)| { + assert!(edit.is_none()); + label + }) + .collect() + } + + pub(crate) fn finish_resolved(self) -> Vec { + assert!(self.resolve); + self.finish() + .into_iter() + .map(|(label, edit)| ResolvedAssist { label, source_change: edit.unwrap() }) + .collect() + } + + pub(crate) fn add( &mut self, id: AssistId, label: impl Into, target: TextRange, - f: impl FnOnce(&mut ActionBuilder), - ) { - let label = AssistLabel::new(id, label.into(), Some(self.group.clone()), target); + f: impl FnOnce(&mut AssistBuilder), + ) -> Option<()> { + let label = AssistLabel::new(id, label.into(), None, target); + self.add_impl(label, f) + } + pub(crate) fn add_group( + &mut self, + group: &GroupLabel, + id: AssistId, + label: impl Into, + target: TextRange, + f: impl FnOnce(&mut AssistBuilder), + ) -> Option<()> { + let label = AssistLabel::new(id, label.into(), Some(group.clone()), target); + self.add_impl(label, f) + } + fn add_impl(&mut self, label: AssistLabel, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { let change_label = label.label.clone(); - let mut info = AssistInfo::new(label).with_group(self.group.clone()); - if self.ctx.should_compute_edit { - let source_change = { - let mut edit = ActionBuilder::new(&self.ctx); - f(&mut edit); - edit.build(change_label) - }; - info = info.resolved(source_change) + let source_change = if self.resolve { + let mut builder = AssistBuilder::new(self.file); + f(&mut builder); + Some(builder.finish(change_label)) + } else { + None }; - self.assists.push(info) + self.buf.push((label, source_change)); + Some(()) } - pub(crate) fn finish(self) -> Option { - if self.assists.is_empty() { - None - } else { - Some(Assist(self.assists)) - } + fn finish(mut self) -> Vec<(AssistLabel, Option)> { + self.buf.sort_by_key(|(label, _edit)| label.target.len()); + self.buf } } -pub(crate) struct ActionBuilder<'a, 'b> { +pub(crate) struct AssistBuilder { edit: TextEditBuilder, cursor_position: Option, file: FileId, - ctx: &'a AssistCtx<'b>, } -impl<'a, 'b> ActionBuilder<'a, 'b> { - fn new(ctx: &'a AssistCtx<'b>) -> Self { - Self { - edit: TextEditBuilder::default(), - cursor_position: None, - file: ctx.frange.file_id, - ctx, - } +impl AssistBuilder { + pub(crate) fn new(file: FileId) -> AssistBuilder { + AssistBuilder { edit: TextEditBuilder::default(), cursor_position: None, file } } - pub(crate) fn ctx(&self) -> &AssistCtx<'b> { - &self.ctx + /// Remove specified `range` of text. + pub(crate) fn delete(&mut self, range: TextRange) { + self.edit.delete(range) + } + /// Append specified `text` at the given `offset` + pub(crate) fn insert(&mut self, offset: TextSize, text: impl Into) { + self.edit.insert(offset, text.into()) } - /// Replaces specified `range` of text with a given string. pub(crate) fn replace(&mut self, range: TextRange, replace_with: impl Into) { self.edit.replace(range, replace_with.into()) } - + pub(crate) fn replace_ast(&mut self, old: N, new: N) { + algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit) + } /// Replaces specified `node` of text with a given string, reindenting the /// string to maintain `node`'s existing indent. // FIXME: remove in favor of ra_syntax::edit::IndentLevel::increase_indent @@ -223,42 +202,28 @@ impl<'a, 'b> ActionBuilder<'a, 'b> { } self.replace(node.text_range(), replace_with) } - - /// Remove specified `range` of text. - #[allow(unused)] - pub(crate) fn delete(&mut self, range: TextRange) { - self.edit.delete(range) - } - - /// Append specified `text` at the given `offset` - pub(crate) fn insert(&mut self, offset: TextSize, text: impl Into) { - self.edit.insert(offset, text.into()) - } - - /// Specify desired position of the cursor after the assist is applied. - pub(crate) fn set_cursor(&mut self, offset: TextSize) { - self.cursor_position = Some(offset) - } - - /// Get access to the raw `TextEditBuilder`. - pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder { - &mut self.edit - } - - pub(crate) fn replace_ast(&mut self, old: N, new: N) { - algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit) - } pub(crate) fn rewrite(&mut self, rewriter: SyntaxRewriter) { let node = rewriter.rewrite_root().unwrap(); let new = rewriter.rewrite(&node); algo::diff(&node, &new).into_text_edit(&mut self.edit) } + /// Specify desired position of the cursor after the assist is applied. + pub(crate) fn set_cursor(&mut self, offset: TextSize) { + self.cursor_position = Some(offset) + } + // FIXME: better API pub(crate) fn set_file(&mut self, assist_file: FileId) { self.file = assist_file; } - fn build(self, change_label: String) -> SourceChange { + // FIXME: kill this API + /// Get access to the raw `TextEditBuilder`. + pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder { + &mut self.edit + } + + fn finish(self, change_label: String) -> SourceChange { let edit = self.edit.finish(); if edit.is_empty() && self.cursor_position.is_none() { panic!("Only call `add_assist` if the assist can be applied") diff --git a/crates/ra_assists/src/handlers/add_custom_impl.rs b/crates/ra_assists/src/handlers/add_custom_impl.rs index 869d4dc0458..795a225a4d8 100644 --- a/crates/ra_assists/src/handlers/add_custom_impl.rs +++ b/crates/ra_assists/src/handlers/add_custom_impl.rs @@ -6,7 +6,10 @@ use ra_syntax::{ }; use stdx::SepBy; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, +}; // Assist: add_custom_impl // @@ -25,7 +28,7 @@ use crate::{Assist, AssistCtx, AssistId}; // // } // ``` -pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option { +pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let input = ctx.find_node_at_offset::()?; let attr = input.syntax().parent().and_then(ast::Attr::cast)?; @@ -49,7 +52,7 @@ pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option { format!("Add custom impl '{}' for '{}'", trait_token.text().as_str(), annotated_name); let target = attr.syntax().text_range(); - ctx.add_assist(AssistId("add_custom_impl"), label, target, |edit| { + acc.add(AssistId("add_custom_impl"), label, target, |edit| { let new_attr_input = input .syntax() .descendants_with_tokens() diff --git a/crates/ra_assists/src/handlers/add_derive.rs b/crates/ra_assists/src/handlers/add_derive.rs index 2a6bb1caedf..fb08c19e936 100644 --- a/crates/ra_assists/src/handlers/add_derive.rs +++ b/crates/ra_assists/src/handlers/add_derive.rs @@ -4,7 +4,7 @@ use ra_syntax::{ TextSize, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: add_derive // @@ -24,11 +24,11 @@ use crate::{Assist, AssistCtx, AssistId}; // y: u32, // } // ``` -pub(crate) fn add_derive(ctx: AssistCtx) -> Option { +pub(crate) fn add_derive(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let nominal = ctx.find_node_at_offset::()?; let node_start = derive_insertion_offset(&nominal)?; let target = nominal.syntax().text_range(); - ctx.add_assist(AssistId("add_derive"), "Add `#[derive]`", target, |edit| { + acc.add(AssistId("add_derive"), "Add `#[derive]`", target, |edit| { let derive_attr = nominal .attrs() .filter_map(|x| x.as_simple_call()) @@ -57,9 +57,10 @@ fn derive_insertion_offset(nominal: &ast::NominalDef) -> Option { #[cfg(test)] mod tests { - use super::*; use crate::tests::{check_assist, check_assist_target}; + use super::*; + #[test] fn add_derive_new() { check_assist( diff --git a/crates/ra_assists/src/handlers/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs index a59ec16b2d9..55409e5013c 100644 --- a/crates/ra_assists/src/handlers/add_explicit_type.rs +++ b/crates/ra_assists/src/handlers/add_explicit_type.rs @@ -4,7 +4,7 @@ use ra_syntax::{ TextRange, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: add_explicit_type // @@ -21,7 +21,7 @@ use crate::{Assist, AssistCtx, AssistId}; // let x: i32 = 92; // } // ``` -pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option { +pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let stmt = ctx.find_node_at_offset::()?; let expr = stmt.initializer()?; let pat = stmt.pat()?; @@ -59,7 +59,7 @@ pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option { let db = ctx.db; let new_type_string = ty.display_truncated(db, None).to_string(); - ctx.add_assist( + acc.add( AssistId("add_explicit_type"), format!("Insert explicit type '{}'", new_type_string), pat_range, diff --git a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs index 81deb3dfa3b..275184e2466 100644 --- a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs +++ b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs @@ -4,10 +4,10 @@ use ra_syntax::{ TextSize, }; use stdx::format_to; - -use crate::{utils::FamousDefs, Assist, AssistCtx, AssistId}; use test_utils::tested_by; +use crate::{utils::FamousDefs, AssistContext, AssistId, Assists}; + // Assist add_from_impl_for_enum // // Adds a From impl for an enum variant with one tuple field @@ -25,7 +25,7 @@ use test_utils::tested_by; // } // } // ``` -pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option { +pub(crate) fn add_from_impl_for_enum(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let variant = ctx.find_node_at_offset::()?; let variant_name = variant.name()?; let enum_name = variant.parent_enum().name()?; @@ -42,13 +42,13 @@ pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option { _ => return None, }; - if existing_from_impl(ctx.sema, &variant).is_some() { + if existing_from_impl(&ctx.sema, &variant).is_some() { tested_by!(test_add_from_impl_already_exists); return None; } let target = variant.syntax().text_range(); - ctx.add_assist( + acc.add( AssistId("add_from_impl_for_enum"), "Add From impl for this enum variant", target, diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index 27807966506..6b5616aa9c8 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -1,14 +1,13 @@ -use ra_syntax::{ - ast::{self, AstNode}, - SyntaxKind, SyntaxNode, TextSize, -}; - -use crate::{Assist, AssistCtx, AssistId}; -use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner}; use hir::HirDisplay; use ra_db::FileId; +use ra_syntax::{ + ast::{self, edit::IndentLevel, ArgListOwner, AstNode, ModuleItemOwner}, + SyntaxKind, SyntaxNode, TextSize, +}; use rustc_hash::{FxHashMap, FxHashSet}; +use crate::{AssistContext, AssistId, Assists}; + // Assist: add_function // // Adds a stub function with a signature matching the function under the cursor. @@ -34,7 +33,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; // } // // ``` -pub(crate) fn add_function(ctx: AssistCtx) -> Option { +pub(crate) fn add_function(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let path_expr: ast::PathExpr = ctx.find_node_at_offset()?; let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let path = path_expr.path()?; @@ -59,7 +58,7 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option { let function_builder = FunctionBuilder::from_call(&ctx, &call, &path, target_module)?; let target = call.syntax().text_range(); - ctx.add_assist(AssistId("add_function"), "Add function", target, |edit| { + acc.add(AssistId("add_function"), "Add function", target, |edit| { let function_template = function_builder.render(); edit.set_file(function_template.file); edit.set_cursor(function_template.cursor_offset); @@ -87,7 +86,7 @@ impl FunctionBuilder { /// Prepares a generated function that matches `call` in `generate_in` /// (or as close to `call` as possible, if `generate_in` is `None`) fn from_call( - ctx: &AssistCtx, + ctx: &AssistContext, call: &ast::CallExpr, path: &ast::Path, target_module: Option>, @@ -152,7 +151,7 @@ fn fn_name(call: &ast::Path) -> Option { /// Computes the type variables and arguments required for the generated function fn fn_args( - ctx: &AssistCtx, + ctx: &AssistContext, call: &ast::CallExpr, ) -> Option<(Option, ast::ParamList)> { let mut arg_names = Vec::new(); @@ -219,7 +218,7 @@ fn fn_arg_name(fn_arg: &ast::Expr) -> Option { } } -fn fn_arg_type(ctx: &AssistCtx, fn_arg: &ast::Expr) -> Option { +fn fn_arg_type(ctx: &AssistContext, fn_arg: &ast::Expr) -> Option { let ty = ctx.sema.type_of_expr(fn_arg)?; if ty.is_unknown() { return None; diff --git a/crates/ra_assists/src/handlers/add_impl.rs b/crates/ra_assists/src/handlers/add_impl.rs index 557344ebb80..df114a0d84d 100644 --- a/crates/ra_assists/src/handlers/add_impl.rs +++ b/crates/ra_assists/src/handlers/add_impl.rs @@ -4,7 +4,7 @@ use ra_syntax::{ }; use stdx::{format_to, SepBy}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: add_impl // @@ -25,43 +25,36 @@ use crate::{Assist, AssistCtx, AssistId}; // // } // ``` -pub(crate) fn add_impl(ctx: AssistCtx) -> Option { +pub(crate) fn add_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let nominal = ctx.find_node_at_offset::()?; let name = nominal.name()?; let target = nominal.syntax().text_range(); - ctx.add_assist( - AssistId("add_impl"), - format!("Implement {}", name.text().as_str()), - target, - |edit| { - let type_params = nominal.type_param_list(); - let start_offset = nominal.syntax().text_range().end(); - let mut buf = String::new(); - buf.push_str("\n\nimpl"); - if let Some(type_params) = &type_params { - format_to!(buf, "{}", type_params.syntax()); - } - buf.push_str(" "); - buf.push_str(name.text().as_str()); - if let Some(type_params) = type_params { - let lifetime_params = type_params - .lifetime_params() - .filter_map(|it| it.lifetime_token()) - .map(|it| it.text().clone()); - let type_params = type_params - .type_params() - .filter_map(|it| it.name()) - .map(|it| it.text().clone()); + acc.add(AssistId("add_impl"), format!("Implement {}", name.text().as_str()), target, |edit| { + let type_params = nominal.type_param_list(); + let start_offset = nominal.syntax().text_range().end(); + let mut buf = String::new(); + buf.push_str("\n\nimpl"); + if let Some(type_params) = &type_params { + format_to!(buf, "{}", type_params.syntax()); + } + buf.push_str(" "); + buf.push_str(name.text().as_str()); + if let Some(type_params) = type_params { + let lifetime_params = type_params + .lifetime_params() + .filter_map(|it| it.lifetime_token()) + .map(|it| it.text().clone()); + let type_params = + type_params.type_params().filter_map(|it| it.name()).map(|it| it.text().clone()); - let generic_params = lifetime_params.chain(type_params).sep_by(", "); - format_to!(buf, "<{}>", generic_params) - } - buf.push_str(" {\n"); - edit.set_cursor(start_offset + TextSize::of(&buf)); - buf.push_str("\n}"); - edit.insert(start_offset, buf); - }, - ) + let generic_params = lifetime_params.chain(type_params).sep_by(", "); + format_to!(buf, "<{}>", generic_params) + } + buf.push_str(" {\n"); + edit.set_cursor(start_offset + TextSize::of(&buf)); + buf.push_str("\n}"); + edit.insert(start_offset, buf); + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs index 7df78659090..3482a75bfcf 100644 --- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs @@ -9,9 +9,10 @@ use ra_syntax::{ }; use crate::{ + assist_context::{AssistContext, Assists}, ast_transform::{self, AstTransform, QualifyPaths, SubstituteTypeParams}, utils::{get_missing_assoc_items, resolve_target_trait}, - Assist, AssistCtx, AssistId, + AssistId, }; #[derive(PartialEq)] @@ -50,8 +51,9 @@ enum AddMissingImplMembersMode { // // } // ``` -pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Option { +pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { add_missing_impl_members_inner( + acc, ctx, AddMissingImplMembersMode::NoDefaultMethods, "add_impl_missing_members", @@ -91,8 +93,9 @@ pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Option { // // } // ``` -pub(crate) fn add_missing_default_members(ctx: AssistCtx) -> Option { +pub(crate) fn add_missing_default_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { add_missing_impl_members_inner( + acc, ctx, AddMissingImplMembersMode::DefaultMethodsOnly, "add_impl_default_members", @@ -101,11 +104,12 @@ pub(crate) fn add_missing_default_members(ctx: AssistCtx) -> Option { } fn add_missing_impl_members_inner( - ctx: AssistCtx, + acc: &mut Assists, + ctx: &AssistContext, mode: AddMissingImplMembersMode, assist_id: &'static str, label: &'static str, -) -> Option { +) -> Option<()> { let _p = ra_prof::profile("add_missing_impl_members_inner"); let impl_def = ctx.find_node_at_offset::()?; let impl_item_list = impl_def.item_list()?; @@ -142,12 +146,11 @@ fn add_missing_impl_members_inner( return None; } - let sema = ctx.sema; let target = impl_def.syntax().text_range(); - ctx.add_assist(AssistId(assist_id), label, target, |edit| { + acc.add(AssistId(assist_id), label, target, |edit| { let n_existing_items = impl_item_list.assoc_items().count(); - let source_scope = sema.scope_for_def(trait_); - let target_scope = sema.scope(impl_item_list.syntax()); + let source_scope = ctx.sema.scope_for_def(trait_); + let target_scope = ctx.sema.scope(impl_item_list.syntax()); let ast_transform = QualifyPaths::new(&target_scope, &source_scope) .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def)); let items = missing_items @@ -170,13 +173,12 @@ fn add_missing_impl_members_inner( } fn add_body(fn_def: ast::FnDef) -> ast::FnDef { - if fn_def.body().is_none() { - let body = make::block_expr(None, Some(make::expr_todo())); - let body = IndentLevel(1).increase_indent(body); - fn_def.with_body(body) - } else { - fn_def + if fn_def.body().is_some() { + return fn_def; } + let body = make::block_expr(None, Some(make::expr_todo())); + let body = IndentLevel(1).increase_indent(body); + fn_def.with_body(body) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/add_new.rs b/crates/ra_assists/src/handlers/add_new.rs index 1c3f8435ab6..fe7451dcfdf 100644 --- a/crates/ra_assists/src/handlers/add_new.rs +++ b/crates/ra_assists/src/handlers/add_new.rs @@ -7,7 +7,7 @@ use ra_syntax::{ }; use stdx::{format_to, SepBy}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: add_new // @@ -29,7 +29,7 @@ use crate::{Assist, AssistCtx, AssistId}; // } // // ``` -pub(crate) fn add_new(ctx: AssistCtx) -> Option { +pub(crate) fn add_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let strukt = ctx.find_node_at_offset::()?; // We want to only apply this to non-union structs with named fields @@ -42,7 +42,7 @@ pub(crate) fn add_new(ctx: AssistCtx) -> Option { let impl_def = find_struct_impl(&ctx, &strukt)?; let target = strukt.syntax().text_range(); - ctx.add_assist(AssistId("add_new"), "Add default constructor", target, |edit| { + acc.add(AssistId("add_new"), "Add default constructor", target, |edit| { let mut buf = String::with_capacity(512); if impl_def.is_some() { @@ -123,7 +123,7 @@ fn generate_impl_text(strukt: &ast::StructDef, code: &str) -> String { // // FIXME: change the new fn checking to a more semantic approach when that's more // viable (e.g. we process proc macros, etc) -fn find_struct_impl(ctx: &AssistCtx, strukt: &ast::StructDef) -> Option> { +fn find_struct_impl(ctx: &AssistContext, strukt: &ast::StructDef) -> Option> { let db = ctx.db; let module = strukt.syntax().ancestors().find(|node| { ast::Module::can_cast(node.kind()) || ast::SourceFile::can_cast(node.kind()) diff --git a/crates/ra_assists/src/handlers/apply_demorgan.rs b/crates/ra_assists/src/handlers/apply_demorgan.rs index a5b26e5b937..0feba5e11f6 100644 --- a/crates/ra_assists/src/handlers/apply_demorgan.rs +++ b/crates/ra_assists/src/handlers/apply_demorgan.rs @@ -1,6 +1,6 @@ use ra_syntax::ast::{self, AstNode}; -use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId}; +use crate::{utils::invert_boolean_expression, AssistContext, AssistId, Assists}; // Assist: apply_demorgan // @@ -21,7 +21,7 @@ use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId}; // if !(x == 4 && y) {} // } // ``` -pub(crate) fn apply_demorgan(ctx: AssistCtx) -> Option { +pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let expr = ctx.find_node_at_offset::()?; let op = expr.op_kind()?; let op_range = expr.op_token()?.text_range(); @@ -39,7 +39,7 @@ pub(crate) fn apply_demorgan(ctx: AssistCtx) -> Option { let rhs_range = rhs.syntax().text_range(); let not_rhs = invert_boolean_expression(rhs); - ctx.add_assist(AssistId("apply_demorgan"), "Apply De Morgan's law", op_range, |edit| { + acc.add(AssistId("apply_demorgan"), "Apply De Morgan's law", op_range, |edit| { edit.replace(op_range, opposite_op); edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text())); edit.replace(rhs_range, format!("{})", not_rhs.syntax().text())); diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs index 2224b9714bc..78d23150d38 100644 --- a/crates/ra_assists/src/handlers/auto_import.rs +++ b/crates/ra_assists/src/handlers/auto_import.rs @@ -1,5 +1,6 @@ use std::collections::BTreeSet; +use either::Either; use hir::{ AsAssocItem, AssocItemContainer, ModPath, Module, ModuleDef, PathResolution, Semantics, Trait, Type, @@ -12,12 +13,7 @@ use ra_syntax::{ }; use rustc_hash::FxHashSet; -use crate::{ - assist_ctx::{Assist, AssistCtx}, - utils::insert_use_statement, - AssistId, -}; -use either::Either; +use crate::{utils::insert_use_statement, AssistContext, AssistId, Assists, GroupLabel}; // Assist: auto_import // @@ -38,7 +34,7 @@ use either::Either; // } // # pub mod std { pub mod collections { pub struct HashMap { } } } // ``` -pub(crate) fn auto_import(ctx: AssistCtx) -> Option { +pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let auto_import_assets = AutoImportAssets::new(&ctx)?; let proposed_imports = auto_import_assets.search_for_imports(ctx.db); if proposed_imports.is_empty() { @@ -46,13 +42,19 @@ pub(crate) fn auto_import(ctx: AssistCtx) -> Option { } let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range; - let mut group = ctx.add_assist_group(auto_import_assets.get_import_group_message()); + let group = auto_import_assets.get_import_group_message(); for import in proposed_imports { - group.add_assist(AssistId("auto_import"), format!("Import `{}`", &import), range, |edit| { - insert_use_statement(&auto_import_assets.syntax_under_caret, &import, edit); - }); + acc.add_group( + &group, + AssistId("auto_import"), + format!("Import `{}`", &import), + range, + |builder| { + insert_use_statement(&auto_import_assets.syntax_under_caret, &import, ctx, builder); + }, + ); } - group.finish() + Some(()) } #[derive(Debug)] @@ -63,7 +65,7 @@ struct AutoImportAssets { } impl AutoImportAssets { - fn new(ctx: &AssistCtx) -> Option { + fn new(ctx: &AssistContext) -> Option { if let Some(path_under_caret) = ctx.find_node_at_offset_with_descend::() { Self::for_regular_path(path_under_caret, &ctx) } else { @@ -71,7 +73,7 @@ impl AutoImportAssets { } } - fn for_method_call(method_call: ast::MethodCallExpr, ctx: &AssistCtx) -> Option { + fn for_method_call(method_call: ast::MethodCallExpr, ctx: &AssistContext) -> Option { let syntax_under_caret = method_call.syntax().to_owned(); let module_with_name_to_import = ctx.sema.scope(&syntax_under_caret).module()?; Some(Self { @@ -81,7 +83,7 @@ impl AutoImportAssets { }) } - fn for_regular_path(path_under_caret: ast::Path, ctx: &AssistCtx) -> Option { + fn for_regular_path(path_under_caret: ast::Path, ctx: &AssistContext) -> Option { let syntax_under_caret = path_under_caret.syntax().to_owned(); if syntax_under_caret.ancestors().find_map(ast::UseItem::cast).is_some() { return None; @@ -104,8 +106,8 @@ impl AutoImportAssets { } } - fn get_import_group_message(&self) -> String { - match &self.import_candidate { + fn get_import_group_message(&self) -> GroupLabel { + let name = match &self.import_candidate { ImportCandidate::UnqualifiedName(name) => format!("Import {}", name), ImportCandidate::QualifierStart(qualifier_start) => { format!("Import {}", qualifier_start) @@ -116,7 +118,8 @@ impl AutoImportAssets { ImportCandidate::TraitMethod(_, trait_method_name) => { format!("Import a trait for method {}", trait_method_name) } - } + }; + GroupLabel(name) } fn search_for_imports(&self, db: &RootDatabase) -> BTreeSet { @@ -383,7 +386,7 @@ mod tests { } ", r" - use PubMod1::PubStruct; + use PubMod3::PubStruct; PubSt<|>ruct diff --git a/crates/ra_assists/src/handlers/change_return_type_to_result.rs b/crates/ra_assists/src/handlers/change_return_type_to_result.rs index 1e8d986cdbc..5c907097e55 100644 --- a/crates/ra_assists/src/handlers/change_return_type_to_result.rs +++ b/crates/ra_assists/src/handlers/change_return_type_to_result.rs @@ -1,11 +1,11 @@ use ra_syntax::{ - ast, AstNode, + ast::{self, BlockExpr, Expr, LoopBodyOwner}, + AstNode, SyntaxKind::{COMMENT, WHITESPACE}, SyntaxNode, TextSize, }; -use crate::{Assist, AssistCtx, AssistId}; -use ast::{BlockExpr, Expr, LoopBodyOwner}; +use crate::{AssistContext, AssistId, Assists}; // Assist: change_return_type_to_result // @@ -18,7 +18,7 @@ use ast::{BlockExpr, Expr, LoopBodyOwner}; // ``` // fn foo() -> Result { Ok(42i32) } // ``` -pub(crate) fn change_return_type_to_result(ctx: AssistCtx) -> Option { +pub(crate) fn change_return_type_to_result(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let fn_def = ctx.find_node_at_offset::(); let fn_def = &mut fn_def?; let ret_type = &fn_def.ret_type()?.type_ref()?; @@ -33,7 +33,7 @@ pub(crate) fn change_return_type_to_result(ctx: AssistCtx) -> Option { return None; } - ctx.add_assist( + acc.add( AssistId("change_return_type_to_result"), "Change return type to Result", ret_type.syntax().text_range(), diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 489db83e6ef..e631766eff1 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -7,10 +7,10 @@ use ra_syntax::{ }, SyntaxNode, TextSize, T, }; - -use crate::{Assist, AssistCtx, AssistId}; use test_utils::tested_by; +use crate::{AssistContext, AssistId, Assists}; + // Assist: change_visibility // // Adds or changes existing visibility specifier. @@ -22,14 +22,14 @@ use test_utils::tested_by; // ``` // pub(crate) fn frobnicate() {} // ``` -pub(crate) fn change_visibility(ctx: AssistCtx) -> Option { +pub(crate) fn change_visibility(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { if let Some(vis) = ctx.find_node_at_offset::() { - return change_vis(ctx, vis); + return change_vis(acc, vis); } - add_vis(ctx) + add_vis(acc, ctx) } -fn add_vis(ctx: AssistCtx) -> Option { +fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let item_keyword = ctx.token_at_offset().find(|leaf| match leaf.kind() { T![const] | T![fn] | T![mod] | T![struct] | T![enum] | T![trait] => true, _ => false, @@ -66,15 +66,10 @@ fn add_vis(ctx: AssistCtx) -> Option { return None; }; - ctx.add_assist( - AssistId("change_visibility"), - "Change visibility to pub(crate)", - target, - |edit| { - edit.insert(offset, "pub(crate) "); - edit.set_cursor(offset); - }, - ) + acc.add(AssistId("change_visibility"), "Change visibility to pub(crate)", target, |edit| { + edit.insert(offset, "pub(crate) "); + edit.set_cursor(offset); + }) } fn vis_offset(node: &SyntaxNode) -> TextSize { @@ -88,10 +83,10 @@ fn vis_offset(node: &SyntaxNode) -> TextSize { .unwrap_or_else(|| node.text_range().start()) } -fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { +fn change_vis(acc: &mut Assists, vis: ast::Visibility) -> Option<()> { if vis.syntax().text() == "pub" { let target = vis.syntax().text_range(); - return ctx.add_assist( + return acc.add( AssistId("change_visibility"), "Change Visibility to pub(crate)", target, @@ -103,7 +98,7 @@ fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { } if vis.syntax().text() == "pub(crate)" { let target = vis.syntax().text_range(); - return ctx.add_assist( + return acc.add( AssistId("change_visibility"), "Change visibility to pub", target, diff --git a/crates/ra_assists/src/handlers/early_return.rs b/crates/ra_assists/src/handlers/early_return.rs index 4bd6040b2ad..ccf91797c68 100644 --- a/crates/ra_assists/src/handlers/early_return.rs +++ b/crates/ra_assists/src/handlers/early_return.rs @@ -9,7 +9,7 @@ use ra_syntax::{ }; use crate::{ - assist_ctx::{Assist, AssistCtx}, + assist_context::{AssistContext, Assists}, utils::invert_boolean_expression, AssistId, }; @@ -36,7 +36,7 @@ use crate::{ // bar(); // } // ``` -pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { +pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let if_expr: ast::IfExpr = ctx.find_node_at_offset()?; if if_expr.else_branch().is_some() { return None; @@ -96,93 +96,88 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { let cursor_position = ctx.frange.range.start(); let target = if_expr.syntax().text_range(); - ctx.add_assist( - AssistId("convert_to_guarded_return"), - "Convert to guarded return", - target, - |edit| { - let if_indent_level = IndentLevel::from_node(&if_expr.syntax()); - let new_block = match if_let_pat { - None => { - // If. - let new_expr = { - let then_branch = - make::block_expr(once(make::expr_stmt(early_expression).into()), None); - let cond = invert_boolean_expression(cond_expr); - let e = make::expr_if(make::condition(cond, None), then_branch); - if_indent_level.increase_indent(e) - }; - replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) - } - Some((path, bound_ident)) => { - // If-let. - let match_expr = { - let happy_arm = { - let pat = make::tuple_struct_pat( - path, - once(make::bind_pat(make::name("it")).into()), - ); - let expr = { - let name_ref = make::name_ref("it"); - let segment = make::path_segment(name_ref); - let path = make::path_unqualified(segment); - make::expr_path(path) - }; - make::match_arm(once(pat.into()), expr) - }; - - let sad_arm = make::match_arm( - // FIXME: would be cool to use `None` or `Err(_)` if appropriate - once(make::placeholder_pat().into()), - early_expression, - ); - - make::expr_match(cond_expr, make::match_arm_list(vec![happy_arm, sad_arm])) - }; - - let let_stmt = make::let_stmt( - make::bind_pat(make::name(&bound_ident.syntax().to_string())).into(), - Some(match_expr), - ); - let let_stmt = if_indent_level.increase_indent(let_stmt); - replace(let_stmt.syntax(), &then_block, &parent_block, &if_expr) - } - }; - edit.replace_ast(parent_block, ast::BlockExpr::cast(new_block).unwrap()); - edit.set_cursor(cursor_position); - - fn replace( - new_expr: &SyntaxNode, - then_block: &ast::BlockExpr, - parent_block: &ast::BlockExpr, - if_expr: &ast::IfExpr, - ) -> SyntaxNode { - let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); - let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); - let end_of_then = - if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { - end_of_then.prev_sibling_or_token().unwrap() - } else { - end_of_then - }; - let mut then_statements = new_expr.children_with_tokens().chain( - then_block_items - .syntax() - .children_with_tokens() - .skip(1) - .take_while(|i| *i != end_of_then), - ); - replace_children( - &parent_block.syntax(), - RangeInclusive::new( - if_expr.clone().syntax().clone().into(), - if_expr.syntax().clone().into(), - ), - &mut then_statements, - ) + acc.add(AssistId("convert_to_guarded_return"), "Convert to guarded return", target, |edit| { + let if_indent_level = IndentLevel::from_node(&if_expr.syntax()); + let new_block = match if_let_pat { + None => { + // If. + let new_expr = { + let then_branch = + make::block_expr(once(make::expr_stmt(early_expression).into()), None); + let cond = invert_boolean_expression(cond_expr); + let e = make::expr_if(make::condition(cond, None), then_branch); + if_indent_level.increase_indent(e) + }; + replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) } - }, - ) + Some((path, bound_ident)) => { + // If-let. + let match_expr = { + let happy_arm = { + let pat = make::tuple_struct_pat( + path, + once(make::bind_pat(make::name("it")).into()), + ); + let expr = { + let name_ref = make::name_ref("it"); + let segment = make::path_segment(name_ref); + let path = make::path_unqualified(segment); + make::expr_path(path) + }; + make::match_arm(once(pat.into()), expr) + }; + + let sad_arm = make::match_arm( + // FIXME: would be cool to use `None` or `Err(_)` if appropriate + once(make::placeholder_pat().into()), + early_expression, + ); + + make::expr_match(cond_expr, make::match_arm_list(vec![happy_arm, sad_arm])) + }; + + let let_stmt = make::let_stmt( + make::bind_pat(make::name(&bound_ident.syntax().to_string())).into(), + Some(match_expr), + ); + let let_stmt = if_indent_level.increase_indent(let_stmt); + replace(let_stmt.syntax(), &then_block, &parent_block, &if_expr) + } + }; + edit.replace_ast(parent_block, ast::BlockExpr::cast(new_block).unwrap()); + edit.set_cursor(cursor_position); + + fn replace( + new_expr: &SyntaxNode, + then_block: &ast::BlockExpr, + parent_block: &ast::BlockExpr, + if_expr: &ast::IfExpr, + ) -> SyntaxNode { + let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); + let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); + let end_of_then = + if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { + end_of_then.prev_sibling_or_token().unwrap() + } else { + end_of_then + }; + let mut then_statements = new_expr.children_with_tokens().chain( + then_block_items + .syntax() + .children_with_tokens() + .skip(1) + .take_while(|i| *i != end_of_then), + ); + replace_children( + &parent_block.syntax(), + RangeInclusive::new( + if_expr.clone().syntax().clone().into(), + if_expr.syntax().clone().into(), + ), + &mut then_statements, + ) + } + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/fill_match_arms.rs b/crates/ra_assists/src/handlers/fill_match_arms.rs index 7c8f8bdf24c..13c1e7e8014 100644 --- a/crates/ra_assists/src/handlers/fill_match_arms.rs +++ b/crates/ra_assists/src/handlers/fill_match_arms.rs @@ -5,7 +5,7 @@ use itertools::Itertools; use ra_ide_db::RootDatabase; use ra_syntax::ast::{self, make, AstNode, MatchArm, NameOwner, Pat}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: fill_match_arms // @@ -31,7 +31,7 @@ use crate::{Assist, AssistCtx, AssistId}; // } // } // ``` -pub(crate) fn fill_match_arms(ctx: AssistCtx) -> Option { +pub(crate) fn fill_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let match_expr = ctx.find_node_at_offset::()?; let match_arm_list = match_expr.match_arm_list()?; @@ -93,7 +93,7 @@ pub(crate) fn fill_match_arms(ctx: AssistCtx) -> Option { } let target = match_expr.syntax().text_range(); - ctx.add_assist(AssistId("fill_match_arms"), "Fill match arms", target, |edit| { + acc.add(AssistId("fill_match_arms"), "Fill match arms", target, |edit| { let new_arm_list = match_arm_list.remove_placeholder().append_arms(missing_arms); edit.set_cursor(expr.syntax().text_range().start()); edit.replace_ast(match_arm_list, new_arm_list); diff --git a/crates/ra_assists/src/handlers/flip_binexpr.rs b/crates/ra_assists/src/handlers/flip_binexpr.rs index cb7264d7bb3..692ba4895cb 100644 --- a/crates/ra_assists/src/handlers/flip_binexpr.rs +++ b/crates/ra_assists/src/handlers/flip_binexpr.rs @@ -1,6 +1,6 @@ use ra_syntax::ast::{AstNode, BinExpr, BinOp}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: flip_binexpr // @@ -17,7 +17,7 @@ use crate::{Assist, AssistCtx, AssistId}; // let _ = 2 + 90; // } // ``` -pub(crate) fn flip_binexpr(ctx: AssistCtx) -> Option { +pub(crate) fn flip_binexpr(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let expr = ctx.find_node_at_offset::()?; let lhs = expr.lhs()?.syntax().clone(); let rhs = expr.rhs()?.syntax().clone(); @@ -33,7 +33,7 @@ pub(crate) fn flip_binexpr(ctx: AssistCtx) -> Option { return None; } - ctx.add_assist(AssistId("flip_binexpr"), "Flip binary expression", op_range, |edit| { + acc.add(AssistId("flip_binexpr"), "Flip binary expression", op_range, |edit| { if let FlipAction::FlipAndReplaceOp(new_op) = action { edit.replace(op_range, new_op); } diff --git a/crates/ra_assists/src/handlers/flip_comma.rs b/crates/ra_assists/src/handlers/flip_comma.rs index 24982ae2253..dfe2a7fedc0 100644 --- a/crates/ra_assists/src/handlers/flip_comma.rs +++ b/crates/ra_assists/src/handlers/flip_comma.rs @@ -1,6 +1,6 @@ use ra_syntax::{algo::non_trivia_sibling, Direction, T}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: flip_comma // @@ -17,7 +17,7 @@ use crate::{Assist, AssistCtx, AssistId}; // ((3, 4), (1, 2)); // } // ``` -pub(crate) fn flip_comma(ctx: AssistCtx) -> Option { +pub(crate) fn flip_comma(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let comma = ctx.find_token_at_offset(T![,])?; let prev = non_trivia_sibling(comma.clone().into(), Direction::Prev)?; let next = non_trivia_sibling(comma.clone().into(), Direction::Next)?; @@ -28,7 +28,7 @@ pub(crate) fn flip_comma(ctx: AssistCtx) -> Option { return None; } - ctx.add_assist(AssistId("flip_comma"), "Flip comma", comma.text_range(), |edit| { + acc.add(AssistId("flip_comma"), "Flip comma", comma.text_range(), |edit| { edit.replace(prev.text_range(), next.to_string()); edit.replace(next.text_range(), prev.to_string()); }) diff --git a/crates/ra_assists/src/handlers/flip_trait_bound.rs b/crates/ra_assists/src/handlers/flip_trait_bound.rs index 6a3b2df679d..8a08702ab29 100644 --- a/crates/ra_assists/src/handlers/flip_trait_bound.rs +++ b/crates/ra_assists/src/handlers/flip_trait_bound.rs @@ -4,7 +4,7 @@ use ra_syntax::{ Direction, T, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: flip_trait_bound // @@ -17,7 +17,7 @@ use crate::{Assist, AssistCtx, AssistId}; // ``` // fn foo() { } // ``` -pub(crate) fn flip_trait_bound(ctx: AssistCtx) -> Option { +pub(crate) fn flip_trait_bound(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { // We want to replicate the behavior of `flip_binexpr` by only suggesting // the assist when the cursor is on a `+` let plus = ctx.find_token_at_offset(T![+])?; @@ -33,7 +33,7 @@ pub(crate) fn flip_trait_bound(ctx: AssistCtx) -> Option { ); let target = plus.text_range(); - ctx.add_assist(AssistId("flip_trait_bound"), "Flip trait bounds", target, |edit| { + acc.add(AssistId("flip_trait_bound"), "Flip trait bounds", target, |edit| { edit.replace(before.text_range(), after.to_string()); edit.replace(after.text_range(), before.to_string()); }) diff --git a/crates/ra_assists/src/handlers/inline_local_variable.rs b/crates/ra_assists/src/handlers/inline_local_variable.rs index e5765c845ee..5b26814d30a 100644 --- a/crates/ra_assists/src/handlers/inline_local_variable.rs +++ b/crates/ra_assists/src/handlers/inline_local_variable.rs @@ -5,7 +5,10 @@ use ra_syntax::{ }; use test_utils::tested_by; -use crate::{assist_ctx::ActionBuilder, Assist, AssistCtx, AssistId}; +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, +}; // Assist: inline_local_variable // @@ -23,7 +26,7 @@ use crate::{assist_ctx::ActionBuilder, Assist, AssistCtx, AssistId}; // (1 + 2) * 4; // } // ``` -pub(crate) fn inline_local_variable(ctx: AssistCtx) -> Option { +pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let let_stmt = ctx.find_node_at_offset::()?; let bind_pat = match let_stmt.pat()? { ast::Pat::BindPat(pat) => pat, @@ -33,7 +36,7 @@ pub(crate) fn inline_local_variable(ctx: AssistCtx) -> Option { tested_by!(test_not_inline_mut_variable); return None; } - if !bind_pat.syntax().text_range().contains_inclusive(ctx.frange.range.start()) { + if !bind_pat.syntax().text_range().contains_inclusive(ctx.offset()) { tested_by!(not_applicable_outside_of_bind_pat); return None; } @@ -107,20 +110,14 @@ pub(crate) fn inline_local_variable(ctx: AssistCtx) -> Option { let init_in_paren = format!("({})", &init_str); let target = bind_pat.syntax().text_range(); - ctx.add_assist( - AssistId("inline_local_variable"), - "Inline variable", - target, - move |edit: &mut ActionBuilder| { - edit.delete(delete_range); - for (desc, should_wrap) in refs.iter().zip(wrap_in_parens) { - let replacement = - if should_wrap { init_in_paren.clone() } else { init_str.clone() }; - edit.replace(desc.file_range.range, replacement) - } - edit.set_cursor(delete_range.start()) - }, - ) + acc.add(AssistId("inline_local_variable"), "Inline variable", target, move |builder| { + builder.delete(delete_range); + for (desc, should_wrap) in refs.iter().zip(wrap_in_parens) { + let replacement = if should_wrap { init_in_paren.clone() } else { init_str.clone() }; + builder.replace(desc.file_range.range, replacement) + } + builder.set_cursor(delete_range.start()) + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/introduce_variable.rs b/crates/ra_assists/src/handlers/introduce_variable.rs index 3c340ff3b97..fdf3ada0d79 100644 --- a/crates/ra_assists/src/handlers/introduce_variable.rs +++ b/crates/ra_assists/src/handlers/introduce_variable.rs @@ -9,7 +9,7 @@ use ra_syntax::{ use stdx::format_to; use test_utils::tested_by; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: introduce_variable // @@ -27,7 +27,7 @@ use crate::{Assist, AssistCtx, AssistId}; // var_name * 4; // } // ``` -pub(crate) fn introduce_variable(ctx: AssistCtx) -> Option { +pub(crate) fn introduce_variable(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { if ctx.frange.range.is_empty() { return None; } @@ -43,7 +43,7 @@ pub(crate) fn introduce_variable(ctx: AssistCtx) -> Option { return None; } let target = expr.syntax().text_range(); - ctx.add_assist(AssistId("introduce_variable"), "Extract into variable", target, move |edit| { + acc.add(AssistId("introduce_variable"), "Extract into variable", target, move |edit| { let mut buf = String::new(); let cursor_offset = if wrap_in_block { diff --git a/crates/ra_assists/src/handlers/invert_if.rs b/crates/ra_assists/src/handlers/invert_if.rs index b16271443e5..527c7caef1a 100644 --- a/crates/ra_assists/src/handlers/invert_if.rs +++ b/crates/ra_assists/src/handlers/invert_if.rs @@ -3,7 +3,11 @@ use ra_syntax::{ T, }; -use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId}; +use crate::{ + assist_context::{AssistContext, Assists}, + utils::invert_boolean_expression, + AssistId, +}; // Assist: invert_if // @@ -24,7 +28,7 @@ use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId}; // } // ``` -pub(crate) fn invert_if(ctx: AssistCtx) -> Option { +pub(crate) fn invert_if(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let if_keyword = ctx.find_token_at_offset(T![if])?; let expr = ast::IfExpr::cast(if_keyword.parent())?; let if_range = if_keyword.text_range(); @@ -40,21 +44,21 @@ pub(crate) fn invert_if(ctx: AssistCtx) -> Option { let cond = expr.condition()?.expr()?; let then_node = expr.then_branch()?.syntax().clone(); + let else_block = match expr.else_branch()? { + ast::ElseBranch::Block(it) => it, + ast::ElseBranch::IfExpr(_) => return None, + }; - if let ast::ElseBranch::Block(else_block) = expr.else_branch()? { - let cond_range = cond.syntax().text_range(); - let flip_cond = invert_boolean_expression(cond); - let else_node = else_block.syntax(); - let else_range = else_node.text_range(); - let then_range = then_node.text_range(); - return ctx.add_assist(AssistId("invert_if"), "Invert if", if_range, |edit| { - edit.replace(cond_range, flip_cond.syntax().text()); - edit.replace(else_range, then_node.text()); - edit.replace(then_range, else_node.text()); - }); - } - - None + let cond_range = cond.syntax().text_range(); + let flip_cond = invert_boolean_expression(cond); + let else_node = else_block.syntax(); + let else_range = else_node.text_range(); + let then_range = then_node.text_range(); + acc.add(AssistId("invert_if"), "Invert if", if_range, |edit| { + edit.replace(cond_range, flip_cond.syntax().text()); + edit.replace(else_range, then_node.text()); + edit.replace(then_range, else_node.text()); + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs index de74d83d850..8e1d9331238 100644 --- a/crates/ra_assists/src/handlers/merge_imports.rs +++ b/crates/ra_assists/src/handlers/merge_imports.rs @@ -6,7 +6,10 @@ use ra_syntax::{ AstNode, Direction, InsertPosition, SyntaxElement, T, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, +}; // Assist: merge_imports // @@ -20,7 +23,7 @@ use crate::{Assist, AssistCtx, AssistId}; // ``` // use std::{fmt::Formatter, io}; // ``` -pub(crate) fn merge_imports(ctx: AssistCtx) -> Option { +pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let tree: ast::UseTree = ctx.find_node_at_offset()?; let mut rewriter = SyntaxRewriter::default(); let mut offset = ctx.frange.range.start(); @@ -53,10 +56,10 @@ pub(crate) fn merge_imports(ctx: AssistCtx) -> Option { }; let target = tree.syntax().text_range(); - ctx.add_assist(AssistId("merge_imports"), "Merge imports", target, |edit| { - edit.rewrite(rewriter); + acc.add(AssistId("merge_imports"), "Merge imports", target, |builder| { + builder.rewrite(rewriter); // FIXME: we only need because our diff is imprecise - edit.set_cursor(offset); + builder.set_cursor(offset); }) } diff --git a/crates/ra_assists/src/handlers/merge_match_arms.rs b/crates/ra_assists/src/handlers/merge_match_arms.rs index 7c4d9d55d78..cfe4df47bce 100644 --- a/crates/ra_assists/src/handlers/merge_match_arms.rs +++ b/crates/ra_assists/src/handlers/merge_match_arms.rs @@ -6,7 +6,7 @@ use ra_syntax::{ Direction, TextSize, }; -use crate::{Assist, AssistCtx, AssistId, TextRange}; +use crate::{AssistContext, AssistId, Assists, TextRange}; // Assist: merge_match_arms // @@ -32,7 +32,7 @@ use crate::{Assist, AssistCtx, AssistId, TextRange}; // } // } // ``` -pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option { +pub(crate) fn merge_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let current_arm = ctx.find_node_at_offset::()?; // Don't try to handle arms with guards for now - can add support for this later if current_arm.guard().is_some() { @@ -70,7 +70,7 @@ pub(crate) fn merge_match_arms(ctx: AssistCtx) -> Option { return None; } - ctx.add_assist(AssistId("merge_match_arms"), "Merge match arms", current_text_range, |edit| { + acc.add(AssistId("merge_match_arms"), "Merge match arms", current_text_range, |edit| { let pats = if arms_to_merge.iter().any(contains_placeholder) { "_".into() } else { diff --git a/crates/ra_assists/src/handlers/move_bounds.rs b/crates/ra_assists/src/handlers/move_bounds.rs index 44e50cb6e66..a41aacfc3dc 100644 --- a/crates/ra_assists/src/handlers/move_bounds.rs +++ b/crates/ra_assists/src/handlers/move_bounds.rs @@ -5,7 +5,7 @@ use ra_syntax::{ T, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: move_bounds_to_where_clause // @@ -22,7 +22,7 @@ use crate::{Assist, AssistCtx, AssistId}; // f(x) // } // ``` -pub(crate) fn move_bounds_to_where_clause(ctx: AssistCtx) -> Option { +pub(crate) fn move_bounds_to_where_clause(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let type_param_list = ctx.find_node_at_offset::()?; let mut type_params = type_param_list.type_params(); @@ -50,36 +50,29 @@ pub(crate) fn move_bounds_to_where_clause(ctx: AssistCtx) -> Option { }; let target = type_param_list.syntax().text_range(); - ctx.add_assist( - AssistId("move_bounds_to_where_clause"), - "Move to where clause", - target, - |edit| { - let new_params = type_param_list - .type_params() - .filter(|it| it.type_bound_list().is_some()) - .map(|type_param| { - let without_bounds = type_param.remove_bounds(); - (type_param, without_bounds) - }); + acc.add(AssistId("move_bounds_to_where_clause"), "Move to where clause", target, |edit| { + let new_params = type_param_list + .type_params() + .filter(|it| it.type_bound_list().is_some()) + .map(|type_param| { + let without_bounds = type_param.remove_bounds(); + (type_param, without_bounds) + }); - let new_type_param_list = type_param_list.replace_descendants(new_params); - edit.replace_ast(type_param_list.clone(), new_type_param_list); + let new_type_param_list = type_param_list.replace_descendants(new_params); + edit.replace_ast(type_param_list.clone(), new_type_param_list); - let where_clause = { - let predicates = type_param_list.type_params().filter_map(build_predicate); - make::where_clause(predicates) - }; + let where_clause = { + let predicates = type_param_list.type_params().filter_map(build_predicate); + make::where_clause(predicates) + }; - let to_insert = match anchor.prev_sibling_or_token() { - Some(ref elem) if elem.kind() == WHITESPACE => { - format!("{} ", where_clause.syntax()) - } - _ => format!(" {}", where_clause.syntax()), - }; - edit.insert(anchor.text_range().start(), to_insert); - }, - ) + let to_insert = match anchor.prev_sibling_or_token() { + Some(ref elem) if elem.kind() == WHITESPACE => format!("{} ", where_clause.syntax()), + _ => format!(" {}", where_clause.syntax()), + }; + edit.insert(anchor.text_range().start(), to_insert); + }) } fn build_predicate(param: ast::TypeParam) -> Option { diff --git a/crates/ra_assists/src/handlers/move_guard.rs b/crates/ra_assists/src/handlers/move_guard.rs index 29bc9a9ffb4..fc0335b5785 100644 --- a/crates/ra_assists/src/handlers/move_guard.rs +++ b/crates/ra_assists/src/handlers/move_guard.rs @@ -4,7 +4,7 @@ use ra_syntax::{ TextSize, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: move_guard_to_arm_body // @@ -31,7 +31,7 @@ use crate::{Assist, AssistCtx, AssistId}; // } // } // ``` -pub(crate) fn move_guard_to_arm_body(ctx: AssistCtx) -> Option { +pub(crate) fn move_guard_to_arm_body(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let match_arm = ctx.find_node_at_offset::()?; let guard = match_arm.guard()?; let space_before_guard = guard.syntax().prev_sibling_or_token(); @@ -41,7 +41,7 @@ pub(crate) fn move_guard_to_arm_body(ctx: AssistCtx) -> Option { let buf = format!("if {} {{ {} }}", guard_conditions.syntax().text(), arm_expr.syntax().text()); let target = guard.syntax().text_range(); - ctx.add_assist(AssistId("move_guard_to_arm_body"), "Move guard to arm body", target, |edit| { + acc.add(AssistId("move_guard_to_arm_body"), "Move guard to arm body", target, |edit| { let offseting_amount = match space_before_guard.and_then(|it| it.into_token()) { Some(tok) => { if ast::Whitespace::cast(tok.clone()).is_some() { @@ -88,7 +88,7 @@ pub(crate) fn move_guard_to_arm_body(ctx: AssistCtx) -> Option { // } // } // ``` -pub(crate) fn move_arm_cond_to_match_guard(ctx: AssistCtx) -> Option { +pub(crate) fn move_arm_cond_to_match_guard(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let match_arm: MatchArm = ctx.find_node_at_offset::()?; let match_pat = match_arm.pat()?; @@ -109,7 +109,7 @@ pub(crate) fn move_arm_cond_to_match_guard(ctx: AssistCtx) -> Option { let buf = format!(" if {}", cond.syntax().text()); let target = if_expr.syntax().text_range(); - ctx.add_assist( + acc.add( AssistId("move_arm_cond_to_match_guard"), "Move condition to match guard", target, diff --git a/crates/ra_assists/src/handlers/raw_string.rs b/crates/ra_assists/src/handlers/raw_string.rs index 155c679b4b3..c20ffe0b30a 100644 --- a/crates/ra_assists/src/handlers/raw_string.rs +++ b/crates/ra_assists/src/handlers/raw_string.rs @@ -5,7 +5,7 @@ use ra_syntax::{ TextSize, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: make_raw_string // @@ -22,11 +22,11 @@ use crate::{Assist, AssistCtx, AssistId}; // r#"Hello, World!"#; // } // ``` -pub(crate) fn make_raw_string(ctx: AssistCtx) -> Option { +pub(crate) fn make_raw_string(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let token = ctx.find_token_at_offset(STRING).and_then(ast::String::cast)?; let value = token.value()?; let target = token.syntax().text_range(); - ctx.add_assist(AssistId("make_raw_string"), "Rewrite as raw string", target, |edit| { + acc.add(AssistId("make_raw_string"), "Rewrite as raw string", target, |edit| { let max_hash_streak = count_hashes(&value); let mut hashes = String::with_capacity(max_hash_streak + 1); for _ in 0..hashes.capacity() { @@ -51,11 +51,11 @@ pub(crate) fn make_raw_string(ctx: AssistCtx) -> Option { // "Hello, \"World!\""; // } // ``` -pub(crate) fn make_usual_string(ctx: AssistCtx) -> Option { +pub(crate) fn make_usual_string(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let token = ctx.find_token_at_offset(RAW_STRING).and_then(ast::RawString::cast)?; let value = token.value()?; let target = token.syntax().text_range(); - ctx.add_assist(AssistId("make_usual_string"), "Rewrite as regular string", target, |edit| { + acc.add(AssistId("make_usual_string"), "Rewrite as regular string", target, |edit| { // parse inside string to escape `"` let escaped = value.escape_default().to_string(); edit.replace(token.syntax().text_range(), format!("\"{}\"", escaped)); @@ -77,10 +77,10 @@ pub(crate) fn make_usual_string(ctx: AssistCtx) -> Option { // r##"Hello, World!"##; // } // ``` -pub(crate) fn add_hash(ctx: AssistCtx) -> Option { +pub(crate) fn add_hash(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let token = ctx.find_token_at_offset(RAW_STRING)?; let target = token.text_range(); - ctx.add_assist(AssistId("add_hash"), "Add # to raw string", target, |edit| { + acc.add(AssistId("add_hash"), "Add # to raw string", target, |edit| { edit.insert(token.text_range().start() + TextSize::of('r'), "#"); edit.insert(token.text_range().end(), "#"); }) @@ -101,7 +101,7 @@ pub(crate) fn add_hash(ctx: AssistCtx) -> Option { // r"Hello, World!"; // } // ``` -pub(crate) fn remove_hash(ctx: AssistCtx) -> Option { +pub(crate) fn remove_hash(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let token = ctx.find_token_at_offset(RAW_STRING)?; let text = token.text().as_str(); if text.starts_with("r\"") { @@ -109,7 +109,7 @@ pub(crate) fn remove_hash(ctx: AssistCtx) -> Option { return None; } let target = token.text_range(); - ctx.add_assist(AssistId("remove_hash"), "Remove hash from raw string", target, |edit| { + acc.add(AssistId("remove_hash"), "Remove hash from raw string", target, |edit| { let result = &text[2..text.len() - 1]; let result = if result.starts_with('\"') { // FIXME: this logic is wrong, not only the last has has to handled specially diff --git a/crates/ra_assists/src/handlers/remove_dbg.rs b/crates/ra_assists/src/handlers/remove_dbg.rs index e6e02f2aec3..8eef578cf40 100644 --- a/crates/ra_assists/src/handlers/remove_dbg.rs +++ b/crates/ra_assists/src/handlers/remove_dbg.rs @@ -3,7 +3,7 @@ use ra_syntax::{ TextSize, T, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: remove_dbg // @@ -20,7 +20,7 @@ use crate::{Assist, AssistCtx, AssistId}; // 92; // } // ``` -pub(crate) fn remove_dbg(ctx: AssistCtx) -> Option { +pub(crate) fn remove_dbg(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let macro_call = ctx.find_node_at_offset::()?; if !is_valid_macrocall(¯o_call, "dbg")? { @@ -58,7 +58,7 @@ pub(crate) fn remove_dbg(ctx: AssistCtx) -> Option { }; let target = macro_call.syntax().text_range(); - ctx.add_assist(AssistId("remove_dbg"), "Remove dbg!()", target, |edit| { + acc.add(AssistId("remove_dbg"), "Remove dbg!()", target, |edit| { edit.replace(macro_range, macro_content); edit.set_cursor(cursor_pos); }) diff --git a/crates/ra_assists/src/handlers/remove_mut.rs b/crates/ra_assists/src/handlers/remove_mut.rs index 9f72f879d5e..dce546db79d 100644 --- a/crates/ra_assists/src/handlers/remove_mut.rs +++ b/crates/ra_assists/src/handlers/remove_mut.rs @@ -1,6 +1,6 @@ use ra_syntax::{SyntaxKind, TextRange, T}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: remove_mut // @@ -17,7 +17,7 @@ use crate::{Assist, AssistCtx, AssistId}; // fn feed(&self, amount: u32) {} // } // ``` -pub(crate) fn remove_mut(ctx: AssistCtx) -> Option { +pub(crate) fn remove_mut(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let mut_token = ctx.find_token_at_offset(T![mut])?; let delete_from = mut_token.text_range().start(); let delete_to = match mut_token.next_token() { @@ -26,7 +26,7 @@ pub(crate) fn remove_mut(ctx: AssistCtx) -> Option { }; let target = mut_token.text_range(); - ctx.add_assist(AssistId("remove_mut"), "Remove `mut` keyword", target, |edit| { + acc.add(AssistId("remove_mut"), "Remove `mut` keyword", target, |edit| { edit.set_cursor(delete_from); edit.delete(TextRange::new(delete_from, delete_to)); }) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index 0b930dea215..757f6406e91 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -3,18 +3,9 @@ use std::collections::HashMap; use hir::{Adt, ModuleDef, PathResolution, Semantics, Struct}; use itertools::Itertools; use ra_ide_db::RootDatabase; -use ra_syntax::{ - algo, - ast::{self, Path, RecordLit, RecordPat}, - match_ast, AstNode, SyntaxKind, - SyntaxKind::*, - SyntaxNode, -}; +use ra_syntax::{algo, ast, match_ast, AstNode, SyntaxKind, SyntaxKind::*, SyntaxNode}; -use crate::{ - assist_ctx::{Assist, AssistCtx}, - AssistId, -}; +use crate::{AssistContext, AssistId, Assists}; // Assist: reorder_fields // @@ -31,13 +22,13 @@ use crate::{ // const test: Foo = Foo {foo: 1, bar: 0} // ``` // -pub(crate) fn reorder_fields(ctx: AssistCtx) -> Option { - reorder::(ctx.clone()).or_else(|| reorder::(ctx)) +pub(crate) fn reorder_fields(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + reorder::(acc, ctx.clone()).or_else(|| reorder::(acc, ctx)) } -fn reorder(ctx: AssistCtx) -> Option { +fn reorder(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let record = ctx.find_node_at_offset::()?; - let path = record.syntax().children().find_map(Path::cast)?; + let path = record.syntax().children().find_map(ast::Path::cast)?; let ranks = compute_fields_ranks(&path, &ctx)?; @@ -51,7 +42,7 @@ fn reorder(ctx: AssistCtx) -> Option { } let target = record.syntax().text_range(); - ctx.add_assist(AssistId("reorder_fields"), "Reorder record fields", target, |edit| { + acc.add(AssistId("reorder_fields"), "Reorder record fields", target, |edit| { for (old, new) in fields.iter().zip(&sorted_fields) { algo::diff(old, new).into_text_edit(edit.text_edit_builder()); } @@ -96,9 +87,9 @@ fn struct_definition(path: &ast::Path, sema: &Semantics) -> Option } } -fn compute_fields_ranks(path: &Path, ctx: &AssistCtx) -> Option> { +fn compute_fields_ranks(path: &ast::Path, ctx: &AssistContext) -> Option> { Some( - struct_definition(path, ctx.sema)? + struct_definition(path, &ctx.sema)? .fields(ctx.db) .iter() .enumerate() diff --git a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs index 2eb8348f826..a59a06efa54 100644 --- a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs @@ -4,7 +4,7 @@ use ra_syntax::{ AstNode, }; -use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; +use crate::{utils::TryEnum, AssistContext, AssistId, Assists}; // Assist: replace_if_let_with_match // @@ -32,7 +32,7 @@ use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; // } // } // ``` -pub(crate) fn replace_if_let_with_match(ctx: AssistCtx) -> Option { +pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let if_expr: ast::IfExpr = ctx.find_node_at_offset()?; let cond = if_expr.condition()?; let pat = cond.pat()?; @@ -43,36 +43,31 @@ pub(crate) fn replace_if_let_with_match(ctx: AssistCtx) -> Option { ast::ElseBranch::IfExpr(_) => return None, }; - let sema = ctx.sema; let target = if_expr.syntax().text_range(); - ctx.add_assist( - AssistId("replace_if_let_with_match"), - "Replace with match", - target, - move |edit| { - let match_expr = { - let then_arm = { - let then_expr = unwrap_trivial_block(then_block); - make::match_arm(vec![pat.clone()], then_expr) - }; - let else_arm = { - let pattern = sema - .type_of_pat(&pat) - .and_then(|ty| TryEnum::from_ty(sema, &ty)) - .map(|it| it.sad_pattern()) - .unwrap_or_else(|| make::placeholder_pat().into()); - let else_expr = unwrap_trivial_block(else_block); - make::match_arm(vec![pattern], else_expr) - }; - make::expr_match(expr, make::match_arm_list(vec![then_arm, else_arm])) + acc.add(AssistId("replace_if_let_with_match"), "Replace with match", target, move |edit| { + let match_expr = { + let then_arm = { + let then_expr = unwrap_trivial_block(then_block); + make::match_arm(vec![pat.clone()], then_expr) }; + let else_arm = { + let pattern = ctx + .sema + .type_of_pat(&pat) + .and_then(|ty| TryEnum::from_ty(&ctx.sema, &ty)) + .map(|it| it.sad_pattern()) + .unwrap_or_else(|| make::placeholder_pat().into()); + let else_expr = unwrap_trivial_block(else_block); + make::match_arm(vec![pattern], else_expr) + }; + make::expr_match(expr, make::match_arm_list(vec![then_arm, else_arm])) + }; - let match_expr = IndentLevel::from_node(if_expr.syntax()).increase_indent(match_expr); + let match_expr = IndentLevel::from_node(if_expr.syntax()).increase_indent(match_expr); - edit.set_cursor(if_expr.syntax().text_range().start()); - edit.replace_ast::(if_expr.into(), match_expr); - }, - ) + edit.set_cursor(if_expr.syntax().text_range().start()); + edit.replace_ast::(if_expr.into(), match_expr); + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs index a5509a56735..d3f214591af 100644 --- a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs +++ b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs @@ -9,11 +9,7 @@ use ra_syntax::{ AstNode, T, }; -use crate::{ - assist_ctx::{Assist, AssistCtx}, - utils::TryEnum, - AssistId, -}; +use crate::{utils::TryEnum, AssistContext, AssistId, Assists}; // Assist: replace_let_with_if_let // @@ -39,16 +35,16 @@ use crate::{ // // fn compute() -> Option { None } // ``` -pub(crate) fn replace_let_with_if_let(ctx: AssistCtx) -> Option { +pub(crate) fn replace_let_with_if_let(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let let_kw = ctx.find_token_at_offset(T![let])?; let let_stmt = let_kw.ancestors().find_map(ast::LetStmt::cast)?; let init = let_stmt.initializer()?; let original_pat = let_stmt.pat()?; let ty = ctx.sema.type_of_expr(&init)?; - let happy_variant = TryEnum::from_ty(ctx.sema, &ty).map(|it| it.happy_case()); + let happy_variant = TryEnum::from_ty(&ctx.sema, &ty).map(|it| it.happy_case()); let target = let_kw.text_range(); - ctx.add_assist(AssistId("replace_let_with_if_let"), "Replace with if-let", target, |edit| { + acc.add(AssistId("replace_let_with_if_let"), "Replace with if-let", target, |edit| { let with_placeholder: ast::Pat = match happy_variant { None => make::placeholder_pat().into(), Some(var_name) => make::tuple_struct_pat( diff --git a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs index fd41da64b8d..1a81d8a0e02 100644 --- a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs @@ -1,11 +1,7 @@ use hir; use ra_syntax::{ast, AstNode, SmolStr, TextRange}; -use crate::{ - assist_ctx::{Assist, AssistCtx}, - utils::insert_use_statement, - AssistId, -}; +use crate::{utils::insert_use_statement, AssistContext, AssistId, Assists}; // Assist: replace_qualified_name_with_use // @@ -20,7 +16,10 @@ use crate::{ // // fn process(map: HashMap) {} // ``` -pub(crate) fn replace_qualified_name_with_use(ctx: AssistCtx) -> Option { +pub(crate) fn replace_qualified_name_with_use( + acc: &mut Assists, + ctx: &AssistContext, +) -> Option<()> { let path: ast::Path = ctx.find_node_at_offset()?; // We don't want to mess with use statements if path.syntax().ancestors().find_map(ast::UseItem::cast).is_some() { @@ -34,18 +33,18 @@ pub(crate) fn replace_qualified_name_with_use(ctx: AssistCtx) -> Option } let target = path.syntax().text_range(); - ctx.add_assist( + acc.add( AssistId("replace_qualified_name_with_use"), "Replace qualified path with use", target, - |edit| { + |builder| { let path_to_import = hir_path.mod_path().clone(); - insert_use_statement(path.syntax(), &path_to_import, edit); + insert_use_statement(path.syntax(), &path_to_import, ctx, builder); if let Some(last) = path.segment() { // Here we are assuming the assist will provide a correct use statement // so we can delete the path qualifier - edit.delete(TextRange::new( + builder.delete(TextRange::new( path.syntax().text_range().start(), last.syntax().text_range().start(), )); diff --git a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs index c6b73da67b0..a46998b8eb0 100644 --- a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs @@ -5,7 +5,7 @@ use ra_syntax::{ AstNode, }; -use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; +use crate::{utils::TryEnum, AssistContext, AssistId, Assists}; // Assist: replace_unwrap_with_match // @@ -29,7 +29,7 @@ use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; // }; // } // ``` -pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option { +pub(crate) fn replace_unwrap_with_match(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let method_call: ast::MethodCallExpr = ctx.find_node_at_offset()?; let name = method_call.name_ref()?; if name.text() != "unwrap" { @@ -37,33 +37,26 @@ pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option { } let caller = method_call.expr()?; let ty = ctx.sema.type_of_expr(&caller)?; - let happy_variant = TryEnum::from_ty(ctx.sema, &ty)?.happy_case(); + let happy_variant = TryEnum::from_ty(&ctx.sema, &ty)?.happy_case(); let target = method_call.syntax().text_range(); - ctx.add_assist( - AssistId("replace_unwrap_with_match"), - "Replace unwrap with match", - target, - |edit| { - let ok_path = make::path_unqualified(make::path_segment(make::name_ref(happy_variant))); - let it = make::bind_pat(make::name("a")).into(); - let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); + acc.add(AssistId("replace_unwrap_with_match"), "Replace unwrap with match", target, |edit| { + let ok_path = make::path_unqualified(make::path_segment(make::name_ref(happy_variant))); + let it = make::bind_pat(make::name("a")).into(); + let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); - let bind_path = make::path_unqualified(make::path_segment(make::name_ref("a"))); - let ok_arm = make::match_arm(iter::once(ok_tuple), make::expr_path(bind_path)); + let bind_path = make::path_unqualified(make::path_segment(make::name_ref("a"))); + let ok_arm = make::match_arm(iter::once(ok_tuple), make::expr_path(bind_path)); - let unreachable_call = make::unreachable_macro_call().into(); - let err_arm = - make::match_arm(iter::once(make::placeholder_pat().into()), unreachable_call); + let unreachable_call = make::unreachable_macro_call().into(); + let err_arm = make::match_arm(iter::once(make::placeholder_pat().into()), unreachable_call); - let match_arm_list = make::match_arm_list(vec![ok_arm, err_arm]); - let match_expr = make::expr_match(caller.clone(), match_arm_list); - let match_expr = - IndentLevel::from_node(method_call.syntax()).increase_indent(match_expr); + let match_arm_list = make::match_arm_list(vec![ok_arm, err_arm]); + let match_expr = make::expr_match(caller.clone(), match_arm_list); + let match_expr = IndentLevel::from_node(method_call.syntax()).increase_indent(match_expr); - edit.set_cursor(caller.syntax().text_range().start()); - edit.replace_ast::(method_call.into(), match_expr); - }, - ) + edit.set_cursor(caller.syntax().text_range().start()); + edit.replace_ast::(method_call.into(), match_expr); + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/handlers/split_import.rs b/crates/ra_assists/src/handlers/split_import.rs index d4956397460..159033731f0 100644 --- a/crates/ra_assists/src/handlers/split_import.rs +++ b/crates/ra_assists/src/handlers/split_import.rs @@ -2,7 +2,7 @@ use std::iter::successors; use ra_syntax::{ast, AstNode, T}; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; // Assist: split_import // @@ -15,7 +15,7 @@ use crate::{Assist, AssistCtx, AssistId}; // ``` // use std::{collections::HashMap}; // ``` -pub(crate) fn split_import(ctx: AssistCtx) -> Option { +pub(crate) fn split_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let colon_colon = ctx.find_token_at_offset(T![::])?; let path = ast::Path::cast(colon_colon.parent())?.qualifier()?; let top_path = successors(Some(path.clone()), |it| it.parent_path()).last()?; @@ -29,7 +29,7 @@ pub(crate) fn split_import(ctx: AssistCtx) -> Option { let cursor = ctx.frange.range.start(); let target = colon_colon.text_range(); - ctx.add_assist(AssistId("split_import"), "Split import", target, |edit| { + acc.add(AssistId("split_import"), "Split import", target, |edit| { edit.replace_ast(use_tree, new_tree); edit.set_cursor(cursor); }) diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 6df927abbdb..eba0631a4c2 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -1,4 +1,4 @@ -use crate::{Assist, AssistCtx, AssistId}; +use crate::{AssistContext, AssistId, Assists}; use ast::LoopBodyOwner; use ra_fmt::unwrap_trivial_block; @@ -21,7 +21,7 @@ use ra_syntax::{ast, match_ast, AstNode, TextRange, T}; // println!("foo"); // } // ``` -pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { +pub(crate) fn unwrap_block(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let l_curly_token = ctx.find_token_at_offset(T!['{'])?; let block = ast::BlockExpr::cast(l_curly_token.parent())?; let parent = block.syntax().parent()?; @@ -58,7 +58,7 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { }; let target = expr_to_unwrap.syntax().text_range(); - ctx.add_assist(AssistId("unwrap_block"), "Unwrap block", target, |edit| { + acc.add(AssistId("unwrap_block"), "Unwrap block", target, |edit| { edit.set_cursor(expr.syntax().text_range().start()); let pat_start: &[_] = &[' ', '{', '\n']; diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 0473fd8c200..01161376276 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -10,7 +10,7 @@ macro_rules! eprintln { ($($tt:tt)*) => { stdx::eprintln!($($tt)*) }; } -mod assist_ctx; +mod assist_context; mod marks; #[cfg(test)] mod tests; @@ -22,7 +22,7 @@ use ra_db::FileRange; use ra_ide_db::{source_change::SourceChange, RootDatabase}; use ra_syntax::TextRange; -pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; +pub(crate) use crate::assist_context::{AssistContext, Assists}; /// Unique identifier of the assist, should not be shown to the user /// directly. @@ -68,13 +68,12 @@ pub struct ResolvedAssist { /// returned, without actual edits. pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { let sema = Semantics::new(db); - let ctx = AssistCtx::new(&sema, range, false); - handlers::all() - .iter() - .filter_map(|f| f(ctx.clone())) - .flat_map(|it| it.0) - .map(|a| a.label) - .collect() + let ctx = AssistContext::new(sema, range); + let mut acc = Assists::new_unresolved(&ctx); + handlers::all().iter().for_each(|handler| { + handler(&mut acc, &ctx); + }); + acc.finish_unresolved() } /// Return all the assists applicable at the given position. @@ -83,31 +82,30 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec Vec { let sema = Semantics::new(db); - let ctx = AssistCtx::new(&sema, range, true); - let mut a = handlers::all() - .iter() - .filter_map(|f| f(ctx.clone())) - .flat_map(|it| it.0) - .map(|it| it.into_resolved().unwrap()) - .collect::>(); - a.sort_by_key(|it| it.label.target.len()); - a + let ctx = AssistContext::new(sema, range); + let mut acc = Assists::new_resolved(&ctx); + handlers::all().iter().for_each(|handler| { + handler(&mut acc, &ctx); + }); + acc.finish_resolved() } mod handlers { - use crate::{Assist, AssistCtx}; + use crate::{AssistContext, Assists}; - pub(crate) type Handler = fn(AssistCtx) -> Option; + pub(crate) type Handler = fn(&mut Assists, &AssistContext) -> Option<()>; mod add_custom_impl; mod add_derive; mod add_explicit_type; + mod add_from_impl_for_enum; mod add_function; mod add_impl; mod add_missing_impl_members; mod add_new; mod apply_demorgan; mod auto_import; + mod change_return_type_to_result; mod change_visibility; mod early_return; mod fill_match_arms; @@ -124,14 +122,12 @@ mod handlers { mod raw_string; mod remove_dbg; mod remove_mut; + mod reorder_fields; mod replace_if_let_with_match; mod replace_let_with_if_let; mod replace_qualified_name_with_use; mod replace_unwrap_with_match; mod split_import; - mod change_return_type_to_result; - mod add_from_impl_for_enum; - mod reorder_fields; mod unwrap_block; pub(crate) fn all() -> &'static [Handler] { diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index 17e3ece9f60..45b2d9733e0 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -11,7 +11,7 @@ use test_utils::{ RangeOrOffset, }; -use crate::{handlers::Handler, resolved_assists, AssistCtx}; +use crate::{handlers::Handler, resolved_assists, AssistContext, Assists}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -71,7 +71,7 @@ enum ExpectedResult<'a> { Target(&'a str), } -fn check(assist: Handler, before: &str, expected: ExpectedResult) { +fn check(handler: Handler, before: &str, expected: ExpectedResult) { let (text_without_caret, file_with_caret_id, range_or_offset, db) = if before.contains("//-") { let (mut db, position) = RootDatabase::with_position(before); db.set_local_roots(Arc::new(vec![db.file_source_root(position.file_id)])); @@ -90,17 +90,20 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) { let frange = FileRange { file_id: file_with_caret_id, range: range_or_offset.into() }; let sema = Semantics::new(&db); - let assist_ctx = AssistCtx::new(&sema, frange, true); - - match (assist(assist_ctx), expected) { + let ctx = AssistContext::new(sema, frange); + let mut acc = Assists::new_resolved(&ctx); + handler(&mut acc, &ctx); + let mut res = acc.finish_resolved(); + let assist = res.pop(); + match (assist, expected) { (Some(assist), ExpectedResult::After(after)) => { - let mut action = assist.0[0].source_change.clone().unwrap(); - let change = action.source_file_edits.pop().unwrap(); + let mut source_change = assist.source_change; + let change = source_change.source_file_edits.pop().unwrap(); let mut actual = db.file_text(change.file_id).as_ref().to_owned(); change.edit.apply(&mut actual); - match action.cursor_position { + match source_change.cursor_position { None => { if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset { let off = change @@ -116,7 +119,7 @@ fn check(assist: Handler, before: &str, expected: ExpectedResult) { assert_eq_text!(after, &actual); } (Some(assist), ExpectedResult::Target(target)) => { - let range = assist.0[0].label.target; + let range = assist.label.target; assert_eq_text!(&text_without_caret[range], target); } (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"), diff --git a/crates/ra_assists/src/utils/insert_use.rs b/crates/ra_assists/src/utils/insert_use.rs index c1f447efe7a..1214e3cd47a 100644 --- a/crates/ra_assists/src/utils/insert_use.rs +++ b/crates/ra_assists/src/utils/insert_use.rs @@ -2,7 +2,6 @@ // FIXME: rewrite according to the plan, outlined in // https://github.com/rust-analyzer/rust-analyzer/issues/3301#issuecomment-592931553 -use crate::assist_ctx::ActionBuilder; use hir::{self, ModPath}; use ra_syntax::{ ast::{self, NameOwner}, @@ -12,6 +11,8 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; +use crate::assist_context::{AssistBuilder, AssistContext}; + /// Creates and inserts a use statement for the given path to import. /// The use statement is inserted in the scope most appropriate to the /// the cursor position given, additionally merged with the existing use imports. @@ -19,10 +20,11 @@ pub(crate) fn insert_use_statement( // Ideally the position of the cursor, used to position: &SyntaxNode, path_to_import: &ModPath, - edit: &mut ActionBuilder, + ctx: &AssistContext, + builder: &mut AssistBuilder, ) { let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::>(); - let container = edit.ctx().sema.ancestors_with_macros(position.clone()).find_map(|n| { + let container = ctx.sema.ancestors_with_macros(position.clone()).find_map(|n| { if let Some(module) = ast::Module::cast(n.clone()) { return module.item_list().map(|it| it.syntax().clone()); } @@ -31,7 +33,7 @@ pub(crate) fn insert_use_statement( if let Some(container) = container { let action = best_action_for_target(container, position.clone(), &target); - make_assist(&action, &target, edit.text_edit_builder()); + make_assist(&action, &target, builder.text_edit_builder()); } } From c839d4f7a9e667b5aca30388c43fa8b1dbab14a7 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 7 May 2020 16:52:14 +0200 Subject: [PATCH 34/46] do not show runnables for main function outside of a binary target #4356 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../rust-analyzer/src/main_loop/handlers.rs | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 15e8305f884..f4353af6470 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -42,6 +42,7 @@ use crate::{ world::WorldSnapshot, LspError, Result, }; +use ra_project_model::TargetKind; pub fn handle_analyzer_status(world: WorldSnapshot, _: ()) -> Result { let _p = profile("handle_analyzer_status"); @@ -384,16 +385,27 @@ pub fn handle_runnables( let offset = params.position.map(|it| it.conv_with(&line_index)); let mut res = Vec::new(); let workspace_root = world.workspace_root_for(file_id); + let cargo_spec = CargoTargetSpec::for_file(&world, file_id)?; for runnable in world.analysis().runnables(file_id)? { if let Some(offset) = offset { if !runnable.range.contains_inclusive(offset) { continue; } } + // Do not suggest binary run on other target than binary + if let RunnableKind::Bin = runnable.kind { + if let Some(spec) = &cargo_spec { + match spec.target_kind { + TargetKind::Bin => {} + _ => continue, + } + } + } res.push(to_lsp_runnable(&world, file_id, runnable)?); } + // Add `cargo check` and `cargo test` for the whole package - match CargoTargetSpec::for_file(&world, file_id)? { + match cargo_spec { Some(spec) => { for &cmd in ["check", "test"].iter() { res.push(req::Runnable { @@ -831,13 +843,23 @@ pub fn handle_code_lens( let mut lenses: Vec = Default::default(); + let cargo_spec = CargoTargetSpec::for_file(&world, file_id)?; // Gather runnables for runnable in world.analysis().runnables(file_id)? { let title = match &runnable.kind { RunnableKind::Test { .. } | RunnableKind::TestMod { .. } => "▶️\u{fe0e}Run Test", RunnableKind::DocTest { .. } => "▶️\u{fe0e}Run Doctest", RunnableKind::Bench { .. } => "Run Bench", - RunnableKind::Bin => "Run", + RunnableKind::Bin => { + // Do not suggest binary run on other target than binary + match &cargo_spec { + Some(spec) => match spec.target_kind { + TargetKind::Bin => "Run", + _ => continue, + }, + None => continue, + } + } } .to_string(); let mut r = to_lsp_runnable(&world, file_id, runnable)?; From c6b81bc013b5278b917d109b723405e0df413323 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 7 May 2020 17:09:59 +0200 Subject: [PATCH 35/46] Rename AssitLabel -> Assist --- crates/ra_assists/src/assist_context.rs | 16 ++++++++-------- crates/ra_assists/src/lib.rs | 12 ++++++------ crates/ra_assists/src/tests.rs | 14 +++++++------- crates/ra_ide/src/lib.rs | 6 +++--- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 203ad127386..81052ab493a 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -15,7 +15,7 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; -use crate::{AssistId, AssistLabel, GroupLabel, ResolvedAssist}; +use crate::{Assist, AssistId, GroupLabel, ResolvedAssist}; /// `AssistContext` allows to apply an assist or check if it could be applied. /// @@ -91,7 +91,7 @@ impl<'a> AssistContext<'a> { pub(crate) struct Assists { resolve: bool, file: FileId, - buf: Vec<(AssistLabel, Option)>, + buf: Vec<(Assist, Option)>, } impl Assists { @@ -102,7 +102,7 @@ impl Assists { Assists { resolve: false, file: ctx.frange.file_id, buf: Vec::new() } } - pub(crate) fn finish_unresolved(self) -> Vec { + pub(crate) fn finish_unresolved(self) -> Vec { assert!(!self.resolve); self.finish() .into_iter() @@ -117,7 +117,7 @@ impl Assists { assert!(self.resolve); self.finish() .into_iter() - .map(|(label, edit)| ResolvedAssist { label, source_change: edit.unwrap() }) + .map(|(label, edit)| ResolvedAssist { assist: label, source_change: edit.unwrap() }) .collect() } @@ -128,7 +128,7 @@ impl Assists { target: TextRange, f: impl FnOnce(&mut AssistBuilder), ) -> Option<()> { - let label = AssistLabel::new(id, label.into(), None, target); + let label = Assist::new(id, label.into(), None, target); self.add_impl(label, f) } pub(crate) fn add_group( @@ -139,10 +139,10 @@ impl Assists { target: TextRange, f: impl FnOnce(&mut AssistBuilder), ) -> Option<()> { - let label = AssistLabel::new(id, label.into(), Some(group.clone()), target); + let label = Assist::new(id, label.into(), Some(group.clone()), target); self.add_impl(label, f) } - fn add_impl(&mut self, label: AssistLabel, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { + fn add_impl(&mut self, label: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { let change_label = label.label.clone(); let source_change = if self.resolve { let mut builder = AssistBuilder::new(self.file); @@ -156,7 +156,7 @@ impl Assists { Some(()) } - fn finish(mut self) -> Vec<(AssistLabel, Option)> { + fn finish(mut self) -> Vec<(Assist, Option)> { self.buf.sort_by_key(|(label, _edit)| label.target.len()); self.buf } diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 01161376276..a91975d8cca 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -30,7 +30,7 @@ pub(crate) use crate::assist_context::{AssistContext, Assists}; pub struct AssistId(pub &'static str); #[derive(Debug, Clone)] -pub struct AssistLabel { +pub struct Assist { pub id: AssistId, /// Short description of the assist, as shown in the UI. pub label: String, @@ -43,22 +43,22 @@ pub struct AssistLabel { #[derive(Clone, Debug)] pub struct GroupLabel(pub String); -impl AssistLabel { +impl Assist { pub(crate) fn new( id: AssistId, label: String, group: Option, target: TextRange, - ) -> AssistLabel { + ) -> Assist { // FIXME: make fields private, so that this invariant can't be broken assert!(label.starts_with(|c: char| c.is_uppercase())); - AssistLabel { id, label, group, target } + Assist { id, label, group, target } } } #[derive(Debug, Clone)] pub struct ResolvedAssist { - pub label: AssistLabel, + pub assist: Assist, pub source_change: SourceChange, } @@ -66,7 +66,7 @@ pub struct ResolvedAssist { /// /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. -pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { +pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { let sema = Semantics::new(db); let ctx = AssistContext::new(sema, range); let mut acc = Assists::new_unresolved(&ctx); diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index 45b2d9733e0..a81c54d0721 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -43,14 +43,14 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { let mut assist = resolved_assists(&db, frange) .into_iter() - .find(|assist| assist.label.id.0 == assist_id) + .find(|assist| assist.assist.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, resolved_assists(&db, frange) .into_iter() - .map(|assist| assist.label.id.0) + .map(|assist| assist.assist.id.0) .collect::>() .join(", ") ) @@ -119,7 +119,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult) { assert_eq_text!(after, &actual); } (Some(assist), ExpectedResult::Target(target)) => { - let range = assist.label.target; + let range = assist.assist.target; assert_eq_text!(&text_without_caret[range], target); } (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"), @@ -140,10 +140,10 @@ fn assist_order_field_struct() { let mut assists = assists.iter(); assert_eq!( - assists.next().expect("expected assist").label.label, + assists.next().expect("expected assist").assist.label, "Change visibility to pub(crate)" ); - assert_eq!(assists.next().expect("expected assist").label.label, "Add `#[derive]`"); + assert_eq!(assists.next().expect("expected assist").assist.label, "Add `#[derive]`"); } #[test] @@ -162,6 +162,6 @@ fn assist_order_if_expr() { let assists = resolved_assists(&db, frange); let mut assists = assists.iter(); - assert_eq!(assists.next().expect("expected assist").label.label, "Extract into variable"); - assert_eq!(assists.next().expect("expected assist").label.label, "Replace with match"); + assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); + assert_eq!(assists.next().expect("expected assist").assist.label, "Replace with match"); } diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 737f8710919..0e15f1ccd91 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -475,9 +475,9 @@ impl Analysis { ra_assists::resolved_assists(db, frange) .into_iter() .map(|assist| Assist { - id: assist.label.id, - label: assist.label.label, - group_label: assist.label.group.map(|it| it.0), + id: assist.assist.id, + label: assist.assist.label, + group_label: assist.assist.group.map(|it| it.0), source_change: assist.source_change, }) .collect() From 28fcff125a73ab2fc4aeaa100fc472af5178db20 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 7 May 2020 17:29:23 +0200 Subject: [PATCH 36/46] Nicer API --- crates/ra_assists/src/lib.rs | 72 +++++++++++++++++----------------- crates/ra_assists/src/tests.rs | 10 ++--- crates/ra_ide/src/lib.rs | 2 +- 3 files changed, 42 insertions(+), 42 deletions(-) diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index a91975d8cca..b6dc7cb1bfc 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -29,6 +29,9 @@ pub(crate) use crate::assist_context::{AssistContext, Assists}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct AssistId(pub &'static str); +#[derive(Clone, Debug)] +pub struct GroupLabel(pub String); + #[derive(Debug, Clone)] pub struct Assist { pub id: AssistId, @@ -40,10 +43,41 @@ pub struct Assist { pub target: TextRange, } -#[derive(Clone, Debug)] -pub struct GroupLabel(pub String); +#[derive(Debug, Clone)] +pub struct ResolvedAssist { + pub assist: Assist, + pub source_change: SourceChange, +} impl Assist { + /// Return all the assists applicable at the given position. + /// + /// Assists are returned in the "unresolved" state, that is only labels are + /// returned, without actual edits. + pub fn unresolved(db: &RootDatabase, range: FileRange) -> Vec { + let sema = Semantics::new(db); + let ctx = AssistContext::new(sema, range); + let mut acc = Assists::new_unresolved(&ctx); + handlers::all().iter().for_each(|handler| { + handler(&mut acc, &ctx); + }); + acc.finish_unresolved() + } + + /// Return all the assists applicable at the given position. + /// + /// Assists are returned in the "resolved" state, that is with edit fully + /// computed. + pub fn resolved(db: &RootDatabase, range: FileRange) -> Vec { + let sema = Semantics::new(db); + let ctx = AssistContext::new(sema, range); + let mut acc = Assists::new_resolved(&ctx); + handlers::all().iter().for_each(|handler| { + handler(&mut acc, &ctx); + }); + acc.finish_resolved() + } + pub(crate) fn new( id: AssistId, label: String, @@ -56,40 +90,6 @@ impl Assist { } } -#[derive(Debug, Clone)] -pub struct ResolvedAssist { - pub assist: Assist, - pub source_change: SourceChange, -} - -/// Return all the assists applicable at the given position. -/// -/// Assists are returned in the "unresolved" state, that is only labels are -/// returned, without actual edits. -pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec { - let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, range); - let mut acc = Assists::new_unresolved(&ctx); - handlers::all().iter().for_each(|handler| { - handler(&mut acc, &ctx); - }); - acc.finish_unresolved() -} - -/// Return all the assists applicable at the given position. -/// -/// Assists are returned in the "resolved" state, that is with edit fully -/// computed. -pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec { - let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, range); - let mut acc = Assists::new_resolved(&ctx); - handlers::all().iter().for_each(|handler| { - handler(&mut acc, &ctx); - }); - acc.finish_resolved() -} - mod handlers { use crate::{AssistContext, Assists}; diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index a81c54d0721..a3eacb8f115 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -11,7 +11,7 @@ use test_utils::{ RangeOrOffset, }; -use crate::{handlers::Handler, resolved_assists, AssistContext, Assists}; +use crate::{handlers::Handler, Assist, AssistContext, Assists}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -41,14 +41,14 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { let (db, file_id) = crate::tests::with_single_file(&before); let frange = FileRange { file_id, range: selection.into() }; - let mut assist = resolved_assists(&db, frange) + let mut assist = Assist::resolved(&db, frange) .into_iter() .find(|assist| assist.assist.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, - resolved_assists(&db, frange) + Assist::resolved(&db, frange) .into_iter() .map(|assist| assist.assist.id.0) .collect::>() @@ -136,7 +136,7 @@ fn assist_order_field_struct() { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::empty(before_cursor_pos) }; - let assists = resolved_assists(&db, frange); + let assists = Assist::resolved(&db, frange); let mut assists = assists.iter(); assert_eq!( @@ -159,7 +159,7 @@ fn assist_order_if_expr() { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; - let assists = resolved_assists(&db, frange); + let assists = Assist::resolved(&db, frange); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 0e15f1ccd91..915199bd878 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -472,7 +472,7 @@ impl Analysis { /// position. pub fn assists(&self, frange: FileRange) -> Cancelable> { self.with_db(|db| { - ra_assists::resolved_assists(db, frange) + ra_assists::Assist::resolved(db, frange) .into_iter() .map(|assist| Assist { id: assist.assist.id, From 1e790ea3149f085e49cb66e6a052920f72da01e9 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 7 May 2020 17:32:01 +0200 Subject: [PATCH 37/46] Simplify --- crates/ra_assists/src/assist_context.rs | 3 +-- crates/ra_assists/src/handlers/early_return.rs | 2 +- crates/ra_assists/src/handlers/merge_imports.rs | 2 +- crates/ra_assists/src/handlers/merge_match_arms.rs | 2 +- crates/ra_assists/src/handlers/split_import.rs | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 81052ab493a..3085c4330c3 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -76,8 +76,7 @@ impl<'a> AssistContext<'a> { find_node_at_offset(self.source_file.syntax(), self.offset()) } pub(crate) fn find_node_at_offset_with_descend(&self) -> Option { - self.sema - .find_node_at_offset_with_descend(self.source_file.syntax(), self.frange.range.start()) + self.sema.find_node_at_offset_with_descend(self.source_file.syntax(), self.offset()) } pub(crate) fn covering_element(&self) -> SyntaxElement { find_covering_element(self.source_file.syntax(), self.frange.range) diff --git a/crates/ra_assists/src/handlers/early_return.rs b/crates/ra_assists/src/handlers/early_return.rs index ccf91797c68..810784ad575 100644 --- a/crates/ra_assists/src/handlers/early_return.rs +++ b/crates/ra_assists/src/handlers/early_return.rs @@ -93,7 +93,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) } then_block.syntax().last_child_or_token().filter(|t| t.kind() == R_CURLY)?; - let cursor_position = ctx.frange.range.start(); + let cursor_position = ctx.offset(); let target = if_expr.syntax().text_range(); acc.add(AssistId("convert_to_guarded_return"), "Convert to guarded return", target, |edit| { diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs index 8e1d9331238..ac3e53c2734 100644 --- a/crates/ra_assists/src/handlers/merge_imports.rs +++ b/crates/ra_assists/src/handlers/merge_imports.rs @@ -26,7 +26,7 @@ use crate::{ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let tree: ast::UseTree = ctx.find_node_at_offset()?; let mut rewriter = SyntaxRewriter::default(); - let mut offset = ctx.frange.range.start(); + let mut offset = ctx.offset(); if let Some(use_item) = tree.syntax().parent().and_then(ast::UseItem::cast) { let (merged, to_delete) = next_prev() diff --git a/crates/ra_assists/src/handlers/merge_match_arms.rs b/crates/ra_assists/src/handlers/merge_match_arms.rs index cfe4df47bce..d4e38aa6a59 100644 --- a/crates/ra_assists/src/handlers/merge_match_arms.rs +++ b/crates/ra_assists/src/handlers/merge_match_arms.rs @@ -45,7 +45,7 @@ pub(crate) fn merge_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option InExpr(TextSize), InPat(TextSize), } - let cursor_pos = ctx.frange.range.start(); + let cursor_pos = ctx.offset(); let cursor_pos = if current_expr.syntax().text_range().contains(cursor_pos) { CursorPos::InExpr(current_text_range.end() - cursor_pos) } else { diff --git a/crates/ra_assists/src/handlers/split_import.rs b/crates/ra_assists/src/handlers/split_import.rs index 159033731f0..b2757e50ce7 100644 --- a/crates/ra_assists/src/handlers/split_import.rs +++ b/crates/ra_assists/src/handlers/split_import.rs @@ -26,7 +26,7 @@ pub(crate) fn split_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> if new_tree == use_tree { return None; } - let cursor = ctx.frange.range.start(); + let cursor = ctx.offset(); let target = colon_colon.text_range(); acc.add(AssistId("split_import"), "Split import", target, |edit| { From 2904311664e0aeb2faa3324616083e1bcf7333fc Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 7 May 2020 18:46:58 +0200 Subject: [PATCH 38/46] Use the correct color for structs This works around https://github.com/microsoft/vscode/issues/97162 --- editors/code/package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/editors/code/package.json b/editors/code/package.json index 12a08ba4001..6935fa7a5ae 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -589,6 +589,9 @@ "union": [ "entity.name.union" ], + "struct": [ + "entity.name.type.struct" + ], "keyword.unsafe": [ "keyword.other.unsafe" ], From 3077eae2a61f97c28c0d4e3456f6ab873126e5b8 Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Thu, 7 May 2020 12:06:44 -0700 Subject: [PATCH 39/46] use home crate instead of dirs --- Cargo.lock | 87 +++++----------------------------------- crates/ra_env/Cargo.toml | 2 +- crates/ra_env/src/lib.rs | 2 +- 3 files changed, 13 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28424b7d4da..000c26c4dd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,12 +21,6 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33954243bd79057c2de7338850b85983a44588021f8a5fee574a8888c6de4344" -[[package]] -name = "arrayref" -version = "0.3.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4c527152e37cf757a3f78aae5a06fbeefdb07ccc535c980a3208ee3060dd544" - [[package]] name = "arrayvec" version = "0.5.1" @@ -72,12 +66,6 @@ dependencies = [ "libc", ] -[[package]] -name = "base64" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b41b7ea54a0c9d92199de89e20e58d49f02f8e699814ef3fdf266f6f748d15c7" - [[package]] name = "base64" version = "0.12.0" @@ -90,17 +78,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" -[[package]] -name = "blake2b_simd" -version = "0.5.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8fb2d74254a3a0b5cac33ac9f8ed0e44aa50378d9dbb2e5d83bd21ed1dc2c8a" -dependencies = [ - "arrayref", - "arrayvec", - "constant_time_eq", -] - [[package]] name = "bstr" version = "0.2.12" @@ -235,12 +212,6 @@ dependencies = [ "winapi 0.3.8", ] -[[package]] -name = "constant_time_eq" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" - [[package]] name = "crossbeam" version = "0.7.3" @@ -318,28 +289,6 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198" -[[package]] -name = "dirs" -version = "2.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13aea89a5c93364a98e9b37b2fa237effbb694d5cfe01c5b70941f7eb087d5e3" -dependencies = [ - "cfg-if", - "dirs-sys", -] - -[[package]] -name = "dirs-sys" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afa0b23de8fd801745c471deffa6e12d248f962c9fd4b4c33787b055599bde7b" -dependencies = [ - "cfg-if", - "libc", - "redox_users", - "winapi 0.3.8", -] - [[package]] name = "drop_bomb" version = "0.1.4" @@ -515,6 +464,15 @@ dependencies = [ "libc", ] +[[package]] +name = "home" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2456aef2e6b6a9784192ae780c0f15bc57df0e918585282325e8c8ac27737654" +dependencies = [ + "winapi 0.3.8", +] + [[package]] name = "idna" version = "0.2.0" @@ -710,7 +668,7 @@ version = "0.74.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0e6a2b8837d27b29deb3f3e6dc1c6d2f57947677f9be1024e482ec5b59525" dependencies = [ - "base64 0.12.0", + "base64", "bitflags", "serde", "serde_json", @@ -1004,7 +962,7 @@ name = "ra_env" version = "0.1.0" dependencies = [ "anyhow", - "dirs", + "home", ] [[package]] @@ -1359,17 +1317,6 @@ version = "0.1.56" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84" -[[package]] -name = "redox_users" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09b23093265f8d200fa7b4c2c76297f47e681c655f6f1285a8780d6a022f7431" -dependencies = [ - "getrandom", - "redox_syscall", - "rust-argon2", -] - [[package]] name = "regex" version = "1.3.7" @@ -1454,18 +1401,6 @@ dependencies = [ "winapi 0.3.8", ] -[[package]] -name = "rust-argon2" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bc8af4bda8e1ff4932523b94d3dd20ee30a87232323eda55903ffd71d2fb017" -dependencies = [ - "base64 0.11.0", - "blake2b_simd", - "constant_time_eq", - "crossbeam-utils", -] - [[package]] name = "rustc-ap-rustc_lexer" version = "656.0.0" diff --git a/crates/ra_env/Cargo.toml b/crates/ra_env/Cargo.toml index 7fed446a701..f0a401be536 100644 --- a/crates/ra_env/Cargo.toml +++ b/crates/ra_env/Cargo.toml @@ -6,4 +6,4 @@ authors = ["rust-analyzer developers"] [dependencies] anyhow = "1.0.26" -dirs = "2.0" +home = "0.5.3" diff --git a/crates/ra_env/src/lib.rs b/crates/ra_env/src/lib.rs index 4f94bffdee1..413da19827e 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_env/src/lib.rs @@ -36,7 +36,7 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result Date: Thu, 7 May 2020 16:22:53 -0400 Subject: [PATCH 40/46] Update deps --- Cargo.lock | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d50a766fe5..264b9b7fb7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -68,9 +68,9 @@ dependencies = [ [[package]] name = "base64" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d5ca2cd0adc3f48f9e9ea5a6bbdf9ccc0bfade884847e484d452414c7ccffb3" +checksum = "53d1ccbaf7d9ec9537465a97bf19edc1a4e158ecb49fc16178202238c569cc42" [[package]] name = "bitflags" @@ -342,9 +342,9 @@ dependencies = [ [[package]] name = "filetime" -version = "0.2.9" +version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f59efc38004c988e4201d11d263b8171f49a2e7ec0bdbb71773433f271504a5e" +checksum = "affc17579b132fc2461adf7c575cc6e8b134ebca52c51f5411388965227dc695" dependencies = [ "cfg-if", "libc", @@ -610,18 +610,18 @@ checksum = "99e85c08494b21a9054e7fe1374a732aeadaff3980b6990b94bfd3a70f690005" [[package]] name = "libloading" -version = "0.6.1" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c4f51b790f5bdb65acb4cc94bb81d7b2ee60348a5431ac1467d390b017600b0" +checksum = "2cadb8e769f070c45df05c78c7520eb4cd17061d4ab262e43cfc68b4d00ac71c" dependencies = [ "winapi 0.3.8", ] [[package]] name = "linked-hash-map" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae91b68aebc4ddb91978b11a1b02ddd8602a05ec19002801c5666000e05e0f83" +checksum = "8dd5a6d5999d9907cda8ed67bbd137d3af8085216c2ac62de5be860bd41f304a" [[package]] name = "lock_api" @@ -824,9 +824,9 @@ dependencies = [ [[package]] name = "paste" -version = "0.1.11" +version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3c897744f63f34f7ae3a024d9162bb5001f4ad661dd24bea0dc9f075d2de1c6" +checksum = "0a229b1c58c692edcaa5b9b0948084f130f55d2dcc15b02fcc5340b2b4521476" dependencies = [ "paste-impl", "proc-macro-hack", @@ -834,9 +834,9 @@ dependencies = [ [[package]] name = "paste-impl" -version = "0.1.11" +version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66fd6f92e3594f2dd7b3fc23e42d82e292f7bcda6d8e5dcd167072327234ab89" +checksum = "2e0bf239e447e67ff6d16a8bb5e4d4bd2343acf5066061c0e8e06ac5ba8ca68c" dependencies = [ "proc-macro-hack", "proc-macro2", @@ -886,9 +886,9 @@ checksum = "0d659fe7c6d27f25e9d80a1a094c223f5246f6a6596453e09d7229bf42750b63" [[package]] name = "proc-macro2" -version = "1.0.10" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df246d292ff63439fea9bc8c0a270bed0e390d5ebd4db4ba15aba81111b5abe3" +checksum = "8872cf6f48eee44265156c111456a700ab3483686b3f96df4cf5481c89157319" dependencies = [ "unicode-xid", ] @@ -1581,9 +1581,9 @@ checksum = "ab16ced94dbd8a46c82fd81e3ed9a8727dac2977ea869d217bcc4ea1f122e81f" [[package]] name = "syn" -version = "1.0.18" +version = "1.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "410a7488c0a728c7ceb4ad59b9567eb4053d02e8cc7f5c0e0eeeb39518369213" +checksum = "e8e5aa70697bb26ee62214ae3288465ecec0000f05182f039b477001f08f5ae7" dependencies = [ "proc-macro2", "quote", From d3110859ba4e97cf17d2c997befa92fb63bfb138 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 02:56:53 +0200 Subject: [PATCH 41/46] Move feature desugaring to the right abstraction layer --- crates/ra_cfg/src/lib.rs | 16 ---------------- crates/ra_project_model/src/lib.rs | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/crates/ra_cfg/src/lib.rs b/crates/ra_cfg/src/lib.rs index 697a0458147..57feabcb27c 100644 --- a/crates/ra_cfg/src/lib.rs +++ b/crates/ra_cfg/src/lib.rs @@ -2,8 +2,6 @@ mod cfg_expr; -use std::iter::IntoIterator; - use ra_syntax::SmolStr; use rustc_hash::FxHashSet; @@ -48,18 +46,4 @@ impl CfgOptions { pub fn insert_key_value(&mut self, key: SmolStr, value: SmolStr) { self.key_values.insert((key, value)); } - - /// Shortcut to set features - pub fn insert_features(&mut self, iter: impl IntoIterator) { - iter.into_iter().for_each(|feat| self.insert_key_value("feature".into(), feat)); - } - - /// Shortcut to set cfgs - pub fn insert_cfgs(&mut self, iter: impl IntoIterator) { - iter.into_iter().for_each(|cfg| match cfg.find('=') { - Some(split) => self - .insert_key_value(cfg[0..split].into(), cfg[split + 1..].trim_matches('"').into()), - None => self.insert_atom(cfg), - }); - } } diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 8703429d445..c226ffa57f1 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -398,8 +398,18 @@ impl ProjectWorkspace { let edition = cargo[pkg].edition; let cfg_options = { let mut opts = default_cfg_options.clone(); - opts.insert_features(cargo[pkg].features.iter().map(Into::into)); - opts.insert_cfgs(cargo[pkg].cfgs.iter().map(Into::into)); + for feature in cargo[pkg].features.iter() { + opts.insert_key_value("feature".into(), feature.into()); + } + for cfg in cargo[pkg].cfgs.iter() { + match cfg.find('=') { + Some(split) => opts.insert_key_value( + cfg[..split].into(), + cfg[split + 1..].trim_matches('"').into(), + ), + None => opts.insert_atom(cfg.into()), + }; + } opts }; let mut env = Env::default(); From 3bf5ef02c0dc3087fb4cdd0a794892edde359a0d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 09:28:15 +0200 Subject: [PATCH 42/46] Add master config for inlayHints to make disabling easy --- editors/code/package.json | 5 +++++ editors/code/src/config.ts | 1 + editors/code/src/inlay_hints.ts | 14 +++++++------- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/editors/code/package.json b/editors/code/package.json index 6935fa7a5ae..853fc513b08 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -300,6 +300,11 @@ "default": true, "markdownDescription": "Check with all features (will be passed as `--all-features`)" }, + "rust-analyzer.inlayHints.enable": { + "type": "boolean", + "default": true, + "description": "Disable all inlay hints" + }, "rust-analyzer.inlayHints.typeHints": { "type": "boolean", "default": true, diff --git a/editors/code/src/config.ts b/editors/code/src/config.ts index 110e541800e..46de922f337 100644 --- a/editors/code/src/config.ts +++ b/editors/code/src/config.ts @@ -94,6 +94,7 @@ export class Config { get inlayHints() { return { + enable: this.get("inlayHints.enable"), typeHints: this.get("inlayHints.typeHints"), parameterHints: this.get("inlayHints.parameterHints"), chainingHints: this.get("inlayHints.chainingHints"), diff --git a/editors/code/src/inlay_hints.ts b/editors/code/src/inlay_hints.ts index a0953179706..a2b07d00378 100644 --- a/editors/code/src/inlay_hints.ts +++ b/editors/code/src/inlay_hints.ts @@ -10,13 +10,13 @@ export function activateInlayHints(ctx: Ctx) { const maybeUpdater = { updater: null as null | HintsUpdater, async onConfigChange() { - if ( - !ctx.config.inlayHints.typeHints && - !ctx.config.inlayHints.parameterHints && - !ctx.config.inlayHints.chainingHints - ) { - return this.dispose(); - } + const anyEnabled = ctx.config.inlayHints.typeHints + || ctx.config.inlayHints.parameterHints + || ctx.config.inlayHints.chainingHints; + const enabled = ctx.config.inlayHints.enable && anyEnabled; + + if (!enabled) return this.dispose(); + await sleep(100); if (this.updater) { this.updater.syncCacheAndRenderHints(); From 6713be0b130670324c61d9deb38b7b6aee6a8bac Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 12:25:36 +0200 Subject: [PATCH 43/46] Rename ra_env -> ra_toolchain --- Cargo.lock | 20 +++++++++---------- crates/ra_flycheck/Cargo.toml | 2 +- crates/ra_flycheck/src/lib.rs | 2 +- crates/ra_project_model/Cargo.toml | 2 +- .../ra_project_model/src/cargo_workspace.rs | 2 +- crates/ra_project_model/src/lib.rs | 2 +- crates/ra_project_model/src/sysroot.rs | 4 ++-- crates/{ra_env => ra_toolchain}/Cargo.toml | 2 +- crates/{ra_env => ra_toolchain}/src/lib.rs | 8 +++++--- 9 files changed, 23 insertions(+), 21 deletions(-) rename crates/{ra_env => ra_toolchain}/Cargo.toml (85%) rename crates/{ra_env => ra_toolchain}/src/lib.rs (97%) diff --git a/Cargo.lock b/Cargo.lock index 36cff6402f5..656969c8740 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -957,14 +957,6 @@ dependencies = [ "test_utils", ] -[[package]] -name = "ra_env" -version = "0.1.0" -dependencies = [ - "anyhow", - "home", -] - [[package]] name = "ra_flycheck" version = "0.1.0" @@ -975,7 +967,7 @@ dependencies = [ "jod-thread", "log", "lsp-types", - "ra_env", + "ra_toolchain", "serde_json", ] @@ -1180,8 +1172,8 @@ dependencies = [ "ra_arena", "ra_cfg", "ra_db", - "ra_env", "ra_proc_macro", + "ra_toolchain", "rustc-hash", "serde", "serde_json", @@ -1213,6 +1205,14 @@ dependencies = [ "text-size", ] +[[package]] +name = "ra_toolchain" +version = "0.1.0" +dependencies = [ + "anyhow", + "home", +] + [[package]] name = "ra_tt" version = "0.1.0" diff --git a/crates/ra_flycheck/Cargo.toml b/crates/ra_flycheck/Cargo.toml index d0f7fb2dcf2..03e55714844 100644 --- a/crates/ra_flycheck/Cargo.toml +++ b/crates/ra_flycheck/Cargo.toml @@ -14,7 +14,7 @@ log = "0.4.8" cargo_metadata = "0.9.1" serde_json = "1.0.48" jod-thread = "0.1.1" -ra_env = { path = "../ra_env" } +ra_toolchain = { path = "../ra_toolchain" } [dev-dependencies] insta = "0.16.0" diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index d8b727b0ead..561657edbda 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -16,7 +16,7 @@ use lsp_types::{ CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, }; -use ra_env::get_path_for_executable; +use ra_toolchain::get_path_for_executable; use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; diff --git a/crates/ra_project_model/Cargo.toml b/crates/ra_project_model/Cargo.toml index 62647846898..a32a5daabde 100644 --- a/crates/ra_project_model/Cargo.toml +++ b/crates/ra_project_model/Cargo.toml @@ -16,7 +16,7 @@ cargo_metadata = "0.9.1" ra_arena = { path = "../ra_arena" } ra_cfg = { path = "../ra_cfg" } ra_db = { path = "../ra_db" } -ra_env = { path = "../ra_env" } +ra_toolchain = { path = "../ra_toolchain" } ra_proc_macro = { path = "../ra_proc_macro" } serde = { version = "1.0.106", features = ["derive"] } diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index eb9f33ee89e..9683bfcc0df 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -11,7 +11,7 @@ use anyhow::{Context, Result}; use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId}; use ra_arena::{Arena, Idx}; use ra_db::Edition; -use ra_env::get_path_for_executable; +use ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; /// `CargoWorkspace` represents the logical structure of, well, a Cargo diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 88a6ffb2a0d..4f0b9c77ee6 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -14,7 +14,7 @@ use std::{ use anyhow::{bail, Context, Result}; use ra_cfg::CfgOptions; use ra_db::{CrateGraph, CrateName, Edition, Env, ExternSource, ExternSourceId, FileId}; -use ra_env::get_path_for_executable; +use ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; use serde_json::from_reader; diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 11c26ad8911..2b628c2a3e5 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -1,14 +1,14 @@ //! FIXME: write short doc here -use anyhow::{bail, Context, Result}; use std::{ env, ops, path::{Path, PathBuf}, process::{Command, Output}, }; +use anyhow::{bail, Context, Result}; use ra_arena::{Arena, Idx}; -use ra_env::get_path_for_executable; +use ra_toolchain::get_path_for_executable; #[derive(Default, Debug, Clone)] pub struct Sysroot { diff --git a/crates/ra_env/Cargo.toml b/crates/ra_toolchain/Cargo.toml similarity index 85% rename from crates/ra_env/Cargo.toml rename to crates/ra_toolchain/Cargo.toml index f0a401be536..fbad1073e0d 100644 --- a/crates/ra_env/Cargo.toml +++ b/crates/ra_toolchain/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2018" -name = "ra_env" +name = "ra_toolchain" version = "0.1.0" authors = ["rust-analyzer developers"] diff --git a/crates/ra_env/src/lib.rs b/crates/ra_toolchain/src/lib.rs similarity index 97% rename from crates/ra_env/src/lib.rs rename to crates/ra_toolchain/src/lib.rs index 413da19827e..110c92c889e 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_toolchain/src/lib.rs @@ -1,11 +1,13 @@ //! This crate contains a single public function //! [`get_path_for_executable`](fn.get_path_for_executable.html). //! See docs there for more information. +use std::{ + env, + path::{Path, PathBuf}, + process::Command, +}; use anyhow::{bail, Result}; -use std::env; -use std::path::{Path, PathBuf}; -use std::process::Command; /// Return a `PathBuf` to use for the given executable. /// From 7c0409e0c7249fe793b5d05829fcd984d06ec770 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 12:34:39 +0200 Subject: [PATCH 44/46] Cleanup --- crates/ra_toolchain/src/lib.rs | 57 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/crates/ra_toolchain/src/lib.rs b/crates/ra_toolchain/src/lib.rs index 110c92c889e..afa76619d8e 100644 --- a/crates/ra_toolchain/src/lib.rs +++ b/crates/ra_toolchain/src/lib.rs @@ -14,7 +14,8 @@ use anyhow::{bail, Result}; /// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that /// gives a valid Cargo executable; or it may return a full path to a valid /// Cargo. -pub fn get_path_for_executable(executable_name: impl AsRef) -> Result { +pub fn get_path_for_executable(executable_name: &'static str) -> Result { + assert!(executable_name == "rustc" || executable_name == "cargo"); // The current implementation checks three places for an executable to use: // 1) Appropriate environment variable (erroring if this is set but not a usable executable) // example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc @@ -23,46 +24,46 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result` // example: for cargo, this tries ~/.cargo/bin/cargo // It seems that this is a reasonable place to try for cargo, rustc, and rustup - let executable_name = executable_name.as_ref(); let env_var = executable_name.to_ascii_uppercase(); if let Ok(path) = env::var(&env_var) { - if is_valid_executable(&path) { + return if is_valid_executable(&path) { Ok(path.into()) } else { bail!( "`{}` environment variable points to something that's not a valid executable", env_var ) - } - } else { - if is_valid_executable(executable_name) { - return Ok(executable_name.into()); - } - if let Some(mut path) = ::home::home_dir() { - path.push(".cargo"); - path.push("bin"); - path.push(executable_name); - if is_valid_executable(&path) { - return Ok(path); - } - } - // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly - // for VSCode, even if they are set correctly in a terminal. - // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode - // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; - // but launching VSCode from Dock does not inherit environment variables from a terminal. - // For more discussion, see #3118. - bail!( - "Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", - executable_name, executable_name, env_var - ) + }; } + + if is_valid_executable(executable_name) { + return Ok(executable_name.into()); + } + + if let Some(mut path) = home::home_dir() { + path.push(".cargo"); + path.push("bin"); + path.push(executable_name); + if is_valid_executable(&path) { + return Ok(path); + } + } + // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly + // for VSCode, even if they are set correctly in a terminal. + // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode + // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; + // but launching VSCode from Dock does not inherit environment variables from a terminal. + // For more discussion, see #3118. + bail!( + "Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", + executable_name, executable_name, env_var + ) } /// Does the given `Path` point to a usable executable? /// /// (assumes the executable takes a `--version` switch and writes to stdout, /// which is true for `cargo`, `rustc`, and `rustup`) -fn is_valid_executable(p: impl AsRef) -> bool { - Command::new(p.as_ref()).arg("--version").output().is_ok() +fn is_valid_executable(p: &'static str) -> bool { + Command::new(p).arg("--version").output().is_ok() } From ecff5dc141046c5b9e40639657247a05fb9b0344 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 14:54:29 +0200 Subject: [PATCH 45/46] Cleanup --- Cargo.lock | 1 - crates/ra_flycheck/src/lib.rs | 7 +- .../ra_project_model/src/cargo_workspace.rs | 5 +- crates/ra_project_model/src/lib.rs | 32 ++++----- crates/ra_project_model/src/sysroot.rs | 49 +++---------- crates/ra_toolchain/Cargo.toml | 1 - crates/ra_toolchain/src/lib.rs | 69 +++++++++---------- 7 files changed, 64 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 656969c8740..41855f22e85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1209,7 +1209,6 @@ dependencies = [ name = "ra_toolchain" version = "0.1.0" dependencies = [ - "anyhow", "home", ] diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index 561657edbda..68dcee2851d 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -16,7 +16,6 @@ use lsp_types::{ CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, }; -use ra_toolchain::get_path_for_executable; use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; @@ -216,10 +215,10 @@ impl FlycheckThread { let mut cmd = match &self.config { FlycheckConfig::CargoCommand { command, all_targets, all_features, extra_args } => { - let mut cmd = Command::new(get_path_for_executable("cargo").unwrap()); + let mut cmd = Command::new(ra_toolchain::cargo()); cmd.arg(command); - cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]); - cmd.arg(self.workspace_root.join("Cargo.toml")); + cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]) + .arg(self.workspace_root.join("Cargo.toml")); if *all_targets { cmd.arg("--all-targets"); } diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index 9683bfcc0df..082af4f9699 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -11,7 +11,6 @@ use anyhow::{Context, Result}; use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId}; use ra_arena::{Arena, Idx}; use ra_db::Edition; -use ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; /// `CargoWorkspace` represents the logical structure of, well, a Cargo @@ -147,7 +146,7 @@ impl CargoWorkspace { cargo_features: &CargoConfig, ) -> Result { let mut meta = MetadataCommand::new(); - meta.cargo_path(get_path_for_executable("cargo")?); + meta.cargo_path(ra_toolchain::cargo()); meta.manifest_path(cargo_toml); if cargo_features.all_features { meta.features(CargoOpt::AllFeatures); @@ -289,7 +288,7 @@ pub fn load_extern_resources( cargo_toml: &Path, cargo_features: &CargoConfig, ) -> Result { - let mut cmd = Command::new(get_path_for_executable("cargo")?); + let mut cmd = Command::new(ra_toolchain::cargo()); cmd.args(&["check", "--message-format=json", "--manifest-path"]).arg(cargo_toml); if cargo_features.all_features { cmd.arg("--all-features"); diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 4f0b9c77ee6..5a0a87ed7d4 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -8,13 +8,12 @@ use std::{ fs::{read_dir, File, ReadDir}, io::{self, BufReader}, path::{Path, PathBuf}, - process::Command, + process::{Command, Output}, }; use anyhow::{bail, Context, Result}; use ra_cfg::CfgOptions; use ra_db::{CrateGraph, CrateName, Edition, Env, ExternSource, ExternSourceId, FileId}; -use ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; use serde_json::from_reader; @@ -568,25 +567,18 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions { } } - match (|| -> Result { + let rustc_cfgs = || -> Result { // `cfg(test)` and `cfg(debug_assertion)` are handled outside, so we suppress them here. - let mut cmd = Command::new(get_path_for_executable("rustc")?); + let mut cmd = Command::new(ra_toolchain::rustc()); cmd.args(&["--print", "cfg", "-O"]); if let Some(target) = target { cmd.args(&["--target", target.as_str()]); } - let output = cmd.output().context("Failed to get output from rustc --print cfg -O")?; - if !output.status.success() { - bail!( - "rustc --print cfg -O exited with exit code ({})", - output - .status - .code() - .map_or(String::from("no exit code"), |code| format!("{}", code)) - ); - } + let output = output(cmd)?; Ok(String::from_utf8(output.stdout)?) - })() { + }(); + + match rustc_cfgs { Ok(rustc_cfgs) => { for line in rustc_cfgs.lines() { match line.find('=') { @@ -599,8 +591,16 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions { } } } - Err(e) => log::error!("failed to get rustc cfgs: {}", e), + Err(e) => log::error!("failed to get rustc cfgs: {:#}", e), } cfg_options } + +fn output(mut cmd: Command) -> Result { + let output = cmd.output().with_context(|| format!("{:?} failed", cmd))?; + if !output.status.success() { + bail!("{:?} failed, {}", cmd, output.status) + } + Ok(output) +} diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 2b628c2a3e5..a8a196e64c9 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -3,12 +3,13 @@ use std::{ env, ops, path::{Path, PathBuf}, - process::{Command, Output}, + process::Command, }; -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; use ra_arena::{Arena, Idx}; -use ra_toolchain::get_path_for_executable; + +use crate::output; #[derive(Default, Debug, Clone)] pub struct Sysroot { @@ -85,50 +86,22 @@ impl Sysroot { } } -fn create_command_text(program: &str, args: &[&str]) -> String { - format!("{} {}", program, args.join(" ")) -} - -fn run_command_in_cargo_dir( - cargo_toml: impl AsRef, - program: impl AsRef, - args: &[&str], -) -> Result { - let program = program.as_ref().as_os_str().to_str().expect("Invalid Unicode in path"); - let output = Command::new(program) - .current_dir(cargo_toml.as_ref().parent().unwrap()) - .args(args) - .output() - .context(format!("{} failed", create_command_text(program, args)))?; - if !output.status.success() { - match output.status.code() { - Some(code) => bail!( - "failed to run the command: '{}' exited with code {}", - create_command_text(program, args), - code - ), - None => bail!( - "failed to run the command: '{}' terminated by signal", - create_command_text(program, args) - ), - }; - } - Ok(output) -} - fn get_or_install_rust_src(cargo_toml: &Path) -> Result { if let Ok(path) = env::var("RUST_SRC_PATH") { return Ok(path.into()); } - let rustc = get_path_for_executable("rustc")?; - let rustc_output = run_command_in_cargo_dir(cargo_toml, &rustc, &["--print", "sysroot"])?; + let current_dir = cargo_toml.parent().unwrap(); + let mut rustc = Command::new(ra_toolchain::rustc()); + rustc.current_dir(current_dir).args(&["--print", "sysroot"]); + let rustc_output = output(rustc)?; let stdout = String::from_utf8(rustc_output.stdout)?; let sysroot_path = Path::new(stdout.trim()); let src_path = sysroot_path.join("lib/rustlib/src/rust/src"); if !src_path.exists() { - let rustup = get_path_for_executable("rustup")?; - run_command_in_cargo_dir(cargo_toml, &rustup, &["component", "add", "rust-src"])?; + let mut rustup = Command::new(ra_toolchain::rustup()); + rustup.current_dir(current_dir).args(&["component", "add", "rust-src"]); + let _output = output(rustup)?; } if !src_path.exists() { bail!( diff --git a/crates/ra_toolchain/Cargo.toml b/crates/ra_toolchain/Cargo.toml index fbad1073e0d..1873fbe1678 100644 --- a/crates/ra_toolchain/Cargo.toml +++ b/crates/ra_toolchain/Cargo.toml @@ -5,5 +5,4 @@ version = "0.1.0" authors = ["rust-analyzer developers"] [dependencies] -anyhow = "1.0.26" home = "0.5.3" diff --git a/crates/ra_toolchain/src/lib.rs b/crates/ra_toolchain/src/lib.rs index afa76619d8e..3c307a0eace 100644 --- a/crates/ra_toolchain/src/lib.rs +++ b/crates/ra_toolchain/src/lib.rs @@ -1,21 +1,26 @@ //! This crate contains a single public function //! [`get_path_for_executable`](fn.get_path_for_executable.html). //! See docs there for more information. -use std::{ - env, - path::{Path, PathBuf}, - process::Command, -}; +use std::{env, iter, path::PathBuf}; -use anyhow::{bail, Result}; +pub fn cargo() -> PathBuf { + get_path_for_executable("cargo") +} + +pub fn rustc() -> PathBuf { + get_path_for_executable("rustc") +} + +pub fn rustup() -> PathBuf { + get_path_for_executable("rustup") +} /// Return a `PathBuf` to use for the given executable. /// /// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that /// gives a valid Cargo executable; or it may return a full path to a valid /// Cargo. -pub fn get_path_for_executable(executable_name: &'static str) -> Result { - assert!(executable_name == "rustc" || executable_name == "cargo"); +fn get_path_for_executable(executable_name: &'static str) -> PathBuf { // The current implementation checks three places for an executable to use: // 1) Appropriate environment variable (erroring if this is set but not a usable executable) // example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc @@ -25,45 +30,35 @@ pub fn get_path_for_executable(executable_name: &'static str) -> Result // example: for cargo, this tries ~/.cargo/bin/cargo // It seems that this is a reasonable place to try for cargo, rustc, and rustup let env_var = executable_name.to_ascii_uppercase(); - if let Ok(path) = env::var(&env_var) { - return if is_valid_executable(&path) { - Ok(path.into()) - } else { - bail!( - "`{}` environment variable points to something that's not a valid executable", - env_var - ) - }; + if let Some(path) = env::var_os(&env_var) { + return path.into(); } - if is_valid_executable(executable_name) { - return Ok(executable_name.into()); + if lookup_in_path(executable_name) { + return executable_name.into(); } if let Some(mut path) = home::home_dir() { path.push(".cargo"); path.push("bin"); path.push(executable_name); - if is_valid_executable(&path) { - return Ok(path); + if path.is_file() { + return path; } } - // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly - // for VSCode, even if they are set correctly in a terminal. - // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode - // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; - // but launching VSCode from Dock does not inherit environment variables from a terminal. - // For more discussion, see #3118. - bail!( - "Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", - executable_name, executable_name, env_var - ) + executable_name.into() } -/// Does the given `Path` point to a usable executable? -/// -/// (assumes the executable takes a `--version` switch and writes to stdout, -/// which is true for `cargo`, `rustc`, and `rustup`) -fn is_valid_executable(p: &'static str) -> bool { - Command::new(p).arg("--version").output().is_ok() +fn lookup_in_path(exec: &str) -> bool { + let paths = env::var_os("PATH").unwrap_or_default(); + let mut candidates = env::split_paths(&paths).flat_map(|path| { + let candidate = path.join(&exec); + let with_exe = if env::consts::EXE_EXTENSION == "" { + None + } else { + Some(candidate.with_extension(env::consts::EXE_EXTENSION)) + }; + iter::once(candidate).chain(with_exe) + }); + candidates.any(|it| it.is_file()) } From f5177f91ae61e1d812efd6b55a1ec7ab07c7c7e1 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 8 May 2020 15:17:35 +0200 Subject: [PATCH 46/46] Fix type of byte literals They're `&[u8; N]`, not `&[u8]` (see #4374). --- crates/ra_hir_ty/src/infer/expr.rs | 4 ++-- crates/ra_hir_ty/src/tests/method_resolution.rs | 13 +++++++------ crates/ra_hir_ty/src/tests/simple.rs | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index 83f946eeea0..614c352a0c3 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -501,8 +501,8 @@ impl<'a> InferenceContext<'a> { } Literal::ByteString(..) => { let byte_type = Ty::simple(TypeCtor::Int(Uncertain::Known(IntTy::u8()))); - let slice_type = Ty::apply_one(TypeCtor::Slice, byte_type); - Ty::apply_one(TypeCtor::Ref(Mutability::Shared), slice_type) + let array_type = Ty::apply_one(TypeCtor::Array, byte_type); + Ty::apply_one(TypeCtor::Ref(Mutability::Shared), array_type) } Literal::Char(..) => Ty::simple(TypeCtor::Char), Literal::Int(_v, ty) => Ty::simple(TypeCtor::Int((*ty).into())), diff --git a/crates/ra_hir_ty/src/tests/method_resolution.rs b/crates/ra_hir_ty/src/tests/method_resolution.rs index ab87f598a62..67f964ab5d7 100644 --- a/crates/ra_hir_ty/src/tests/method_resolution.rs +++ b/crates/ra_hir_ty/src/tests/method_resolution.rs @@ -17,8 +17,8 @@ impl [T] { #[lang = "slice_alloc"] impl [T] {} -fn test() { - <[_]>::foo(b"foo"); +fn test(x: &[u8]) { + <[_]>::foo(x); } "#), @r###" @@ -26,10 +26,11 @@ fn test() { 56..79 '{ ... }': T 66..73 'loop {}': ! 71..73 '{}': () - 133..160 '{ ...o"); }': () - 139..149 '<[_]>::foo': fn foo(&[u8]) -> u8 - 139..157 '<[_]>:..."foo")': u8 - 150..156 'b"foo"': &[u8] + 131..132 'x': &[u8] + 141..163 '{ ...(x); }': () + 147..157 '<[_]>::foo': fn foo(&[u8]) -> u8 + 147..160 '<[_]>::foo(x)': u8 + 158..159 'x': &[u8] "### ); } diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index 3d3088965de..e17a179004c 100644 --- a/crates/ra_hir_ty/src/tests/simple.rs +++ b/crates/ra_hir_ty/src/tests/simple.rs @@ -414,7 +414,7 @@ fn test() { 27..31 '5f32': f32 37..41 '5f64': f64 47..54 '"hello"': &str - 60..68 'b"bytes"': &[u8] + 60..68 'b"bytes"': &[u8; _] 74..77 ''c'': char 83..87 'b'b'': u8 93..97 '3.14': f64 @@ -422,7 +422,7 @@ fn test() { 113..118 'false': bool 124..128 'true': bool 134..202 'r#" ... "#': &str - 208..218 'br#"yolo"#': &[u8] + 208..218 'br#"yolo"#': &[u8; _] "### ); }