From c463d217a1e001abe6a812f309d93527e28a70c6 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 13:18:47 +0300 Subject: [PATCH 01/38] Add names for diagnostics and add a possibility to disable them --- crates/ra_hir_def/src/diagnostics.rs | 3 +++ crates/ra_hir_expand/src/diagnostics.rs | 18 ++++++++++++++++-- crates/ra_hir_ty/src/diagnostics.rs | 25 +++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 30db48f868..1d28c24e88 100644 --- a/crates/ra_hir_def/src/diagnostics.rs +++ b/crates/ra_hir_def/src/diagnostics.rs @@ -15,6 +15,9 @@ pub struct UnresolvedModule { } impl Diagnostic for UnresolvedModule { + fn name(&self) -> String { + "unresolved-module".to_string() + } fn message(&self) -> String { "unresolved module".to_string() } diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 84ba97b14a..bf9fb081aa 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -14,13 +14,14 @@ //! subsystem provides a separate, non-query-based API which can walk all stored //! values and transform them into instances of `Diagnostic`. -use std::{any::Any, fmt}; +use std::{any::Any, collections::HashSet, fmt}; use ra_syntax::{SyntaxNode, SyntaxNodePtr}; use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { + fn name(&self) -> String; fn message(&self) -> String; fn source(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); @@ -49,10 +50,16 @@ pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, default_callback: Box, + disabled_diagnostics: HashSet, } impl<'a> DiagnosticSink<'a> { pub fn push(&mut self, d: impl Diagnostic) { + if self.disabled_diagnostics.contains(&d.name()) { + // This diagnostic is disabled, ignore it completely. + return; + } + let d: &dyn Diagnostic = &d; self._push(d); } @@ -76,11 +83,12 @@ impl<'a> DiagnosticSink<'a> { pub struct DiagnosticSinkBuilder<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, + disabled_diagnostics: HashSet, } impl<'a> DiagnosticSinkBuilder<'a> { pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new() } + Self { callbacks: Vec::new(), filters: Vec::new(), disabled_diagnostics: HashSet::new() } } pub fn filter bool + 'a>(mut self, cb: F) -> Self { @@ -100,11 +108,17 @@ impl<'a> DiagnosticSinkBuilder<'a> { self } + pub fn disable_diagnostic(mut self, diagnostic: impl Into) -> Self { + self.disabled_diagnostics.insert(diagnostic.into()); + self + } + pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { DiagnosticSink { callbacks: self.callbacks, filters: self.filters, default_callback: Box::new(default_callback), + disabled_diagnostics: self.disabled_diagnostics, } } } diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 977c0525b5..0b3e16ae7a 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -33,6 +33,10 @@ pub struct NoSuchField { } impl Diagnostic for NoSuchField { + fn name(&self) -> String { + "no-such-field".to_string() + } + fn message(&self) -> String { "no such field".to_string() } @@ -64,6 +68,9 @@ pub struct MissingFields { } impl Diagnostic for MissingFields { + fn name(&self) -> String { + "missing-structure-fields".to_string() + } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); for field in &self.missed_fields { @@ -97,6 +104,9 @@ pub struct MissingPatFields { } impl Diagnostic for MissingPatFields { + fn name(&self) -> String { + "missing-pat-fields".to_string() + } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); for field in &self.missed_fields { @@ -120,6 +130,9 @@ pub struct MissingMatchArms { } impl Diagnostic for MissingMatchArms { + fn name(&self) -> String { + "missing-match-arm".to_string() + } fn message(&self) -> String { String::from("Missing match arm") } @@ -138,6 +151,9 @@ pub struct MissingOkInTailExpr { } impl Diagnostic for MissingOkInTailExpr { + fn name(&self) -> String { + "missing-ok-in-tail-expr".to_string() + } fn message(&self) -> String { "wrap return expression in Ok".to_string() } @@ -166,6 +182,9 @@ pub struct BreakOutsideOfLoop { } impl Diagnostic for BreakOutsideOfLoop { + fn name(&self) -> String { + "break-outside-of-loop".to_string() + } fn message(&self) -> String { "break outside of loop".to_string() } @@ -194,6 +213,9 @@ pub struct MissingUnsafe { } impl Diagnostic for MissingUnsafe { + fn name(&self) -> String { + "missing-unsafe".to_string() + } fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") } @@ -224,6 +246,9 @@ pub struct MismatchedArgCount { } impl Diagnostic for MismatchedArgCount { + fn name(&self) -> String { + "mismatched-arg-count".to_string() + } fn message(&self) -> String { let s = if self.expected == 1 { "" } else { "s" }; format!("Expected {} argument{}, found {}", self.expected, s, self.found) From 90857ff8b08d73945598bac12a841559e86402b1 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 14:25:55 +0300 Subject: [PATCH 02/38] Add an AnalysisConfig structure and use it to configure diagnostics run --- crates/ra_hir_expand/src/diagnostics.rs | 17 ++------------ crates/ra_ide/src/diagnostics.rs | 24 +++++++++++++++++--- crates/ra_ide/src/lib.rs | 28 +++++++++++++++++++----- crates/rust-analyzer/src/config.rs | 12 ++++++++-- crates/rust-analyzer/src/global_state.rs | 2 +- 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index bf9fb081aa..ef1d61144e 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -14,7 +14,7 @@ //! subsystem provides a separate, non-query-based API which can walk all stored //! values and transform them into instances of `Diagnostic`. -use std::{any::Any, collections::HashSet, fmt}; +use std::{any::Any, fmt}; use ra_syntax::{SyntaxNode, SyntaxNodePtr}; @@ -50,16 +50,10 @@ pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, default_callback: Box, - disabled_diagnostics: HashSet, } impl<'a> DiagnosticSink<'a> { pub fn push(&mut self, d: impl Diagnostic) { - if self.disabled_diagnostics.contains(&d.name()) { - // This diagnostic is disabled, ignore it completely. - return; - } - let d: &dyn Diagnostic = &d; self._push(d); } @@ -83,12 +77,11 @@ impl<'a> DiagnosticSink<'a> { pub struct DiagnosticSinkBuilder<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, - disabled_diagnostics: HashSet, } impl<'a> DiagnosticSinkBuilder<'a> { pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new(), disabled_diagnostics: HashSet::new() } + Self { callbacks: Vec::new(), filters: Vec::new() } } pub fn filter bool + 'a>(mut self, cb: F) -> Self { @@ -108,17 +101,11 @@ impl<'a> DiagnosticSinkBuilder<'a> { self } - pub fn disable_diagnostic(mut self, diagnostic: impl Into) -> Self { - self.disabled_diagnostics.insert(diagnostic.into()); - self - } - pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { DiagnosticSink { callbacks: self.callbacks, filters: self.filters, default_callback: Box::new(default_callback), - disabled_diagnostics: self.disabled_diagnostics, } } } diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 73c0b82754..33e4f17437 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -21,7 +21,7 @@ use ra_syntax::{ }; use ra_text_edit::{TextEdit, TextEditBuilder}; -use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; +use crate::{AnalysisConfig, Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; #[derive(Debug, Copy, Clone)] pub enum Severity { @@ -33,6 +33,7 @@ pub(crate) fn diagnostics( db: &RootDatabase, file_id: FileId, enable_experimental: bool, + analysis_config: &AnalysisConfig, ) -> Vec { let _p = profile("diagnostics"); let sema = Semantics::new(db); @@ -41,6 +42,7 @@ pub(crate) fn diagnostics( // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { + name: None, range: err.range(), message: format!("Syntax Error: {}", err), severity: Severity::Error, @@ -52,7 +54,7 @@ pub(crate) fn diagnostics( check_struct_shorthand_initialization(&mut res, file_id, &node); } let res = RefCell::new(res); - let mut sink = DiagnosticSinkBuilder::new() + let mut sink_builder = DiagnosticSinkBuilder::new() .on::(|d| { let original_file = d.source().file_id.original_file(db); let fix = Fix::new( @@ -61,6 +63,7 @@ pub(crate) fn diagnostics( .into(), ); res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -95,6 +98,7 @@ pub(crate) fn diagnostics( }; res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -108,6 +112,7 @@ pub(crate) fn diagnostics( let source_change = SourceFileEdit { file_id, edit }.into(); let fix = Fix::new("Wrap with ok", source_change); res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -116,6 +121,7 @@ pub(crate) fn diagnostics( }) .on::(|d| { res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -123,10 +129,20 @@ pub(crate) fn diagnostics( }) }) // Only collect experimental diagnostics when they're enabled. - .filter(|diag| !diag.is_experimental() || enable_experimental) + .filter(|diag| !diag.is_experimental() || enable_experimental); + + if !analysis_config.disabled_diagnostics.is_empty() { + // Do not collect disabled diagnostics. + sink_builder = sink_builder + .filter(|diag| !analysis_config.disabled_diagnostics.contains(&diag.name())); + } + + // Finalize the `DiagnosticSink` building process. + let mut sink = sink_builder // Diagnostics not handled above get no fix and default treatment. .build(|d| { res.borrow_mut().push(Diagnostic { + name: Some(d.name()), message: d.message(), range: sema.diagnostics_range(d).range, severity: Severity::Error, @@ -234,6 +250,7 @@ fn check_unnecessary_braces_in_use_statement( }); acc.push(Diagnostic { + name: None, range, message: "Unnecessary braces in use statement".to_string(), severity: Severity::WeakWarning, @@ -279,6 +296,7 @@ fn check_struct_shorthand_initialization( let edit = edit_builder.finish(); acc.push(Diagnostic { + name: None, range: record_field.syntax().text_range(), message: "Shorthand struct initialization".to_string(), severity: Severity::WeakWarning, diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 0fede0d879..3822b9409d 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -45,7 +45,7 @@ mod syntax_highlighting; mod syntax_tree; mod typing; -use std::sync::Arc; +use std::{collections::HashSet, sync::Arc}; use ra_cfg::CfgOptions; use ra_db::{ @@ -100,8 +100,15 @@ pub use ra_text_edit::{Indel, TextEdit}; pub type Cancelable = Result; +/// Configuration parameters for the analysis run. +#[derive(Debug, Default, Clone)] +pub struct AnalysisConfig { + pub disabled_diagnostics: HashSet, +} + #[derive(Debug)] pub struct Diagnostic { + pub name: Option, pub message: String, pub range: TextRange, pub severity: Severity, @@ -139,11 +146,16 @@ impl RangeInfo { #[derive(Debug)] pub struct AnalysisHost { db: RootDatabase, + config: AnalysisConfig, } impl AnalysisHost { - pub fn new(lru_capacity: Option) -> AnalysisHost { - AnalysisHost { db: RootDatabase::new(lru_capacity) } + pub fn new(lru_capacity: Option) -> Self { + Self::with_config(lru_capacity, AnalysisConfig::default()) + } + + pub fn with_config(lru_capacity: Option, config: AnalysisConfig) -> Self { + AnalysisHost { db: RootDatabase::new(lru_capacity), config } } pub fn update_lru_capacity(&mut self, lru_capacity: Option) { @@ -153,7 +165,7 @@ impl AnalysisHost { /// Returns a snapshot of the current state, which you can query for /// semantic information. pub fn analysis(&self) -> Analysis { - Analysis { db: self.db.snapshot() } + Analysis { db: self.db.snapshot(), config: self.config.clone() } } /// Applies changes to the current state of the world. If there are @@ -197,6 +209,7 @@ impl Default for AnalysisHost { #[derive(Debug)] pub struct Analysis { db: salsa::Snapshot, + config: AnalysisConfig, } // As a general design guideline, `Analysis` API are intended to be independent @@ -492,7 +505,7 @@ impl Analysis { file_id: FileId, enable_experimental: bool, ) -> Cancelable> { - self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental)) + self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental, &self.config)) } /// Returns the edit required to rename reference at the position to the new @@ -518,6 +531,11 @@ impl Analysis { }) } + /// Sets the provided config. + pub fn set_config(&mut self, config: AnalysisConfig) { + self.config = config; + } + /// Performs an operation on that may be Canceled. fn with_db T + std::panic::UnwindSafe, T>( &self, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 70b4512d0b..2c1db95461 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -7,11 +7,11 @@ //! configure the server itself, feature flags are passed into analysis, and //! tweak things like automatic insertion of `()` in completions. -use std::{ffi::OsString, path::PathBuf}; +use std::{collections::HashSet, ffi::OsString, path::PathBuf}; use flycheck::FlycheckConfig; use lsp_types::ClientCapabilities; -use ra_ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; +use ra_ide::{AnalysisConfig, AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; use ra_project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest}; use serde::Deserialize; use vfs::AbsPathBuf; @@ -45,6 +45,8 @@ pub struct Config { pub with_sysroot: bool, pub linked_projects: Vec, pub root_path: AbsPathBuf, + + pub analysis: AnalysisConfig, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -176,6 +178,8 @@ impl Config { hover: HoverConfig::default(), linked_projects: Vec::new(), root_path, + + analysis: AnalysisConfig::default(), } } @@ -293,6 +297,8 @@ impl Config { goto_type_def: data.hoverActions_enable && data.hoverActions_gotoTypeDef, }; + self.analysis = AnalysisConfig { disabled_diagnostics: data.analysis_disabledDiagnostics }; + log::info!("Config::update() = {:#?}", self); } @@ -444,5 +450,7 @@ config_data! { rustfmt_overrideCommand: Option> = None, withSysroot: bool = true, + + analysis_disabledDiagnostics: HashSet = HashSet::new(), } } diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 0e592ac1be..46cb7ebe2e 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -108,7 +108,7 @@ impl GlobalState { Handle { handle, receiver } }; - let analysis_host = AnalysisHost::new(config.lru_capacity); + let analysis_host = AnalysisHost::with_config(config.lru_capacity, config.analysis.clone()); let (flycheck_sender, flycheck_receiver) = unbounded(); GlobalState { sender, From 78c1a87797b93e9f6fac9daecb9e5d5d8b7e60fd Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 14:26:36 +0300 Subject: [PATCH 03/38] Add a test for disabled diagnostics --- crates/ra_ide/src/diagnostics.rs | 58 +++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 33e4f17437..4abf602ade 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -316,7 +316,10 @@ mod tests { use stdx::trim_indent; use test_utils::assert_eq_text; - use crate::mock_analysis::{analysis_and_position, single_file, MockAnalysis}; + use crate::{ + mock_analysis::{analysis_and_position, single_file, MockAnalysis}, + AnalysisConfig, + }; use expect::{expect, Expect}; /// Takes a multi-file input fixture with annotated cursor positions, @@ -380,6 +383,54 @@ mod tests { assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); } + /// Takes a multi-file input fixture with annotated cursor position and the list of disabled diagnostics, + /// and checks that provided diagnostics aren't spawned during analysis. + fn check_disabled_diagnostics( + ra_fixture: &str, + disabled_diagnostics: impl IntoIterator, + ) { + let disabled_diagnostics: std::collections::HashSet<_> = + disabled_diagnostics.into_iter().collect(); + + let mock = MockAnalysis::with_files(ra_fixture); + let files = mock.files().map(|(it, _)| it).collect::>(); + let mut analysis = mock.analysis(); + analysis.set_config(AnalysisConfig { disabled_diagnostics: disabled_diagnostics.clone() }); + + let diagnostics = files + .clone() + .into_iter() + .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .collect::>(); + + // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list. + for diagnostic in diagnostics { + if let Some(name) = diagnostic.name { + assert!(!disabled_diagnostics.contains(&name), "Diagnostic {} is disabled", name); + } + } + + // Then, we must reset the config and repeat the check, so that we'll be sure that without + // config these diagnostics are emitted. + // This is required for tests to not become outdated if e.g. diagnostics name changes: + // without this additional run the test will pass simply because a diagnostic with an old name + // will no longer exist. + analysis.set_config(AnalysisConfig { disabled_diagnostics: Default::default() }); + + let diagnostics = files + .into_iter() + .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .collect::>(); + + assert!( + diagnostics + .into_iter() + .filter_map(|diag| diag.name) + .any(|name| disabled_diagnostics.contains(&name)), + "At least one of the diagnostics was not emitted even without config; are the diagnostics names correct?" + ); + } + fn check_expect(ra_fixture: &str, expect: Expect) { let (analysis, file_id) = single_file(ra_fixture); let diagnostics = analysis.diagnostics(file_id, true).unwrap(); @@ -814,4 +865,9 @@ struct Foo { ", ) } + + #[test] + fn test_disabled_diagnostics() { + check_disabled_diagnostics(r#"mod foo;"#, vec!["unresolved-module".to_string()]); + } } From c51fb7aca531b98e01a8a71a30bb35d1376efe02 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 14:35:43 +0300 Subject: [PATCH 04/38] Fix failing test --- crates/ra_ide/src/diagnostics.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 4abf602ade..8e011a40d8 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -693,6 +693,9 @@ fn test_fn() { expect![[r#" [ Diagnostic { + name: Some( + "unresolved-module", + ), message: "unresolved module", range: 0..8, severity: Error, From b970d675f5072e638ea4c2166e82ca5a580f6c50 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sat, 8 Aug 2020 19:18:56 +0900 Subject: [PATCH 05/38] Reverse document symbols for each scope (#5655) --- crates/rust-analyzer/src/handlers.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 067259e246..9944757adb 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -273,19 +273,24 @@ pub(crate) fn handle_document_symbol( parents.push((doc_symbol, symbol.parent)); } let mut document_symbols = Vec::new(); + // Constructs `document_symbols` from `parents`, in order from the end. while let Some((node, parent)) = parents.pop() { match parent { None => document_symbols.push(node), Some(i) => { - let children = &mut parents[i].0.children; - if children.is_none() { - *children = Some(Vec::new()); - } - children.as_mut().unwrap().push(node); + parents[i].0.children.get_or_insert_with(Vec::new).push(node); } } } + fn reverse(symbols: &mut Vec) { + for sym in symbols.iter_mut() { + sym.children.as_mut().map(|c| reverse(c)); + } + symbols.reverse(); + } + reverse(&mut document_symbols); + let res = if snap.config.client_caps.hierarchical_symbols { document_symbols.into() } else { From cd49e41642c1e0bf2d22157914246d181a608c86 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Sat, 8 Aug 2020 19:34:32 +0900 Subject: [PATCH 06/38] Add test for `handle_document_symbol` (#5655) --- .../rust-analyzer/tests/heavy_tests/main.rs | 321 +++++++++++++++++- 1 file changed, 317 insertions(+), 4 deletions(-) diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index 7370505f8b..9a4655e4d5 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -5,11 +5,14 @@ use std::{collections::HashMap, path::PathBuf, time::Instant}; use lsp_types::{ notification::DidOpenTextDocument, - request::{CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest}, + request::{ + CodeActionRequest, Completion, DocumentSymbolRequest, Formatting, GotoTypeDefinition, + HoverRequest, + }, CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, - DocumentFormattingParams, FormattingOptions, GotoDefinitionParams, HoverParams, - PartialResultParams, Position, Range, TextDocumentItem, TextDocumentPositionParams, - WorkDoneProgressParams, + DocumentFormattingParams, DocumentSymbolParams, FormattingOptions, GotoDefinitionParams, + HoverParams, PartialResultParams, Position, Range, TextDocumentItem, + TextDocumentPositionParams, WorkDoneProgressParams, }; use rust_analyzer::lsp_ext::{OnEnter, Runnables, RunnablesParams}; use serde_json::json; @@ -682,3 +685,313 @@ pub fn foo(_input: TokenStream) -> TokenStream { let value = res.get("contents").unwrap().get("value").unwrap().to_string(); assert_eq!(value, r#""```rust\nfoo::Bar\n```\n\n```rust\nfn bar()\n```""#) } + +#[test] +fn test_document_symbol_with_hierarchy() { + if skip_slow_tests() { + return; + } + + let server = Project::with_fixture( + r#" +//- /Cargo.toml +[package] +name = "foo" +version = "0.0.0" + +//- /src/lib.rs +mod a { + mod b { + struct B1; + fn b2() {} + } + struct A1; +} +"#, + ) + .with_config(|config| { + config.client_caps.hierarchical_symbols = true; + }) + .server(); + server.wait_until_workspace_is_loaded(); + + server.request::( + DocumentSymbolParams { + text_document: server.doc_id("src/lib.rs"), + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }, + json!([ + { + "children": [ + { + "children": [ + { + "deprecated": false, + "kind": 23, + "name": "B1", + "range": { + "end": { + "character": 18, + "line": 2 + }, + "start": { + "character": 8, + "line": 2 + } + }, + "selectionRange": { + "end": { + "character": 17, + "line": 2 + }, + "start": { + "character": 15, + "line": 2 + } + }, + "tags": [] + }, + { + "deprecated": false, + "detail": "fn()", + "kind": 12, + "name": "b2", + "range": { + "end": { + "character": 18, + "line": 3 + }, + "start": { + "character": 8, + "line": 3 + } + }, + "selectionRange": { + "end": { + "character": 13, + "line": 3 + }, + "start": { + "character": 11, + "line": 3 + } + }, + "tags": [] + } + ], + "deprecated": false, + "kind": 2, + "name": "b", + "range": { + "end": { + "character": 5, + "line": 4 + }, + "start": { + "character": 4, + "line": 1 + } + }, + "selectionRange": { + "end": { + "character": 9, + "line": 1 + }, + "start": { + "character": 8, + "line": 1 + } + }, + "tags": [] + }, + { + "deprecated": false, + "kind": 23, + "name": "A1", + "range": { + "end": { + "character": 14, + "line": 5 + }, + "start": { + "character": 4, + "line": 5 + } + }, + "selectionRange": { + "end": { + "character": 13, + "line": 5 + }, + "start": { + "character": 11, + "line": 5 + } + }, + "tags": [] + } + ], + "deprecated": false, + "kind": 2, + "name": "a", + "range": { + "end": { + "character": 1, + "line": 6 + }, + "start": { + "character": 0, + "line": 0 + } + }, + "selectionRange": { + "end": { + "character": 5, + "line": 0 + }, + "start": { + "character": 4, + "line": 0 + } + }, + "tags": [] + } + ]), + ); +} + +#[test] +fn test_document_symbol_without_hierarchy() { + if skip_slow_tests() { + return; + } + + let server = project( + r#" +//- /Cargo.toml +[package] +name = "foo" +version = "0.0.0" + +//- /src/lib.rs +mod a { + mod b { + struct B1; + fn b2() {} + } + struct A1; +} +"#, + ); + server.wait_until_workspace_is_loaded(); + + server.request::( + DocumentSymbolParams { + text_document: server.doc_id("src/lib.rs"), + work_done_progress_params: WorkDoneProgressParams::default(), + partial_result_params: PartialResultParams::default(), + }, + json!([ + { + "deprecated": false, + "kind": 2, + "location": { + "range": { + "end": { + "character": 1, + "line": 6 + }, + "start": { + "character": 0, + "line": 0 + } + }, + "uri": "file:///[..]/src/lib.rs" + }, + "name": "a", + "tags": [] + }, + { + "containerName": "a", + "deprecated": false, + "kind": 2, + "location": { + "range": { + "end": { + "character": 5, + "line": 4 + }, + "start": { + "character": 4, + "line": 1 + } + }, + "uri": "file:///[..]/src/lib.rs" + }, + "name": "b", + "tags": [] + }, + { + "containerName": "b", + "deprecated": false, + "kind": 23, + "location": { + "range": { + "end": { + "character": 18, + "line": 2 + }, + "start": { + "character": 8, + "line": 2 + } + }, + "uri": "file:///[..]/src/lib.rs" + }, + "name": "B1", + "tags": [] + }, + { + "containerName": "b", + "deprecated": false, + "kind": 12, + "location": { + "range": { + "end": { + "character": 18, + "line": 3 + }, + "start": { + "character": 8, + "line": 3 + } + }, + "uri": "file:///[..]/src/lib.rs" + }, + "name": "b2", + "tags": [] + }, + { + "containerName": "a", + "deprecated": false, + "kind": 23, + "location": { + "range": { + "end": { + "character": 14, + "line": 5 + }, + "start": { + "character": 4, + "line": 5 + } + }, + "uri": "file:///[..]/src/lib.rs" + }, + "name": "A1", + "tags": [] + } + ]), + ); +} From 831e3d58b32ad64329f0c84ac93b7b97c7d6c268 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Wed, 12 Aug 2020 15:33:07 +0300 Subject: [PATCH 07/38] Replace String with &'static str --- crates/ra_hir_def/src/diagnostics.rs | 4 ++-- crates/ra_hir_expand/src/diagnostics.rs | 2 +- crates/ra_hir_ty/src/diagnostics.rs | 32 ++++++++++++------------- crates/ra_ide/src/diagnostics.rs | 26 ++++++++++---------- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 1d28c24e88..481b13a878 100644 --- a/crates/ra_hir_def/src/diagnostics.rs +++ b/crates/ra_hir_def/src/diagnostics.rs @@ -15,8 +15,8 @@ pub struct UnresolvedModule { } impl Diagnostic for UnresolvedModule { - fn name(&self) -> String { - "unresolved-module".to_string() + fn name(&self) -> &'static str { + "unresolved-module" } fn message(&self) -> String { "unresolved module".to_string() diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index ef1d61144e..507132a131 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -21,7 +21,7 @@ use ra_syntax::{SyntaxNode, SyntaxNodePtr}; use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { - fn name(&self) -> String; + fn name(&self) -> &'static str; fn message(&self) -> String; fn source(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 0b3e16ae7a..56acd3bbf4 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -33,8 +33,8 @@ pub struct NoSuchField { } impl Diagnostic for NoSuchField { - fn name(&self) -> String { - "no-such-field".to_string() + fn name(&self) -> &'static str { + "no-such-field" } fn message(&self) -> String { @@ -68,8 +68,8 @@ pub struct MissingFields { } impl Diagnostic for MissingFields { - fn name(&self) -> String { - "missing-structure-fields".to_string() + fn name(&self) -> &'static str { + "missing-structure-fields" } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); @@ -104,8 +104,8 @@ pub struct MissingPatFields { } impl Diagnostic for MissingPatFields { - fn name(&self) -> String { - "missing-pat-fields".to_string() + fn name(&self) -> &'static str { + "missing-pat-fields" } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); @@ -130,8 +130,8 @@ pub struct MissingMatchArms { } impl Diagnostic for MissingMatchArms { - fn name(&self) -> String { - "missing-match-arm".to_string() + fn name(&self) -> &'static str { + "missing-match-arm" } fn message(&self) -> String { String::from("Missing match arm") @@ -151,8 +151,8 @@ pub struct MissingOkInTailExpr { } impl Diagnostic for MissingOkInTailExpr { - fn name(&self) -> String { - "missing-ok-in-tail-expr".to_string() + fn name(&self) -> &'static str { + "missing-ok-in-tail-expr" } fn message(&self) -> String { "wrap return expression in Ok".to_string() @@ -182,8 +182,8 @@ pub struct BreakOutsideOfLoop { } impl Diagnostic for BreakOutsideOfLoop { - fn name(&self) -> String { - "break-outside-of-loop".to_string() + fn name(&self) -> &'static str { + "break-outside-of-loop" } fn message(&self) -> String { "break outside of loop".to_string() @@ -213,8 +213,8 @@ pub struct MissingUnsafe { } impl Diagnostic for MissingUnsafe { - fn name(&self) -> String { - "missing-unsafe".to_string() + fn name(&self) -> &'static str { + "missing-unsafe" } fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") @@ -246,8 +246,8 @@ pub struct MismatchedArgCount { } impl Diagnostic for MismatchedArgCount { - fn name(&self) -> String { - "mismatched-arg-count".to_string() + fn name(&self) -> &'static str { + "mismatched-arg-count" } fn message(&self) -> String { let s = if self.expected == 1 { "" } else { "s" }; diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 8e011a40d8..d97bde939e 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -63,7 +63,7 @@ pub(crate) fn diagnostics( .into(), ); res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -98,7 +98,7 @@ pub(crate) fn diagnostics( }; res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -112,7 +112,7 @@ pub(crate) fn diagnostics( let source_change = SourceFileEdit { file_id, edit }.into(); let fix = Fix::new("Wrap with ok", source_change); res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -121,7 +121,7 @@ pub(crate) fn diagnostics( }) .on::(|d| { res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -133,8 +133,8 @@ pub(crate) fn diagnostics( if !analysis_config.disabled_diagnostics.is_empty() { // Do not collect disabled diagnostics. - sink_builder = sink_builder - .filter(|diag| !analysis_config.disabled_diagnostics.contains(&diag.name())); + sink_builder = + sink_builder.filter(|diag| !analysis_config.disabled_diagnostics.contains(diag.name())); } // Finalize the `DiagnosticSink` building process. @@ -142,7 +142,7 @@ pub(crate) fn diagnostics( // Diagnostics not handled above get no fix and default treatment. .build(|d| { res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), message: d.message(), range: sema.diagnostics_range(d).range, severity: Severity::Error, @@ -313,6 +313,7 @@ fn check_struct_shorthand_initialization( #[cfg(test)] mod tests { + use std::collections::HashSet; use stdx::trim_indent; use test_utils::assert_eq_text; @@ -385,12 +386,9 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor position and the list of disabled diagnostics, /// and checks that provided diagnostics aren't spawned during analysis. - fn check_disabled_diagnostics( - ra_fixture: &str, - disabled_diagnostics: impl IntoIterator, - ) { - let disabled_diagnostics: std::collections::HashSet<_> = - disabled_diagnostics.into_iter().collect(); + fn check_disabled_diagnostics(ra_fixture: &str, disabled_diagnostics: &[&'static str]) { + let disabled_diagnostics: HashSet<_> = + disabled_diagnostics.into_iter().map(|diag| diag.to_string()).collect(); let mock = MockAnalysis::with_files(ra_fixture); let files = mock.files().map(|(it, _)| it).collect::>(); @@ -871,6 +869,6 @@ struct Foo { #[test] fn test_disabled_diagnostics() { - check_disabled_diagnostics(r#"mod foo;"#, vec!["unresolved-module".to_string()]); + check_disabled_diagnostics(r#"mod foo;"#, &vec!["unresolved-module"]); } } From ae0b2477fe5cb8b496b41bd37cb06d1a585bc66b Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Wed, 12 Aug 2020 15:48:56 +0300 Subject: [PATCH 08/38] Update crates/ra_ide/src/diagnostics.rs Co-authored-by: Jonas Schievink --- crates/ra_ide/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index d97bde939e..5ce900bf4d 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -869,6 +869,6 @@ struct Foo { #[test] fn test_disabled_diagnostics() { - check_disabled_diagnostics(r#"mod foo;"#, &vec!["unresolved-module"]); + check_disabled_diagnostics(r#"mod foo;"#, &["unresolved-module"]); } } From 13f736d4a13bdf5af2cdd6a4832a41470431a70b Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Wed, 12 Aug 2020 16:06:55 +0300 Subject: [PATCH 09/38] Add a configuration option for the vscode extension --- editors/code/package.json | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/editors/code/package.json b/editors/code/package.json index 1adf055d0c..86584c071c 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -609,6 +609,15 @@ }, "description": "List of warnings warnings that should be displayed with hint severity.\nThe warnings will be indicated by faded text or three dots in code and will not show up in the problems panel.", "default": [] + }, + "rust-analyzer.analysis.disabledDiagnostics": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "string" + }, + "description": "List of rust-analyzer diagnostics to disable", + "default": [] } } }, @@ -904,4 +913,4 @@ ] } } -} +} \ No newline at end of file From 3c018bf84de5c693b5ee1c6bec0fed3b201c2060 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Thu, 13 Aug 2020 06:58:26 +0300 Subject: [PATCH 10/38] Restore final newline in package.json --- 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 d186d1474e..429ff5def4 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -913,4 +913,4 @@ ] } } -} \ No newline at end of file +} From c84f98385a28eeb7595f38b7cfaf861a6e06f4ea Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Thu, 6 Aug 2020 11:30:52 +1000 Subject: [PATCH 11/38] Refactor SSR so that placeholders store a Var This allows lookup of placeholder bindings given a placeholder without needing to create a Var instance. --- crates/ssr/src/matching.rs | 15 +++++---------- crates/ssr/src/parsing.rs | 24 +++++++++++++++++++----- crates/ssr/src/replacing.rs | 3 +-- crates/ssr/src/tests.rs | 2 +- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/crates/ssr/src/matching.rs b/crates/ssr/src/matching.rs index ffc7202ae5..7f0b5061e2 100644 --- a/crates/ssr/src/matching.rs +++ b/crates/ssr/src/matching.rs @@ -2,7 +2,7 @@ //! process of matching, placeholder values are recorded. use crate::{ - parsing::{Constraint, NodeKind, Placeholder}, + parsing::{Constraint, NodeKind, Placeholder, Var}, resolving::{ResolvedPattern, ResolvedRule, UfcsCallInfo}, SsrMatches, }; @@ -56,10 +56,6 @@ pub struct Match { pub(crate) rendered_template_paths: FxHashMap, } -/// Represents a `$var` in an SSR query. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub(crate) struct Var(pub String); - /// Information about a placeholder bound in a match. #[derive(Debug)] pub(crate) struct PlaceholderMatch { @@ -182,10 +178,9 @@ impl<'db, 'sema> Matcher<'db, 'sema> { // We validated the range for the node when we started the match, so the placeholder // probably can't fail range validation, but just to be safe... self.validate_range(&original_range)?; - matches_out.placeholder_values.insert( - Var(placeholder.ident.to_string()), - PlaceholderMatch::new(code, original_range), - ); + matches_out + .placeholder_values + .insert(placeholder.ident.clone(), PlaceholderMatch::new(code, original_range)); } return Ok(()); } @@ -487,7 +482,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> { } if let Phase::Second(match_out) = phase { match_out.placeholder_values.insert( - Var(placeholder.ident.to_string()), + placeholder.ident.clone(), PlaceholderMatch::from_range(FileRange { file_id: self.sema.original_range(code).file_id, range: first_matched_token diff --git a/crates/ssr/src/parsing.rs b/crates/ssr/src/parsing.rs index 9570e96e36..05b66dcd78 100644 --- a/crates/ssr/src/parsing.rs +++ b/crates/ssr/src/parsing.rs @@ -8,7 +8,7 @@ use crate::errors::bail; use crate::{SsrError, SsrPattern, SsrRule}; use rustc_hash::{FxHashMap, FxHashSet}; -use std::str::FromStr; +use std::{fmt::Display, str::FromStr}; use syntax::{ast, AstNode, SmolStr, SyntaxKind, SyntaxNode, T}; use test_utils::mark; @@ -34,12 +34,16 @@ pub(crate) enum PatternElement { #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct Placeholder { /// The name of this placeholder. e.g. for "$a", this would be "a" - pub(crate) ident: SmolStr, + pub(crate) ident: Var, /// A unique name used in place of this placeholder when we parse the pattern as Rust code. stand_in_name: String, pub(crate) constraints: Vec, } +/// Represents a `$var` in an SSR query. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub(crate) struct Var(pub String); + #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Constraint { Kind(NodeKind), @@ -205,7 +209,7 @@ fn parse_pattern(pattern_str: &str) -> Result, SsrError> { if token.kind == T![$] { let placeholder = parse_placeholder(&mut tokens)?; if !placeholder_names.insert(placeholder.ident.clone()) { - bail!("Name `{}` repeats more than once", placeholder.ident); + bail!("Placeholder `{}` repeats more than once", placeholder.ident); } res.push(PatternElement::Placeholder(placeholder)); } else { @@ -228,7 +232,7 @@ fn validate_rule(rule: &SsrRule) -> Result<(), SsrError> { for p in &rule.template.tokens { if let PatternElement::Placeholder(placeholder) = p { if !defined_placeholders.contains(&placeholder.ident) { - undefined.push(format!("${}", placeholder.ident)); + undefined.push(placeholder.ident.to_string()); } if !placeholder.constraints.is_empty() { bail!("Replacement placeholders cannot have constraints"); @@ -344,7 +348,17 @@ impl NodeKind { impl Placeholder { fn new(name: SmolStr, constraints: Vec) -> Self { - Self { stand_in_name: format!("__placeholder_{}", name), constraints, ident: name } + Self { + stand_in_name: format!("__placeholder_{}", name), + constraints, + ident: Var(name.to_string()), + } + } +} + +impl Display for Var { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "${}", self.0) } } diff --git a/crates/ssr/src/replacing.rs b/crates/ssr/src/replacing.rs index 8f8fe6149a..496a21e6e1 100644 --- a/crates/ssr/src/replacing.rs +++ b/crates/ssr/src/replacing.rs @@ -1,6 +1,5 @@ //! Code for applying replacement templates for matches that have previously been found. -use crate::matching::Var; use crate::{resolving::ResolvedRule, Match, SsrMatches}; use rustc_hash::{FxHashMap, FxHashSet}; use syntax::ast::{self, AstToken}; @@ -114,7 +113,7 @@ impl ReplacementRenderer<'_> { fn render_token(&mut self, token: &SyntaxToken) { if let Some(placeholder) = self.rule.get_placeholder(&token) { if let Some(placeholder_value) = - self.match_info.placeholder_values.get(&Var(placeholder.ident.to_string())) + self.match_info.placeholder_values.get(&placeholder.ident) { let range = &placeholder_value.range.range; let mut matched_text = diff --git a/crates/ssr/src/tests.rs b/crates/ssr/src/tests.rs index 0d0a000906..65cd387530 100644 --- a/crates/ssr/src/tests.rs +++ b/crates/ssr/src/tests.rs @@ -31,7 +31,7 @@ fn parser_two_delimiters() { fn parser_repeated_name() { assert_eq!( parse_error_text("foo($a, $a) ==>>"), - "Parse error: Name `a` repeats more than once" + "Parse error: Placeholder `$a` repeats more than once" ); } From a4a504e1328111c184603ddc0b2c113ad5a5c555 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Thu, 6 Aug 2020 07:26:28 +1000 Subject: [PATCH 12/38] SSR: Explicitly autoderef and ref placeholders as needed Structured search replace now inserts *, & and &mut in the replacement to match any auto[de]ref in the matched code. --- crates/ssr/src/lib.rs | 5 +- crates/ssr/src/matching.rs | 90 ++++++++++++++++++++++-------- crates/ssr/src/replacing.rs | 46 ++++++++++++++++ crates/ssr/src/tests.rs | 106 ++++++++++++++++++++++++++++++++++++ 4 files changed, 222 insertions(+), 25 deletions(-) diff --git a/crates/ssr/src/lib.rs b/crates/ssr/src/lib.rs index 292bd5b9a7..ba669fd56c 100644 --- a/crates/ssr/src/lib.rs +++ b/crates/ssr/src/lib.rs @@ -21,7 +21,10 @@ // code in the `foo` module, we'll insert just `Bar`. // // Inherent method calls should generally be written in UFCS form. e.g. `foo::Bar::baz($s, $a)` will -// match `$s.baz($a)`, provided the method call `baz` resolves to the method `foo::Bar::baz`. +// match `$s.baz($a)`, provided the method call `baz` resolves to the method `foo::Bar::baz`. When a +// placeholder is the receiver of a method call in the search pattern (e.g. `$s.foo()`), but not in +// the replacement template (e.g. `bar($s)`), then *, & and &mut will be added as needed to mirror +// whatever autoderef and autoref was happening implicitly in the matched code. // // The scope of the search / replace will be restricted to the current selection if any, otherwise // it will apply to the whole workspace. diff --git a/crates/ssr/src/matching.rs b/crates/ssr/src/matching.rs index 7f0b5061e2..8bb5ced900 100644 --- a/crates/ssr/src/matching.rs +++ b/crates/ssr/src/matching.rs @@ -65,6 +65,10 @@ pub(crate) struct PlaceholderMatch { pub(crate) range: FileRange, /// More matches, found within `node`. pub(crate) inner_matches: SsrMatches, + /// How many times the code that the placeholder matched needed to be dereferenced. Will only be + /// non-zero if the placeholder matched to the receiver of a method call. + pub(crate) autoderef_count: usize, + pub(crate) autoref_kind: ast::SelfParamKind, } #[derive(Debug)] @@ -169,7 +173,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> { code: &SyntaxNode, ) -> Result<(), MatchFailed> { // Handle placeholders. - if let Some(placeholder) = self.get_placeholder(&SyntaxElement::Node(pattern.clone())) { + if let Some(placeholder) = self.get_placeholder_for_node(pattern) { for constraint in &placeholder.constraints { self.check_constraint(constraint, code)?; } @@ -178,9 +182,10 @@ impl<'db, 'sema> Matcher<'db, 'sema> { // We validated the range for the node when we started the match, so the placeholder // probably can't fail range validation, but just to be safe... self.validate_range(&original_range)?; - matches_out - .placeholder_values - .insert(placeholder.ident.clone(), PlaceholderMatch::new(code, original_range)); + matches_out.placeholder_values.insert( + placeholder.ident.clone(), + PlaceholderMatch::new(Some(code), original_range), + ); } return Ok(()); } @@ -531,18 +536,40 @@ impl<'db, 'sema> Matcher<'db, 'sema> { if pattern_ufcs.function != code_resolved_function { fail_match!("Method call resolved to a different function"); } - if code_resolved_function.has_self_param(self.sema.db) { - if let (Some(pattern_type), Some(expr)) = (&pattern_ufcs.qualifier_type, &code.expr()) { - self.check_expr_type(pattern_type, expr)?; - } - } // Check arguments. let mut pattern_args = pattern_ufcs .call_expr .arg_list() .ok_or_else(|| match_error!("Pattern function call has no args"))? .args(); - self.attempt_match_opt(phase, pattern_args.next(), code.expr())?; + // If the function we're calling takes a self parameter, then we store additional + // information on the placeholder match about autoderef and autoref. This allows us to use + // the placeholder in a context where autoderef and autoref don't apply. + if code_resolved_function.has_self_param(self.sema.db) { + if let (Some(pattern_type), Some(expr)) = (&pattern_ufcs.qualifier_type, &code.expr()) { + let deref_count = self.check_expr_type(pattern_type, expr)?; + let pattern_receiver = pattern_args.next(); + self.attempt_match_opt(phase, pattern_receiver.clone(), code.expr())?; + if let Phase::Second(match_out) = phase { + if let Some(placeholder_value) = pattern_receiver + .and_then(|n| self.get_placeholder_for_node(n.syntax())) + .and_then(|placeholder| { + match_out.placeholder_values.get_mut(&placeholder.ident) + }) + { + placeholder_value.autoderef_count = deref_count; + placeholder_value.autoref_kind = self + .sema + .resolve_method_call_as_callable(code) + .and_then(|callable| callable.receiver_param(self.sema.db)) + .map(|self_param| self_param.kind()) + .unwrap_or(ast::SelfParamKind::Owned); + } + } + } + } else { + self.attempt_match_opt(phase, pattern_args.next(), code.expr())?; + } let mut code_args = code.arg_list().ok_or_else(|| match_error!("Code method call has no args"))?.args(); loop { @@ -570,26 +597,35 @@ impl<'db, 'sema> Matcher<'db, 'sema> { self.attempt_match_node_children(phase, pattern_ufcs.call_expr.syntax(), code.syntax()) } + /// Verifies that `expr` matches `pattern_type`, possibly after dereferencing some number of + /// times. Returns the number of times it needed to be dereferenced. fn check_expr_type( &self, pattern_type: &hir::Type, expr: &ast::Expr, - ) -> Result<(), MatchFailed> { + ) -> Result { use hir::HirDisplay; let code_type = self.sema.type_of_expr(&expr).ok_or_else(|| { match_error!("Failed to get receiver type for `{}`", expr.syntax().text()) })?; - if !code_type + // Temporary needed to make the borrow checker happy. + let res = code_type .autoderef(self.sema.db) - .any(|deref_code_type| *pattern_type == deref_code_type) - { - fail_match!( - "Pattern type `{}` didn't match code type `{}`", - pattern_type.display(self.sema.db), - code_type.display(self.sema.db) - ); - } - Ok(()) + .enumerate() + .find(|(_, deref_code_type)| pattern_type == deref_code_type) + .map(|(count, _)| count) + .ok_or_else(|| { + match_error!( + "Pattern type `{}` didn't match code type `{}`", + pattern_type.display(self.sema.db), + code_type.display(self.sema.db) + ) + }); + res + } + + fn get_placeholder_for_node(&self, node: &SyntaxNode) -> Option<&Placeholder> { + self.get_placeholder(&SyntaxElement::Node(node.clone())) } fn get_placeholder(&self, element: &SyntaxElement) -> Option<&Placeholder> { @@ -671,12 +707,18 @@ fn recording_match_fail_reasons() -> bool { } impl PlaceholderMatch { - fn new(node: &SyntaxNode, range: FileRange) -> Self { - Self { node: Some(node.clone()), range, inner_matches: SsrMatches::default() } + fn new(node: Option<&SyntaxNode>, range: FileRange) -> Self { + Self { + node: node.cloned(), + range, + inner_matches: SsrMatches::default(), + autoderef_count: 0, + autoref_kind: ast::SelfParamKind::Owned, + } } fn from_range(range: FileRange) -> Self { - Self { node: None, range, inner_matches: SsrMatches::default() } + Self::new(None, range) } } diff --git a/crates/ssr/src/replacing.rs b/crates/ssr/src/replacing.rs index 496a21e6e1..21d0aa8a8c 100644 --- a/crates/ssr/src/replacing.rs +++ b/crates/ssr/src/replacing.rs @@ -118,6 +118,27 @@ impl ReplacementRenderer<'_> { let range = &placeholder_value.range.range; let mut matched_text = self.file_src[usize::from(range.start())..usize::from(range.end())].to_owned(); + // If a method call is performed directly on the placeholder, then autoderef and + // autoref will apply, so we can just substitute whatever the placeholder matched to + // directly. If we're not applying a method call, then we need to add explicitly + // deref and ref in order to match whatever was being done implicitly at the match + // site. + if !token_is_method_call_receiver(token) + && (placeholder_value.autoderef_count > 0 + || placeholder_value.autoref_kind != ast::SelfParamKind::Owned) + { + let ref_kind = match placeholder_value.autoref_kind { + ast::SelfParamKind::Owned => "", + ast::SelfParamKind::Ref => "&", + ast::SelfParamKind::MutRef => "&mut ", + }; + matched_text = format!( + "{}{}{}", + ref_kind, + "*".repeat(placeholder_value.autoderef_count), + matched_text + ); + } let edit = matches_to_edit_at_offset( &placeholder_value.inner_matches, self.file_src, @@ -178,6 +199,31 @@ impl ReplacementRenderer<'_> { } } +/// Returns whether token is the receiver of a method call. Note, being within the receiver of a +/// method call doesn't count. e.g. if the token is `$a`, then `$a.foo()` will return true, while +/// `($a + $b).foo()` or `x.foo($a)` will return false. +fn token_is_method_call_receiver(token: &SyntaxToken) -> bool { + use syntax::ast::AstNode; + // Find the first method call among the ancestors of `token`, then check if the only token + // within the receiver is `token`. + if let Some(receiver) = token + .ancestors() + .find(|node| node.kind() == SyntaxKind::METHOD_CALL_EXPR) + .and_then(|node| ast::MethodCallExpr::cast(node).unwrap().expr()) + { + let mut tokens = receiver.syntax().descendants_with_tokens().filter_map(|node_or_token| { + match node_or_token { + SyntaxElement::Token(t) => Some(t), + _ => None, + } + }); + if let (Some(only_token), None) = (tokens.next(), tokens.next()) { + return only_token == *token; + } + } + false +} + fn parse_as_kind(code: &str, kind: SyntaxKind) -> Option { use syntax::ast::AstNode; if ast::Expr::can_cast(kind) { diff --git a/crates/ssr/src/tests.rs b/crates/ssr/src/tests.rs index 65cd387530..0ad3512bad 100644 --- a/crates/ssr/src/tests.rs +++ b/crates/ssr/src/tests.rs @@ -1172,3 +1172,109 @@ fn match_trait_method_call() { assert_matches("Bar::foo($a, $b)", code, &["v1.foo(1)", "Bar::foo(&v1, 3)", "v1_ref.foo(5)"]); assert_matches("Bar2::foo($a, $b)", code, &["v2.foo(2)", "Bar2::foo(&v2, 4)", "v2_ref.foo(6)"]); } + +#[test] +fn replace_autoref_autoderef_capture() { + // Here we have several calls to `$a.foo()`. In the first case autoref is applied, in the + // second, we already have a reference, so it isn't. When $a is used in a context where autoref + // doesn't apply, we need to prefix it with `&`. Finally, we have some cases where autoderef + // needs to be applied. + let code = r#" + struct Foo {} + impl Foo { + fn foo(&self) {} + fn foo2(&self) {} + } + fn bar(_: &Foo) {} + fn main() { + let f = Foo {}; + let fr = &f; + let fr2 = &fr; + let fr3 = &fr2; + f.foo(); + fr.foo(); + fr2.foo(); + fr3.foo(); + } + "#; + assert_ssr_transform( + "Foo::foo($a) ==>> bar($a)", + code, + expect![[r#" + struct Foo {} + impl Foo { + fn foo(&self) {} + fn foo2(&self) {} + } + fn bar(_: &Foo) {} + fn main() { + let f = Foo {}; + let fr = &f; + let fr2 = &fr; + let fr3 = &fr2; + bar(&f); + bar(&*fr); + bar(&**fr2); + bar(&***fr3); + } + "#]], + ); + // If the placeholder is used as the receiver of another method call, then we don't need to + // explicitly autoderef or autoref. + assert_ssr_transform( + "Foo::foo($a) ==>> $a.foo2()", + code, + expect![[r#" + struct Foo {} + impl Foo { + fn foo(&self) {} + fn foo2(&self) {} + } + fn bar(_: &Foo) {} + fn main() { + let f = Foo {}; + let fr = &f; + let fr2 = &fr; + let fr3 = &fr2; + f.foo2(); + fr.foo2(); + fr2.foo2(); + fr3.foo2(); + } + "#]], + ); +} + +#[test] +fn replace_autoref_mut() { + let code = r#" + struct Foo {} + impl Foo { + fn foo(&mut self) {} + } + fn bar(_: &mut Foo) {} + fn main() { + let mut f = Foo {}; + f.foo(); + let fr = &mut f; + fr.foo(); + } + "#; + assert_ssr_transform( + "Foo::foo($a) ==>> bar($a)", + code, + expect![[r#" + struct Foo {} + impl Foo { + fn foo(&mut self) {} + } + fn bar(_: &mut Foo) {} + fn main() { + let mut f = Foo {}; + bar(&mut f); + let fr = &mut f; + bar(&mut *fr); + } + "#]], + ); +} From 9f548a02957314819aa0530d01f7c4a27dffdfc8 Mon Sep 17 00:00:00 2001 From: jDomantas Date: Fri, 14 Aug 2020 16:10:52 +0300 Subject: [PATCH 13/38] fixup whitespace when adding missing impl items --- .../src/handlers/add_missing_impl_members.rs | 63 +++++++++++++++++-- crates/syntax/src/ast/edit.rs | 33 +++++++++- 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/crates/assists/src/handlers/add_missing_impl_members.rs b/crates/assists/src/handlers/add_missing_impl_members.rs index 81b61ebf8e..83a2ada9a2 100644 --- a/crates/assists/src/handlers/add_missing_impl_members.rs +++ b/crates/assists/src/handlers/add_missing_impl_members.rs @@ -48,7 +48,6 @@ enum AddMissingImplMembersMode { // fn foo(&self) -> u32 { // ${0:todo!()} // } -// // } // ``` pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -89,8 +88,8 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) - // impl Trait for () { // Type X = (); // fn foo(&self) {} -// $0fn bar(&self) {} // +// $0fn bar(&self) {} // } // ``` pub(crate) fn add_missing_default_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -240,15 +239,18 @@ struct S; impl Foo for S { fn bar(&self) {} + $0type Output; + const CONST: usize = 42; + fn foo(&self) { todo!() } + fn baz(&self) { todo!() } - }"#, ); } @@ -281,10 +283,10 @@ struct S; impl Foo for S { fn bar(&self) {} + fn foo(&self) { ${0:todo!()} } - }"#, ); } @@ -599,6 +601,7 @@ trait Foo { struct S; impl Foo for S { $0type Output; + fn foo(&self) { todo!() } @@ -705,6 +708,58 @@ trait Tr { impl Tr for () { $0type Ty; +}"#, + ) + } + + #[test] + fn test_whitespace_fixup_preserves_bad_tokens() { + check_assist( + add_missing_impl_members, + r#" +trait Tr { + fn foo(); +} + +impl Tr for ()<|> { + +++ +}"#, + r#" +trait Tr { + fn foo(); +} + +impl Tr for () { + fn foo() { + ${0:todo!()} + } + +++ +}"#, + ) + } + + #[test] + fn test_whitespace_fixup_preserves_comments() { + check_assist( + add_missing_impl_members, + r#" +trait Tr { + fn foo(); +} + +impl Tr for ()<|> { + // very important +}"#, + r#" +trait Tr { + fn foo(); +} + +impl Tr for () { + fn foo() { + ${0:todo!()} + } + // very important }"#, ) } diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index 190746e09e..b295b5bc67 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs @@ -91,29 +91,56 @@ impl ast::AssocItemList { res = make_multiline(res); } items.into_iter().for_each(|it| res = res.append_item(it)); - res + res.fixup_trailing_whitespace().unwrap_or(res) } #[must_use] pub fn append_item(&self, item: ast::AssocItem) -> ast::AssocItemList { - let (indent, position) = match self.assoc_items().last() { + let (indent, position, whitespace) = match self.assoc_items().last() { Some(it) => ( leading_indent(it.syntax()).unwrap_or_default().to_string(), InsertPosition::After(it.syntax().clone().into()), + "\n\n", ), None => match self.l_curly_token() { Some(it) => ( " ".to_string() + &leading_indent(self.syntax()).unwrap_or_default(), InsertPosition::After(it.into()), + "\n", ), None => return self.clone(), }, }; - let ws = tokens::WsBuilder::new(&format!("\n{}", indent)); + let ws = tokens::WsBuilder::new(&format!("{}{}", whitespace, indent)); let to_insert: ArrayVec<[SyntaxElement; 2]> = [ws.ws().into(), item.syntax().clone().into()].into(); self.insert_children(position, to_insert) } + + /// Remove extra whitespace between last item and closing curly brace. + fn fixup_trailing_whitespace(&self) -> Option { + let first_token_after_items = self + .assoc_items() + .last()? + .syntax() + .next_sibling_or_token()?; + let last_token_before_curly = self + .r_curly_token()? + .prev_sibling_or_token()?; + if last_token_before_curly != first_token_after_items { + // there is something more between last item and + // right curly than just whitespace - bail out + return None; + } + let whitespace = last_token_before_curly + .clone() + .into_token() + .and_then(ast::Whitespace::cast)?; + let text = whitespace.syntax().text(); + let newline = text.rfind("\n")?; + let keep = tokens::WsBuilder::new(&text[newline..]); + Some(self.replace_children(first_token_after_items..=last_token_before_curly, std::iter::once(keep.ws().into()))) + } } impl ast::RecordExprFieldList { From cb816b1ea87b24f34eeecfdd98aeeb629915d661 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 15 Aug 2020 00:19:47 +0200 Subject: [PATCH 14/38] Add a proc_macro_test crate This exports all 3 kinds of proc macros and is useful for testing --- Cargo.lock | 5 +++++ crates/proc_macro_srv/Cargo.toml | 4 +++- crates/proc_macro_srv/src/tests/mod.rs | 15 ++++++++++++++- crates/proc_macro_srv/src/tests/utils.rs | 2 +- crates/proc_macro_test/Cargo.toml | 10 ++++++++++ crates/proc_macro_test/src/lib.rs | 18 ++++++++++++++++++ 6 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 crates/proc_macro_test/Cargo.toml create mode 100644 crates/proc_macro_test/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 2386c8f3a5..c7809a65a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1097,12 +1097,17 @@ dependencies = [ "mbe", "memmap", "proc_macro_api", + "proc_macro_test", "serde_derive", "test_utils", "toolchain", "tt", ] +[[package]] +name = "proc_macro_test" +version = "0.0.0" + [[package]] name = "profile" version = "0.0.0" diff --git a/crates/proc_macro_srv/Cargo.toml b/crates/proc_macro_srv/Cargo.toml index 7171f08084..a468b55609 100644 --- a/crates/proc_macro_srv/Cargo.toml +++ b/crates/proc_macro_srv/Cargo.toml @@ -21,7 +21,9 @@ test_utils = { path = "../test_utils" } [dev-dependencies] cargo_metadata = "0.11.1" difference = "2.0.0" -# used as proc macro test target + +# used as proc macro test targets serde_derive = "1.0.106" +proc_macro_test = { path = "../proc_macro_test" } toolchain = { path = "../toolchain" } diff --git a/crates/proc_macro_srv/src/tests/mod.rs b/crates/proc_macro_srv/src/tests/mod.rs index 8e6f28abdf..1a827cbd76 100644 --- a/crates/proc_macro_srv/src/tests/mod.rs +++ b/crates/proc_macro_srv/src/tests/mod.rs @@ -35,7 +35,7 @@ SUBTREE $ #[test] fn test_derive_proc_macro_list() { - let res = list("serde_derive", "1.0").join("\n"); + let res = list("serde_derive", "1").join("\n"); assert_eq_text!( &res, @@ -43,3 +43,16 @@ fn test_derive_proc_macro_list() { Deserialize [CustomDerive]"# ); } + +/// Tests that we find and classify non-derive macros correctly. +#[test] +fn list_test_macros() { + let res = list("proc_macro_test", "0.0.0").join("\n"); + + assert_eq_text!( + &res, + r#"function_like_macro [FuncLike] +attribute_macro [Attr] +DummyTrait [CustomDerive]"# + ); +} diff --git a/crates/proc_macro_srv/src/tests/utils.rs b/crates/proc_macro_srv/src/tests/utils.rs index 5828512d6e..36942147d9 100644 --- a/crates/proc_macro_srv/src/tests/utils.rs +++ b/crates/proc_macro_srv/src/tests/utils.rs @@ -13,7 +13,7 @@ mod fixtures { // Use current project metadata to get the proc-macro dylib path pub fn dylib_path(crate_name: &str, version: &str) -> std::path::PathBuf { let command = Command::new(toolchain::cargo()) - .args(&["check", "--message-format", "json"]) + .args(&["check", "--tests", "--message-format", "json"]) .output() .unwrap() .stdout; diff --git a/crates/proc_macro_test/Cargo.toml b/crates/proc_macro_test/Cargo.toml new file mode 100644 index 0000000000..7b0f64f318 --- /dev/null +++ b/crates/proc_macro_test/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "proc_macro_test" +version = "0.0.0" +license = "MIT OR Apache-2.0" +authors = ["rust-analyzer developers"] +edition = "2018" + +[lib] +doctest = false +proc-macro = true diff --git a/crates/proc_macro_test/src/lib.rs b/crates/proc_macro_test/src/lib.rs new file mode 100644 index 0000000000..ec2a114a35 --- /dev/null +++ b/crates/proc_macro_test/src/lib.rs @@ -0,0 +1,18 @@ +//! Exports a few trivial procedural macros for testing. + +use proc_macro::TokenStream; + +#[proc_macro] +pub fn function_like_macro(args: TokenStream) -> TokenStream { + args +} + +#[proc_macro_attribute] +pub fn attribute_macro(_args: TokenStream, item: TokenStream) -> TokenStream { + item +} + +#[proc_macro_derive(DummyTrait)] +pub fn derive_macro(_item: TokenStream) -> TokenStream { + TokenStream::new() +} From bee56e68a3e6b8d70bd8320f6372b95959e377df Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 15 Aug 2020 15:34:56 +0200 Subject: [PATCH 15/38] Hacky support for fn-like proc macros --- crates/proc_macro_api/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/proc_macro_api/src/lib.rs b/crates/proc_macro_api/src/lib.rs index 15db57eb28..d5e87cf7d5 100644 --- a/crates/proc_macro_api/src/lib.rs +++ b/crates/proc_macro_api/src/lib.rs @@ -89,9 +89,8 @@ impl ProcMacroClient { macros .into_iter() .filter_map(|(name, kind)| { - // FIXME: Support custom derive only for now. match kind { - ProcMacroKind::CustomDerive => { + ProcMacroKind::CustomDerive | ProcMacroKind::FuncLike => { let name = SmolStr::new(&name); let expander: Arc = Arc::new(ProcMacroProcessExpander { @@ -101,7 +100,8 @@ impl ProcMacroClient { }); Some((name, expander)) } - _ => None, + // FIXME: Attribute macro are currently unsupported. + ProcMacroKind::Attr => None, } }) .collect() From 2052d33b9b0e2254f53848501a9113aa12ddf4da Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 15 Aug 2020 18:22:16 +0200 Subject: [PATCH 16/38] Remove deprecated Path::from_ast Long term, we probably should make hir::Path private to hir. --- crates/assists/src/ast_transform.rs | 32 ++++++++++++----------------- crates/hir_def/src/path.rs | 6 ------ 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/crates/assists/src/ast_transform.rs b/crates/assists/src/ast_transform.rs index 4c41c16d86..5216862ba5 100644 --- a/crates/assists/src/ast_transform.rs +++ b/crates/assists/src/ast_transform.rs @@ -7,6 +7,17 @@ use syntax::{ ast::{self, AstNode}, }; +pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: N) -> N { + SyntaxRewriter::from_fn(|element| match element { + syntax::SyntaxElement::Node(n) => { + let replacement = transformer.get_substitution(&n)?; + Some(replacement.into()) + } + _ => None, + }) + .rewrite_ast(&node) +} + pub trait AstTransform<'a> { fn get_substitution(&self, node: &syntax::SyntaxNode) -> Option; @@ -107,10 +118,7 @@ impl<'a> SubstituteTypeParams<'a> { ast::Type::PathType(path_type) => path_type.path()?, _ => return None, }; - // FIXME: use `hir::Path::from_src` instead. - #[allow(deprecated)] - let path = hir::Path::from_ast(path)?; - let resolution = self.source_scope.resolve_hir_path(&path)?; + let resolution = self.source_scope.speculative_resolve(&path)?; match resolution { hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()), _ => None, @@ -146,10 +154,7 @@ impl<'a> QualifyPaths<'a> { // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } - // FIXME: use `hir::Path::from_src` instead. - #[allow(deprecated)] - let hir_path = hir::Path::from_ast(p.clone()); - let resolution = self.source_scope.resolve_hir_path(&hir_path?)?; + let resolution = self.source_scope.speculative_resolve(&p)?; match resolution { PathResolution::Def(def) => { let found_path = from.find_use_path(self.source_scope.db.upcast(), def)?; @@ -175,17 +180,6 @@ impl<'a> QualifyPaths<'a> { } } -pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: N) -> N { - SyntaxRewriter::from_fn(|element| match element { - syntax::SyntaxElement::Node(n) => { - let replacement = transformer.get_substitution(&n)?; - Some(replacement.into()) - } - _ => None, - }) - .rewrite_ast(&node) -} - impl<'a> AstTransform<'a> for QualifyPaths<'a> { fn get_substitution(&self, node: &syntax::SyntaxNode) -> Option { self.get_substitution_inner(node).or_else(|| self.previous.get_substitution(node)) diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs index 74d26f08b3..99395667de 100644 --- a/crates/hir_def/src/path.rs +++ b/crates/hir_def/src/path.rs @@ -153,12 +153,6 @@ pub enum GenericArg { } impl Path { - /// Converts an `ast::Path` to `Path`. Works with use trees. - #[deprecated = "Doesn't handle hygiene, don't add new calls, remove old ones"] - pub fn from_ast(path: ast::Path) -> Option { - lower::lower_path(path, &Hygiene::new_unhygienic()) - } - /// Converts an `ast::Path` to `Path`. Works with use trees. /// It correctly handles `$crate` based path from macro call. pub fn from_src(path: ast::Path, hygiene: &Hygiene) -> Option { From 0ca1ba29e8e88c060dcf36946e4e02a6f015754b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 15 Aug 2020 18:50:41 +0200 Subject: [PATCH 17/38] Don't expose hir::Path out of hir Conjecture: it's impossible to use hir::Path *correctly* from an IDE. I am not entirely sure about this, and we might need to add it back at some point, but I have to arguments that convince me that we probably won't: * `hir::Path` has to know about hygiene, which an IDE can't set up properly. * `hir::Path` lacks identity, but you actually have to know identity to resolve it correctly --- crates/assists/src/handlers/auto_import.rs | 2 +- .../src/handlers/expand_glob_import.rs | 20 +++----- .../extract_struct_from_enum_variant.rs | 7 ++- .../replace_qualified_name_with_use.rs | 50 ++++++++----------- crates/assists/src/utils/insert_use.rs | 5 +- crates/hir/src/code_model.rs | 9 ++-- crates/hir/src/lib.rs | 7 ++- crates/hir/src/semantics.rs | 37 ++------------ crates/hir/src/source_analyzer.rs | 6 +-- 9 files changed, 51 insertions(+), 92 deletions(-) diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs index cce789972e..b9ec3f10b6 100644 --- a/crates/assists/src/handlers/auto_import.rs +++ b/crates/assists/src/handlers/auto_import.rs @@ -53,7 +53,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> |builder| { insert_use_statement( &auto_import_assets.syntax_under_caret, - &import, + &import.to_string(), ctx, builder.text_edit_builder(), ); diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index f690ec343b..81d0af2f35 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -1,3 +1,4 @@ +use either::Either; use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; use ide_db::{ defs::{classify_name_ref, Definition, NameRefClass}, @@ -10,8 +11,6 @@ use crate::{ AssistId, AssistKind, }; -use either::Either; - // Assist: expand_glob_import // // Expands glob imports. @@ -40,11 +39,15 @@ use either::Either; pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let star = ctx.find_token_at_offset(T![*])?; let mod_path = find_mod_path(&star)?; + let module = match ctx.sema.resolve_path(&mod_path)? { + PathResolution::Def(ModuleDef::Module(it)) => it, + _ => return None, + }; let source_file = ctx.source_file(); let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset()); - let defs_in_mod = find_defs_in_mod(ctx, scope, &mod_path)?; + let defs_in_mod = find_defs_in_mod(ctx, scope, module)?; let name_refs_in_source_file = source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect(); let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file); @@ -82,17 +85,8 @@ impl Def { fn find_defs_in_mod( ctx: &AssistContext, from: SemanticsScope<'_>, - path: &ast::Path, + module: hir::Module, ) -> Option> { - let hir_path = ctx.sema.lower_path(&path)?; - let module = if let Some(PathResolution::Def(ModuleDef::Module(module))) = - from.resolve_hir_path_qualifier(&hir_path) - { - module - } else { - return None; - }; - let module_scope = module.scope(ctx.db(), from.module()); let mut defs = vec![]; diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index 4bcdae7ba0..d62e06b4a8 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -106,7 +106,12 @@ fn insert_import( if let Some(mut mod_path) = mod_path { mod_path.segments.pop(); mod_path.segments.push(variant_hir_name.clone()); - insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder()); + insert_use_statement( + path.syntax(), + &mod_path.to_string(), + ctx, + builder.text_edit_builder(), + ); } Some(()) } diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs index 011bf1106d..470e5f8ff7 100644 --- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs @@ -1,5 +1,5 @@ -use hir; -use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode}; +use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRange}; +use test_utils::mark; use crate::{ utils::{find_insert_use_container, insert_use_statement}, @@ -28,12 +28,19 @@ pub(crate) fn replace_qualified_name_with_use( if path.syntax().ancestors().find_map(ast::Use::cast).is_some() { return None; } - - let hir_path = ctx.sema.lower_path(&path)?; - let segments = collect_hir_path_segments(&hir_path)?; - if segments.len() < 2 { + if path.qualifier().is_none() { + mark::hit!(dont_import_trivial_paths); return None; } + let path_to_import = path.to_string().clone(); + let path_to_import = match path.segment()?.generic_arg_list() { + Some(generic_args) => { + let generic_args_start = + generic_args.syntax().text_range().start() - path.syntax().text_range().start(); + &path_to_import[TextRange::up_to(generic_args_start)] + } + None => path_to_import.as_str(), + }; let target = path.syntax().text_range(); acc.add( @@ -41,12 +48,16 @@ pub(crate) fn replace_qualified_name_with_use( "Replace qualified path with use", target, |builder| { - let path_to_import = hir_path.mod_path().clone(); let container = match find_insert_use_container(path.syntax(), ctx) { Some(c) => c, None => return, }; - insert_use_statement(path.syntax(), &path_to_import, ctx, builder.text_edit_builder()); + insert_use_statement( + path.syntax(), + &path_to_import.to_string(), + ctx, + builder.text_edit_builder(), + ); // Now that we've brought the name into scope, re-qualify all paths that could be // affected (that is, all paths inside the node we added the `use` to). @@ -58,26 +69,6 @@ pub(crate) fn replace_qualified_name_with_use( ) } -fn collect_hir_path_segments(path: &hir::Path) -> Option> { - let mut ps = Vec::::with_capacity(10); - match path.kind() { - hir::PathKind::Abs => ps.push("".into()), - hir::PathKind::Crate => ps.push("crate".into()), - hir::PathKind::Plain => {} - hir::PathKind::Super(0) => ps.push("self".into()), - hir::PathKind::Super(lvl) => { - let mut chain = "super".to_string(); - for _ in 0..*lvl { - chain += "::super"; - } - ps.push(chain.into()); - } - hir::PathKind::DollarCrate(_) => return None, - } - ps.extend(path.segments().iter().map(|it| it.name.to_string().into())); - Some(ps) -} - /// Adds replacements to `re` that shorten `path` in all descendants of `node`. fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) { for child in node.children() { @@ -467,7 +458,8 @@ impl Debug for Foo { } #[test] - fn test_replace_not_applicable_one_segment() { + fn dont_import_trivial_paths() { + mark::check!(dont_import_trivial_paths); check_assist_not_applicable( replace_qualified_name_with_use, r" diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs index 50a62ee829..49096a67c7 100644 --- a/crates/assists/src/utils/insert_use.rs +++ b/crates/assists/src/utils/insert_use.rs @@ -5,7 +5,6 @@ use std::iter::successors; use either::Either; -use hir::{self, ModPath}; use syntax::{ ast::{self, NameOwner, VisibilityOwner}, AstNode, AstToken, Direction, SmolStr, @@ -35,11 +34,11 @@ pub(crate) fn find_insert_use_container( pub(crate) fn insert_use_statement( // Ideally the position of the cursor, used to position: &SyntaxNode, - path_to_import: &ModPath, + path_to_import: &str, ctx: &AssistContext, builder: &mut TextEditBuilder, ) { - let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::>(); + let target = path_to_import.split("::").map(SmolStr::new).collect::>(); let container = find_insert_use_container(position, ctx); if let Some(container) = container { diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 5dc3ae3b19..c442654dd0 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -12,6 +12,7 @@ use hir_def::{ docs::Documentation, expr::{BindingAnnotation, Pat, PatId}, import_map, + path::ModPath, per_ns::PerNs, resolver::{HasResolver, Resolver}, src::HasSource as _, @@ -344,11 +345,7 @@ impl Module { /// Finds a path that can be used to refer to the given item from within /// this module, if possible. - pub fn find_use_path( - self, - db: &dyn DefDatabase, - item: impl Into, - ) -> Option { + pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into) -> Option { hir_def::find_path::find_path(db, item.into(), self.into()) } } @@ -1126,7 +1123,7 @@ impl ImplDef { .value .attrs() .filter_map(|it| { - let path = hir_def::path::ModPath::from_src(it.path()?, &hygenic)?; + let path = ModPath::from_src(it.path()?, &hygenic)?; if path.as_ident()?.to_string() == "derive" { Some(it) } else { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 4ae2bd0855..8961ba8fd6 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -48,7 +48,7 @@ pub use hir_def::{ builtin_type::BuiltinType, docs::Documentation, nameres::ModuleSource, - path::{ModPath, Path, PathKind}, + path::ModPath, type_ref::{Mutability, TypeRef}, }; pub use hir_expand::{ @@ -60,4 +60,7 @@ pub use hir_ty::display::HirDisplay; // These are negative re-exports: pub using these names is forbidden, they // should remain private to hir internals. #[allow(unused)] -use hir_expand::hygiene::Hygiene; +use { + hir_def::path::{Path, PathKind}, + hir_expand::hygiene::Hygiene, +}; diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 3953017c3b..c693176fa4 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -6,7 +6,7 @@ use std::{cell::RefCell, fmt, iter::successors}; use base_db::{FileId, FileRange}; use hir_def::{ - resolver::{self, HasResolver, Resolver}, + resolver::{self, HasResolver, Resolver, TypeNs}, AsMacroCall, FunctionId, TraitId, VariantId, }; use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo}; @@ -22,12 +22,11 @@ use crate::{ db::HirDatabase, diagnostics::Diagnostic, semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx}, - source_analyzer::{resolve_hir_path, resolve_hir_path_qualifier, SourceAnalyzer}, + source_analyzer::{resolve_hir_path, SourceAnalyzer}, AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef, Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, TypeRef, VariantDef, }; -use resolver::TypeNs; #[derive(Debug, Clone, PartialEq, Eq)] pub enum PathResolution { @@ -228,10 +227,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.resolve_variant(record_lit).map(VariantDef::from) } - pub fn lower_path(&self, path: &ast::Path) -> Option { - self.imp.lower_path(path) - } - pub fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option { self.imp.resolve_bind_pat_to_const(pat) } @@ -467,11 +462,6 @@ impl<'db> SemanticsImpl<'db> { self.analyze(record_lit.syntax()).resolve_variant(self.db, record_lit) } - fn lower_path(&self, path: &ast::Path) -> Option { - let src = self.find_file(path.syntax().clone()); - Path::from_src(path.clone(), &Hygiene::new(self.db.upcast(), src.file_id.into())) - } - fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option { self.analyze(pat.syntax()).resolve_bind_pat_to_const(self.db, pat) } @@ -758,28 +748,7 @@ impl<'a> SemanticsScope<'a> { pub fn speculative_resolve(&self, path: &ast::Path) -> Option { let hygiene = Hygiene::new(self.db.upcast(), self.file_id); let path = Path::from_src(path.clone(), &hygiene)?; - self.resolve_hir_path(&path) - } - - pub fn resolve_hir_path(&self, path: &Path) -> Option { - resolve_hir_path(self.db, &self.resolver, path) - } - - /// Resolves a path where we know it is a qualifier of another path. - /// - /// For example, if we have: - /// ``` - /// mod my { - /// pub mod foo { - /// struct Bar; - /// } - /// - /// pub fn foo() {} - /// } - /// ``` - /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function. - pub fn resolve_hir_path_qualifier(&self, path: &Path) -> Option { - resolve_hir_path_qualifier(self.db, &self.resolver, path) + resolve_hir_path(self.db, &self.resolver, &path) } } diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 8750584f94..1d13c4f1d3 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -13,6 +13,7 @@ use hir_def::{ Body, BodySourceMap, }, expr::{ExprId, Pat, PatId}, + path::{ModPath, Path, PathKind}, resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs}, AsMacroCall, DefWithBodyId, FieldId, FunctionId, LocalFieldId, VariantId, }; @@ -28,8 +29,7 @@ use syntax::{ use crate::{ db::HirDatabase, semantics::PathResolution, Adt, Const, EnumVariant, Field, Function, Local, - MacroDef, ModPath, ModuleDef, Path, PathKind, Static, Struct, Trait, Type, TypeAlias, - TypeParam, + MacroDef, ModuleDef, Static, Struct, Trait, Type, TypeAlias, TypeParam, }; use base_db::CrateId; @@ -508,7 +508,7 @@ pub(crate) fn resolve_hir_path( /// } /// ``` /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function. -pub(crate) fn resolve_hir_path_qualifier( +fn resolve_hir_path_qualifier( db: &dyn HirDatabase, resolver: &Resolver, path: &Path, From d31634940dbe0b301fb9a99cbd4b8288c78f0e96 Mon Sep 17 00:00:00 2001 From: Dave Lage Date: Sat, 15 Aug 2020 16:37:44 -0400 Subject: [PATCH 18/38] Fix typo in comment --- crates/ide/src/inlay_hints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 002adf9159..596bc872db 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -43,7 +43,7 @@ pub struct InlayHint { // rust-analyzer shows additional information inline with the source code. // Editors usually render this using read-only virtual text snippets interspersed with code. // -// rust-analyzer shows hits for +// rust-analyzer shows hints for // // * types of local variables // * names of function arguments From e8e1eb4263d5bcf109550432143dd54de9f64946 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Mon, 17 Aug 2020 00:19:29 +0900 Subject: [PATCH 19/38] Remove test for `handle_document_symbol` --- .../rust-analyzer/tests/heavy_tests/main.rs | 321 +----------------- 1 file changed, 4 insertions(+), 317 deletions(-) diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index 9a4655e4d5..7370505f8b 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -5,14 +5,11 @@ use std::{collections::HashMap, path::PathBuf, time::Instant}; use lsp_types::{ notification::DidOpenTextDocument, - request::{ - CodeActionRequest, Completion, DocumentSymbolRequest, Formatting, GotoTypeDefinition, - HoverRequest, - }, + request::{CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest}, CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, - DocumentFormattingParams, DocumentSymbolParams, FormattingOptions, GotoDefinitionParams, - HoverParams, PartialResultParams, Position, Range, TextDocumentItem, - TextDocumentPositionParams, WorkDoneProgressParams, + DocumentFormattingParams, FormattingOptions, GotoDefinitionParams, HoverParams, + PartialResultParams, Position, Range, TextDocumentItem, TextDocumentPositionParams, + WorkDoneProgressParams, }; use rust_analyzer::lsp_ext::{OnEnter, Runnables, RunnablesParams}; use serde_json::json; @@ -685,313 +682,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\n```\n\n```rust\nfn bar()\n```""#) } - -#[test] -fn test_document_symbol_with_hierarchy() { - if skip_slow_tests() { - return; - } - - let server = Project::with_fixture( - r#" -//- /Cargo.toml -[package] -name = "foo" -version = "0.0.0" - -//- /src/lib.rs -mod a { - mod b { - struct B1; - fn b2() {} - } - struct A1; -} -"#, - ) - .with_config(|config| { - config.client_caps.hierarchical_symbols = true; - }) - .server(); - server.wait_until_workspace_is_loaded(); - - server.request::( - DocumentSymbolParams { - text_document: server.doc_id("src/lib.rs"), - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: PartialResultParams::default(), - }, - json!([ - { - "children": [ - { - "children": [ - { - "deprecated": false, - "kind": 23, - "name": "B1", - "range": { - "end": { - "character": 18, - "line": 2 - }, - "start": { - "character": 8, - "line": 2 - } - }, - "selectionRange": { - "end": { - "character": 17, - "line": 2 - }, - "start": { - "character": 15, - "line": 2 - } - }, - "tags": [] - }, - { - "deprecated": false, - "detail": "fn()", - "kind": 12, - "name": "b2", - "range": { - "end": { - "character": 18, - "line": 3 - }, - "start": { - "character": 8, - "line": 3 - } - }, - "selectionRange": { - "end": { - "character": 13, - "line": 3 - }, - "start": { - "character": 11, - "line": 3 - } - }, - "tags": [] - } - ], - "deprecated": false, - "kind": 2, - "name": "b", - "range": { - "end": { - "character": 5, - "line": 4 - }, - "start": { - "character": 4, - "line": 1 - } - }, - "selectionRange": { - "end": { - "character": 9, - "line": 1 - }, - "start": { - "character": 8, - "line": 1 - } - }, - "tags": [] - }, - { - "deprecated": false, - "kind": 23, - "name": "A1", - "range": { - "end": { - "character": 14, - "line": 5 - }, - "start": { - "character": 4, - "line": 5 - } - }, - "selectionRange": { - "end": { - "character": 13, - "line": 5 - }, - "start": { - "character": 11, - "line": 5 - } - }, - "tags": [] - } - ], - "deprecated": false, - "kind": 2, - "name": "a", - "range": { - "end": { - "character": 1, - "line": 6 - }, - "start": { - "character": 0, - "line": 0 - } - }, - "selectionRange": { - "end": { - "character": 5, - "line": 0 - }, - "start": { - "character": 4, - "line": 0 - } - }, - "tags": [] - } - ]), - ); -} - -#[test] -fn test_document_symbol_without_hierarchy() { - if skip_slow_tests() { - return; - } - - let server = project( - r#" -//- /Cargo.toml -[package] -name = "foo" -version = "0.0.0" - -//- /src/lib.rs -mod a { - mod b { - struct B1; - fn b2() {} - } - struct A1; -} -"#, - ); - server.wait_until_workspace_is_loaded(); - - server.request::( - DocumentSymbolParams { - text_document: server.doc_id("src/lib.rs"), - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: PartialResultParams::default(), - }, - json!([ - { - "deprecated": false, - "kind": 2, - "location": { - "range": { - "end": { - "character": 1, - "line": 6 - }, - "start": { - "character": 0, - "line": 0 - } - }, - "uri": "file:///[..]/src/lib.rs" - }, - "name": "a", - "tags": [] - }, - { - "containerName": "a", - "deprecated": false, - "kind": 2, - "location": { - "range": { - "end": { - "character": 5, - "line": 4 - }, - "start": { - "character": 4, - "line": 1 - } - }, - "uri": "file:///[..]/src/lib.rs" - }, - "name": "b", - "tags": [] - }, - { - "containerName": "b", - "deprecated": false, - "kind": 23, - "location": { - "range": { - "end": { - "character": 18, - "line": 2 - }, - "start": { - "character": 8, - "line": 2 - } - }, - "uri": "file:///[..]/src/lib.rs" - }, - "name": "B1", - "tags": [] - }, - { - "containerName": "b", - "deprecated": false, - "kind": 12, - "location": { - "range": { - "end": { - "character": 18, - "line": 3 - }, - "start": { - "character": 8, - "line": 3 - } - }, - "uri": "file:///[..]/src/lib.rs" - }, - "name": "b2", - "tags": [] - }, - { - "containerName": "a", - "deprecated": false, - "kind": 23, - "location": { - "range": { - "end": { - "character": 14, - "line": 5 - }, - "start": { - "character": 4, - "line": 5 - } - }, - "uri": "file:///[..]/src/lib.rs" - }, - "name": "A1", - "tags": [] - } - ]), - ); -} From 951fc3f65ac023ef61d3a447399e1432e38700fa Mon Sep 17 00:00:00 2001 From: xiaofa Date: Sun, 16 Aug 2020 23:28:26 +0800 Subject: [PATCH 20/38] Fix eslint errors on .eslintrc.js and rollup.config.js --- editors/code/.eslintignore | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 editors/code/.eslintignore diff --git a/editors/code/.eslintignore b/editors/code/.eslintignore new file mode 100644 index 0000000000..3df5c860bd --- /dev/null +++ b/editors/code/.eslintignore @@ -0,0 +1,3 @@ +node_modules +.eslintrc.js +rollup.config.js \ No newline at end of file From 9aca8d664738716cd07e9c9b0fe3d9d808342f26 Mon Sep 17 00:00:00 2001 From: Jeremy Kolb Date: Sun, 16 Aug 2020 11:34:13 -0400 Subject: [PATCH 21/38] Update chrono --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7809a65a1..40ae42f87f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,9 +214,9 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.13" +version = "0.4.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c74d84029116787153e02106bf53e66828452a4b325cc8652b788b5967c0a0b6" +checksum = "942f72db697d8767c22d46a598e01f2d3b475501ea43d0db4f16d90259182d0b" dependencies = [ "num-integer", "num-traits", From 7819e794db1c3c0e9e930e9b403f929bdb809be5 Mon Sep 17 00:00:00 2001 From: Jeremy Kolb Date: Sun, 16 Aug 2020 11:57:10 -0400 Subject: [PATCH 22/38] Bump rustc_lexer --- Cargo.lock | 4 ++-- crates/syntax/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40ae42f87f..15cc261808 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1264,9 +1264,9 @@ dependencies = [ [[package]] name = "rustc-ap-rustc_lexer" -version = "671.0.0" +version = "673.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22e1221f3bfa2943c942cf8da319ab2346887f8757778c29c7f1822cd27b521f" +checksum = "f6b71fa1285bdefe5fb61e59b63d6cc246abf337f4acafdd620d721bc488e671" dependencies = [ "unicode-xid", ] diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml index 47e351f9d1..ec3132da8d 100644 --- a/crates/syntax/Cargo.toml +++ b/crates/syntax/Cargo.toml @@ -13,7 +13,7 @@ doctest = false [dependencies] itertools = "0.9.0" rowan = "0.10.0" -rustc_lexer = { version = "671.0.0", package = "rustc-ap-rustc_lexer" } +rustc_lexer = { version = "673.0.0", package = "rustc-ap-rustc_lexer" } rustc-hash = "1.1.0" arrayvec = "0.5.1" once_cell = "1.3.1" From 409090e74cd76de35d5ae5ddd7a1e353a9c161f6 Mon Sep 17 00:00:00 2001 From: Jeremy Kolb Date: Sun, 16 Aug 2020 12:15:44 -0400 Subject: [PATCH 23/38] Chalk 0.23 --- Cargo.lock | 16 ++++++++-------- crates/hir_ty/Cargo.toml | 6 +++--- crates/hir_ty/src/traits.rs | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 15cc261808..24dea133dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -162,9 +162,9 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" [[package]] name = "chalk-derive" -version = "0.21.0" +version = "0.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1df0dbb57d74b4acd20f20fa66ab2acd09776b79eaeb9d8f947b2f3e01c40bf" +checksum = "c3cb438e961fd7f1183dc5e0bdcfd09253bf9b90592cf665d1ce6787d8a4908f" dependencies = [ "proc-macro2", "quote", @@ -174,9 +174,9 @@ dependencies = [ [[package]] name = "chalk-ir" -version = "0.21.0" +version = "0.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44361a25dbdb1dc428f56ad7a3c21ba9ca12f3225c26a47919ff6fcb10a583d4" +checksum = "bb332abfcb015b148c6fbab39b1d13282745b0f7f312019dd8e138f5f3f0855d" dependencies = [ "chalk-derive", "lazy_static", @@ -184,9 +184,9 @@ dependencies = [ [[package]] name = "chalk-recursive" -version = "0.21.0" +version = "0.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd89556b98de156d5eaf21077d297cd2198628f10f2df140798ea3a5dd84bc86" +checksum = "e7c7673f10c5fa1acf7fa07d4f4c5917cbcf161ed3a952d14530c79950de32d2" dependencies = [ "chalk-derive", "chalk-ir", @@ -197,9 +197,9 @@ dependencies = [ [[package]] name = "chalk-solve" -version = "0.21.0" +version = "0.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a886da37a0dc457057d86f78f026f7a09c6d8088aa13f4f4127fdb8dc80119a3" +checksum = "802de4eff72e5a5d2828e6c07224c74d66949dc6308aff025d0ae2871a11b4eb" dependencies = [ "chalk-derive", "chalk-ir", diff --git a/crates/hir_ty/Cargo.toml b/crates/hir_ty/Cargo.toml index 83b5013a90..a319b0ce8e 100644 --- a/crates/hir_ty/Cargo.toml +++ b/crates/hir_ty/Cargo.toml @@ -16,9 +16,9 @@ ena = "0.14.0" log = "0.4.8" rustc-hash = "1.1.0" scoped-tls = "1" -chalk-solve = { version = "0.21.0" } -chalk-ir = { version = "0.21.0" } -chalk-recursive = { version = "0.21.0" } +chalk-solve = { version = "0.23.0" } +chalk-ir = { version = "0.23.0" } +chalk-recursive = { version = "0.23.0" } stdx = { path = "../stdx" } hir_def = { path = "../hir_def" } diff --git a/crates/hir_ty/src/traits.rs b/crates/hir_ty/src/traits.rs index 1c3abb18f0..14cd3a2b44 100644 --- a/crates/hir_ty/src/traits.rs +++ b/crates/hir_ty/src/traits.rs @@ -170,11 +170,11 @@ fn solve( let mut solve = || { if is_chalk_print() { let logging_db = LoggingRustIrDatabase::new(context); - let solution = solver.solve_limited(&logging_db, goal, should_continue); + let solution = solver.solve_limited(&logging_db, goal, &should_continue); log::debug!("chalk program:\n{}", logging_db); solution } else { - solver.solve_limited(&context, goal, should_continue) + solver.solve_limited(&context, goal, &should_continue) } }; From 38e3088a563b634d593b7289281f344ddb22f97f Mon Sep 17 00:00:00 2001 From: jDomantas Date: Mon, 17 Aug 2020 10:47:13 +0300 Subject: [PATCH 24/38] update generated tests --- crates/assists/src/tests/generated.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index d16e6fb0a6..1735670037 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs @@ -82,8 +82,8 @@ trait Trait { impl Trait for () { Type X = (); fn foo(&self) {} - $0fn bar(&self) {} + $0fn bar(&self) {} } "#####, ) @@ -115,7 +115,6 @@ impl Trait for () { fn foo(&self) -> u32 { ${0:todo!()} } - } "#####, ) From a565a42f46a2ad20dac598046943b09429f1a946 Mon Sep 17 00:00:00 2001 From: jDomantas Date: Mon, 17 Aug 2020 11:36:46 +0300 Subject: [PATCH 25/38] format --- crates/syntax/src/ast/edit.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index b295b5bc67..060b209663 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs @@ -119,27 +119,23 @@ impl ast::AssocItemList { /// Remove extra whitespace between last item and closing curly brace. fn fixup_trailing_whitespace(&self) -> Option { - let first_token_after_items = self - .assoc_items() - .last()? - .syntax() - .next_sibling_or_token()?; - let last_token_before_curly = self - .r_curly_token()? - .prev_sibling_or_token()?; + let first_token_after_items = + self.assoc_items().last()?.syntax().next_sibling_or_token()?; + let last_token_before_curly = self.r_curly_token()?.prev_sibling_or_token()?; if last_token_before_curly != first_token_after_items { // there is something more between last item and // right curly than just whitespace - bail out return None; } - let whitespace = last_token_before_curly - .clone() - .into_token() - .and_then(ast::Whitespace::cast)?; + let whitespace = + last_token_before_curly.clone().into_token().and_then(ast::Whitespace::cast)?; let text = whitespace.syntax().text(); let newline = text.rfind("\n")?; let keep = tokens::WsBuilder::new(&text[newline..]); - Some(self.replace_children(first_token_after_items..=last_token_before_curly, std::iter::once(keep.ws().into()))) + Some(self.replace_children( + first_token_after_items..=last_token_before_curly, + std::iter::once(keep.ws().into()), + )) } } From 1eed036a6e68aee6128d099e3a8f2c06a90b846b Mon Sep 17 00:00:00 2001 From: vsrs Date: Mon, 17 Aug 2020 14:56:27 +0300 Subject: [PATCH 26/38] Fix StatusNotification --- crates/rust-analyzer/src/lsp_ext.rs | 7 ++++++- crates/rust-analyzer/src/reload.rs | 5 ++++- docs/dev/lsp-extensions.md | 8 +++++++- editors/code/src/ctx.ts | 2 +- editors/code/src/lsp_ext.ts | 5 ++++- 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 3976b6529e..e1a28b1b4b 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -237,8 +237,13 @@ pub enum Status { Invalid, } +#[derive(Deserialize, Serialize)] +pub struct StatusParams { + pub status: Status, +} + impl Notification for StatusNotification { - type Params = Status; + type Params = StatusParams; const METHOD: &'static str = "rust-analyzer/status"; } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 1907f2f132..b70efcb4d2 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -14,6 +14,7 @@ use crate::{ lsp_ext, main_loop::Task, }; +use lsp_ext::StatusParams; impl GlobalState { pub(crate) fn update_configuration(&mut self, config: Config) { @@ -86,7 +87,9 @@ impl GlobalState { Status::Invalid => lsp_ext::Status::Invalid, Status::NeedsReload => lsp_ext::Status::NeedsReload, }; - self.send_notification::(lsp_status); + self.send_notification::(StatusParams { + status: lsp_status, + }); } } pub(crate) fn fetch_workspaces(&mut self) { diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index 1be01fd884..2e3133449f 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -412,7 +412,13 @@ Reloads project information (that is, re-executes `cargo metadata`). **Method:** `rust-analyzer/status` -**Notification:** `"loading" | "ready" | "invalid" | "needsReload"` +**Notification:** + +```typescript +interface StatusParams { + status: "loading" | "ready" | "invalid" | "needsReload", +} +``` This notification is sent from server to client. The client can use it to display persistent status to the user (in modline). diff --git a/editors/code/src/ctx.ts b/editors/code/src/ctx.ts index 6e767babf4..543f7e02e3 100644 --- a/editors/code/src/ctx.ts +++ b/editors/code/src/ctx.ts @@ -36,7 +36,7 @@ export class Ctx { res.pushCleanup(client.start()); await client.onReady(); - client.onNotification(ra.status, (status) => res.setStatus(status)); + client.onNotification(ra.status, (params) => res.setStatus(params.status)); return res; } diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index 494d51c83a..8663737a68 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -8,7 +8,10 @@ export const analyzerStatus = new lc.RequestType("rust-analy export const memoryUsage = new lc.RequestType("rust-analyzer/memoryUsage"); export type Status = "loading" | "ready" | "invalid" | "needsReload"; -export const status = new lc.NotificationType("rust-analyzer/status"); +export interface StatusParams { + status: Status; +} +export const status = new lc.NotificationType("rust-analyzer/status"); export const reloadWorkspace = new lc.RequestType("rust-analyzer/reloadWorkspace"); From b82d967182d29f1302187fa0e3a56661d0c4082a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 17 Aug 2020 15:49:46 +0200 Subject: [PATCH 27/38] Mention that generated .adocs are generaterd --- xtask/src/codegen.rs | 18 +++++++++++++++++- xtask/src/codegen/gen_assists_docs.rs | 6 +++--- xtask/src/codegen/gen_feature_docs.rs | 4 ++-- xtask/src/codegen/gen_syntax.rs | 8 ++++---- xtask/src/lib.rs | 11 ----------- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/xtask/src/codegen.rs b/xtask/src/codegen.rs index 78a84f68d0..98acd7fa67 100644 --- a/xtask/src/codegen.rs +++ b/xtask/src/codegen.rs @@ -15,7 +15,11 @@ use std::{ path::{Path, PathBuf}, }; -use crate::{not_bash::fs2, project_root, Result}; +use crate::{ + ensure_rustfmt, + not_bash::{fs2, pushenv, run}, + project_root, Result, +}; pub use self::{ gen_assists_docs::{generate_assists_docs, generate_assists_tests}, @@ -62,6 +66,18 @@ fn update(path: &Path, contents: &str, mode: Mode) -> Result<()> { } } +const PREAMBLE: &str = "Generated file, do not edit by hand, see `xtask/src/codegen`"; + +fn reformat(text: impl std::fmt::Display) -> Result { + let _e = pushenv("RUSTUP_TOOLCHAIN", "stable"); + ensure_rustfmt()?; + let stdout = run!( + "rustfmt --config-path {} --config fn_single_line=true", project_root().join("rustfmt.toml").display(); + Vec> { do_extract_comment_blocks(text, false).into_iter().map(|(_line, block)| block).collect() } diff --git a/xtask/src/codegen/gen_assists_docs.rs b/xtask/src/codegen/gen_assists_docs.rs index 526941f73a..4f49685944 100644 --- a/xtask/src/codegen/gen_assists_docs.rs +++ b/xtask/src/codegen/gen_assists_docs.rs @@ -3,7 +3,7 @@ use std::{fmt, fs, path::Path}; use crate::{ - codegen::{self, extract_comment_blocks_with_empty_lines, Location, Mode}, + codegen::{self, extract_comment_blocks_with_empty_lines, reformat, Location, Mode, PREAMBLE}, project_root, rust_files, Result, }; @@ -15,7 +15,7 @@ pub fn generate_assists_tests(mode: Mode) -> Result<()> { pub fn generate_assists_docs(mode: Mode) -> Result<()> { let assists = Assist::collect()?; let contents = assists.into_iter().map(|it| it.to_string()).collect::>().join("\n\n"); - let contents = contents.trim().to_string() + "\n"; + let contents = format!("//{}\n{}\n", PREAMBLE, contents.trim()); let dst = project_root().join("docs/user/generated_assists.adoc"); codegen::update(&dst, &contents, mode) } @@ -134,7 +134,7 @@ r#####" buf.push_str(&test) } - let buf = crate::reformat(buf)?; + let buf = reformat(buf)?; codegen::update(&project_root().join(codegen::ASSISTS_TESTS), &buf, mode) } diff --git a/xtask/src/codegen/gen_feature_docs.rs b/xtask/src/codegen/gen_feature_docs.rs index 31bc3839d0..3f0013e82a 100644 --- a/xtask/src/codegen/gen_feature_docs.rs +++ b/xtask/src/codegen/gen_feature_docs.rs @@ -3,14 +3,14 @@ use std::{fmt, fs, path::PathBuf}; use crate::{ - codegen::{self, extract_comment_blocks_with_empty_lines, Location, Mode}, + codegen::{self, extract_comment_blocks_with_empty_lines, Location, Mode, PREAMBLE}, project_root, rust_files, Result, }; pub fn generate_feature_docs(mode: Mode) -> Result<()> { let features = Feature::collect()?; let contents = features.into_iter().map(|it| it.to_string()).collect::>().join("\n\n"); - let contents = contents.trim().to_string() + "\n"; + let contents = format!("//{}\n{}\n", PREAMBLE, contents.trim()); let dst = project_root().join("docs/user/generated_features.adoc"); codegen::update(&dst, &contents, mode)?; Ok(()) diff --git a/xtask/src/codegen/gen_syntax.rs b/xtask/src/codegen/gen_syntax.rs index dd1f4d6a2c..df3ec22c8d 100644 --- a/xtask/src/codegen/gen_syntax.rs +++ b/xtask/src/codegen/gen_syntax.rs @@ -14,7 +14,7 @@ use ungrammar::{rust_grammar, Grammar, Rule}; use crate::{ ast_src::{AstEnumSrc, AstNodeSrc, AstSrc, Cardinality, Field, KindsSrc, KINDS_SRC}, - codegen::{self, update, Mode}, + codegen::{self, reformat, update, Mode}, project_root, Result, }; @@ -61,7 +61,7 @@ fn generate_tokens(grammar: &AstSrc) -> Result { } }); - let pretty = crate::reformat(quote! { + let pretty = reformat(quote! { use crate::{SyntaxKind::{self, *}, SyntaxToken, ast::AstToken}; #(#tokens)* })? @@ -261,7 +261,7 @@ fn generate_nodes(kinds: KindsSrc<'_>, grammar: &AstSrc) -> Result { } } - let pretty = crate::reformat(res)?; + let pretty = reformat(res)?; Ok(pretty) } @@ -383,7 +383,7 @@ fn generate_syntax_kinds(grammar: KindsSrc<'_>) -> Result { } }; - crate::reformat(ast) + reformat(ast) } fn to_upper_snake_case(s: &str) -> String { diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index 807ef587ce..f3ad81ba72 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -62,17 +62,6 @@ pub fn run_rustfmt(mode: Mode) -> Result<()> { Ok(()) } -fn reformat(text: impl std::fmt::Display) -> Result { - let _e = pushenv("RUSTUP_TOOLCHAIN", "stable"); - ensure_rustfmt()?; - let stdout = run!( - "rustfmt --config-path {} --config fn_single_line=true", project_root().join("rustfmt.toml").display(); - Result<()> { let out = run!("rustfmt --version")?; if !out.contains("stable") { From 6a4c9fc9fd6cb9ecf08bd5a22890eb19d22ad34e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 17 Aug 2020 16:11:29 +0200 Subject: [PATCH 28/38] Don't make fields private unless you have to --- crates/assists/src/lib.rs | 37 ++++++++-------------------- crates/rust-analyzer/src/handlers.rs | 4 +-- crates/rust-analyzer/src/to_proto.rs | 8 +++--- docs/dev/style.md | 29 ++++++++++++++++++++++ 4 files changed, 45 insertions(+), 33 deletions(-) diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index ae90d68a35..c589b08dc4 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -66,13 +66,13 @@ pub struct GroupLabel(pub String); #[derive(Debug, Clone)] pub struct Assist { - id: AssistId, + pub id: AssistId, /// Short description of the assist, as shown in the UI. label: String, - group: Option, + pub group: Option, /// Target ranges are used to sort assists: the smaller the target range, /// the more specific assist is, and so it should be sorted first. - target: TextRange, + pub target: TextRange, } #[derive(Debug, Clone)] @@ -82,6 +82,11 @@ pub struct ResolvedAssist { } impl Assist { + fn new(id: AssistId, label: String, group: Option, target: TextRange) -> Assist { + assert!(label.starts_with(char::is_uppercase)); + Assist { id, label, group, target } + } + /// Return all the assists applicable at the given position. /// /// Assists are returned in the "unresolved" state, that is only labels are @@ -114,30 +119,8 @@ impl Assist { acc.finish_resolved() } - pub(crate) fn new( - id: AssistId, - label: String, - group: Option, - target: TextRange, - ) -> Assist { - assert!(label.starts_with(|c: char| c.is_uppercase())); - Assist { id, label, group, target } - } - - pub fn id(&self) -> AssistId { - self.id - } - - pub fn label(&self) -> String { - self.label.clone() - } - - pub fn group(&self) -> Option { - self.group.clone() - } - - pub fn target(&self) -> TextRange { - self.target + pub fn label(&self) -> &str { + self.label.as_str() } } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 74f73655a4..e05ffc768e 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -859,10 +859,10 @@ pub(crate) fn handle_resolve_code_action( .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect()); let assists = snap.analysis.resolved_assists(&snap.config.assist, frange)?; - let (id_string, index) = split_once(¶ms.id, ':').unwrap(); + let (id, index) = split_once(¶ms.id, ':').unwrap(); let index = index.parse::().unwrap(); let assist = &assists[index]; - assert!(assist.assist.id().0 == id_string); + assert!(assist.assist.id.0 == id); Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit) } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 8a2cfa2aee..535de2f71a 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -704,10 +704,10 @@ pub(crate) fn unresolved_code_action( index: usize, ) -> Result { let res = lsp_ext::CodeAction { - title: assist.label(), - id: Some(format!("{}:{}", assist.id().0.to_owned(), index.to_string())), - group: assist.group().filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0), - kind: Some(code_action_kind(assist.id().1)), + title: assist.label().to_string(), + id: Some(format!("{}:{}", assist.id.0, index.to_string())), + group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0), + kind: Some(code_action_kind(assist.id.1)), edit: None, is_preferred: None, }; diff --git a/docs/dev/style.md b/docs/dev/style.md index 963a6d73d0..8effddcda5 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -176,6 +176,35 @@ fn frobnicate(walrus: Option) { } ``` +# Getters & Setters + +If a field can have any value without breaking invariants, make the field public. +Conversely, if there is an invariant, document it, enforce it in the "constructor" function, make the field private, and provide a getter. +Never provide setters. + +Getters should return borrowed data: + +``` +struct Person { + // Invariant: never empty + first_name: String, + middle_name: Option +} + +// Good +impl Person { + fn first_name(&self) -> &str { self.first_name.as_str() } + fn middle_name(&self) -> Option<&str> { self.middle_name.as_ref() } +} + +// Not as good +impl Person { + fn first_name(&self) -> String { self.first_name.clone() } + fn middle_name(&self) -> &Option { &self.middle_name } +} +``` + + # Premature Pessimization Avoid writing code which is slower than it needs to be. From 2eaf79cfbb447156151cb5435eff5f14f41c40f7 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Mon, 17 Aug 2020 13:19:15 -0400 Subject: [PATCH 29/38] Document missing match arm false positive This should already be guarded against (https://github.com/rust-analyzer/rust-analyzer/blob/d2212a49f6d447a14cdc87a9de2a4844e78b6905/crates/hir_ty/src/diagnostics/expr.rs#L225-L230) but it isn't preventing this false positive for some reason. --- crates/hir_ty/src/diagnostics/match_check.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/hir_ty/src/diagnostics/match_check.rs b/crates/hir_ty/src/diagnostics/match_check.rs index 7f007f1d65..1602c3fb4c 100644 --- a/crates/hir_ty/src/diagnostics/match_check.rs +++ b/crates/hir_ty/src/diagnostics/match_check.rs @@ -1335,6 +1335,25 @@ fn panic(a: Category, b: Category) { ); } + #[test] + fn unknown_type() { + check_diagnostics( + r#" +enum Option { Some(T), None } + +fn main() { + // FIXME: This is a false positive, as the `Never` type is not known here. + // `Never` is deliberately not defined so that it's an uninferred type. + match Option::::None { + None => (), + Some(never) => match never {}, + // ^^^^^ Missing match arm + } +} +"#, + ); + } + mod false_negatives { //! The implementation of match checking here is a work in progress. As we roll this out, we //! prefer false negatives to false positives (ideally there would be no false positives). This From c822bb68ce78171e4017938a66118fc4ccf077d5 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Mon, 17 Aug 2020 13:27:12 -0400 Subject: [PATCH 30/38] Fix missing match arm false error on unknown type --- crates/hir_ty/src/diagnostics/expr.rs | 4 ++-- crates/hir_ty/src/diagnostics/match_check.rs | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index fb76e2e4ec..278a4b9472 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -223,10 +223,10 @@ impl<'a, 'b> ExprValidator<'a, 'b> { db.body_with_source_map(self.owner.into()); let match_expr_ty = match infer.type_of_expr.get(match_expr) { - Some(ty) => ty, // If we can't resolve the type of the match expression // we cannot perform exhaustiveness checks. - None => return, + None | Some(Ty::Unknown) => return, + Some(ty) => ty, }; let cx = MatchCheckCtx { match_expr, body, infer: infer.clone(), db }; diff --git a/crates/hir_ty/src/diagnostics/match_check.rs b/crates/hir_ty/src/diagnostics/match_check.rs index 1602c3fb4c..5bd03f2ac0 100644 --- a/crates/hir_ty/src/diagnostics/match_check.rs +++ b/crates/hir_ty/src/diagnostics/match_check.rs @@ -1342,12 +1342,10 @@ fn panic(a: Category, b: Category) { enum Option { Some(T), None } fn main() { - // FIXME: This is a false positive, as the `Never` type is not known here. // `Never` is deliberately not defined so that it's an uninferred type. match Option::::None { None => (), Some(never) => match never {}, - // ^^^^^ Missing match arm } } "#, From 80ab6c8cd53bc9bca43b8b95e80e39677cd319f8 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 09:38:32 +0200 Subject: [PATCH 31/38] Re-enable mac build --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2deb009cef..ea194a9442 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -25,7 +25,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] #, macos-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - name: Checkout repository From 6cff076513924430c8cdf422fa56dbe711faee40 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 10:38:57 +0200 Subject: [PATCH 32/38] Revive cache cleaning The idea here is that, on CI, we only want to cache crates.io dependencies, and not local crates. This keeps the size of the cache low, and also improves performance, as network and moving files on disk (on Windows) can be slow. --- .../{heavy_tests => rust-analyzer}/main.rs | 10 +++ .../{heavy_tests => rust-analyzer}/support.rs | 0 .../{heavy_tests => rust-analyzer}/testdir.rs | 0 xtask/src/lib.rs | 45 ++--------- xtask/src/main.rs | 5 +- xtask/src/pre_cache.rs | 80 +++++++++++++++++++ 6 files changed, 98 insertions(+), 42 deletions(-) rename crates/rust-analyzer/tests/{heavy_tests => rust-analyzer}/main.rs (97%) rename crates/rust-analyzer/tests/{heavy_tests => rust-analyzer}/support.rs (100%) rename crates/rust-analyzer/tests/{heavy_tests => rust-analyzer}/testdir.rs (100%) create mode 100644 xtask/src/pre_cache.rs diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/rust-analyzer/main.rs similarity index 97% rename from crates/rust-analyzer/tests/heavy_tests/main.rs rename to crates/rust-analyzer/tests/rust-analyzer/main.rs index 7370505f8b..fa315ff8ee 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/rust-analyzer/main.rs @@ -1,3 +1,13 @@ +//! The most high-level integrated tests for rust-analyzer. +//! +//! This tests run a full LSP event loop, spawn cargo and process stdlib from +//! sysroot. For this reason, the tests here are very slow, and should be +//! avoided unless absolutely necessary. +//! +//! In particular, it's fine *not* to test that client & server agree on +//! specific JSON shapes here -- there's little value in such tests, as we can't +//! be sure without a real client anyway. + mod testdir; mod support; diff --git a/crates/rust-analyzer/tests/heavy_tests/support.rs b/crates/rust-analyzer/tests/rust-analyzer/support.rs similarity index 100% rename from crates/rust-analyzer/tests/heavy_tests/support.rs rename to crates/rust-analyzer/tests/rust-analyzer/support.rs diff --git a/crates/rust-analyzer/tests/heavy_tests/testdir.rs b/crates/rust-analyzer/tests/rust-analyzer/testdir.rs similarity index 100% rename from crates/rust-analyzer/tests/heavy_tests/testdir.rs rename to crates/rust-analyzer/tests/rust-analyzer/testdir.rs diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index f3ad81ba72..e790d995fb 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -3,14 +3,15 @@ //! See https://github.com/matklad/cargo-xtask/ pub mod not_bash; +pub mod codegen; +mod ast_src; + pub mod install; pub mod release; pub mod dist; pub mod pre_commit; pub mod metrics; - -pub mod codegen; -mod ast_src; +pub mod pre_cache; use std::{ env, @@ -21,7 +22,7 @@ use walkdir::{DirEntry, WalkDir}; use crate::{ codegen::Mode, - not_bash::{fs2, pushd, pushenv, rm_rf}, + not_bash::{pushd, pushenv}, }; pub use anyhow::{bail, Context as _, Result}; @@ -108,42 +109,6 @@ pub fn run_fuzzer() -> Result<()> { Ok(()) } -/// Cleans the `./target` dir after the build such that only -/// dependencies are cached on CI. -pub fn run_pre_cache() -> Result<()> { - let slow_tests_cookie = Path::new("./target/.slow_tests_cookie"); - if !slow_tests_cookie.exists() { - panic!("slow tests were skipped on CI!") - } - rm_rf(slow_tests_cookie)?; - - for entry in Path::new("./target/debug").read_dir()? { - let entry = entry?; - if entry.file_type().map(|it| it.is_file()).ok() == Some(true) { - // Can't delete yourself on windows :-( - if !entry.path().ends_with("xtask.exe") { - rm_rf(&entry.path())? - } - } - } - - fs2::remove_file("./target/.rustc_info.json")?; - let to_delete = ["hir", "heavy_test", "xtask", "ide", "rust-analyzer"]; - for &dir in ["./target/debug/deps", "target/debug/.fingerprint"].iter() { - for entry in Path::new(dir).read_dir()? { - let entry = entry?; - if to_delete.iter().any(|&it| entry.path().display().to_string().contains(it)) { - // Can't delete yourself on windows :-( - if !entry.path().ends_with("xtask.exe") { - rm_rf(&entry.path())? - } - } - } - } - - Ok(()) -} - fn is_release_tag(tag: &str) -> bool { tag.len() == "2020-02-24".len() && tag.starts_with(|c: char| c.is_ascii_digit()) } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index b69b884e54..fb38fdc92f 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -17,9 +17,10 @@ use xtask::{ install::{ClientOpt, InstallCmd, Malloc, ServerOpt}, metrics::MetricsCmd, not_bash::pushd, + pre_cache::PreCacheCmd, pre_commit, project_root, release::{PromoteCmd, ReleaseCmd}, - run_clippy, run_fuzzer, run_pre_cache, run_rustfmt, Result, + run_clippy, run_fuzzer, run_rustfmt, Result, }; fn main() -> Result<()> { @@ -100,7 +101,7 @@ FLAGS: } "pre-cache" => { args.finish()?; - run_pre_cache() + PreCacheCmd.run() } "release" => { let dry_run = args.contains("--dry-run"); diff --git a/xtask/src/pre_cache.rs b/xtask/src/pre_cache.rs new file mode 100644 index 0000000000..47ba6ba246 --- /dev/null +++ b/xtask/src/pre_cache.rs @@ -0,0 +1,80 @@ +use std::{ + fs::FileType, + path::{Path, PathBuf}, +}; + +use anyhow::Result; + +use crate::not_bash::{fs2, rm_rf}; + +pub struct PreCacheCmd; + +impl PreCacheCmd { + /// Cleans the `./target` dir after the build such that only + /// dependencies are cached on CI. + pub fn run(self) -> Result<()> { + let slow_tests_cookie = Path::new("./target/.slow_tests_cookie"); + if !slow_tests_cookie.exists() { + panic!("slow tests were skipped on CI!") + } + rm_rf(slow_tests_cookie)?; + + for path in read_dir("./target/debug", FileType::is_file)? { + // Can't delete yourself on windows :-( + if !path.ends_with("xtask.exe") { + rm_rf(&path)? + } + } + + fs2::remove_file("./target/.rustc_info.json")?; + + let to_delete = read_dir("./crates", FileType::is_dir)? + .into_iter() + .map(|path| path.file_name().unwrap().to_string_lossy().replace('-', "_")) + .collect::>(); + + for &dir in ["./target/debug/deps", "target/debug/.fingerprint"].iter() { + for path in read_dir(dir, |_file_type| true)? { + if path.ends_with("xtask.exe") { + continue; + } + let file_name = path.file_name().unwrap().to_string_lossy(); + let (stem, _) = match rsplit_once(&file_name, '-') { + Some(it) => it, + None => { + rm_rf(path)?; + continue; + } + }; + let stem = stem.replace('-', "_"); + if to_delete.contains(&stem) { + rm_rf(path)?; + } + } + } + + Ok(()) + } +} +fn read_dir(path: impl AsRef, cond: impl Fn(&FileType) -> bool) -> Result> { + read_dir_impl(path.as_ref(), &cond) +} + +fn read_dir_impl(path: &Path, cond: &dyn Fn(&FileType) -> bool) -> Result> { + let mut res = Vec::new(); + for entry in path.read_dir()? { + let entry = entry?; + let file_type = entry.file_type()?; + if cond(&file_type) { + res.push(entry.path()) + } + } + Ok(res) +} + +fn rsplit_once(haystack: &str, delim: char) -> Option<(&str, &str)> { + let mut split = haystack.rsplitn(2, delim); + let suffix = split.next()?; + let prefix = split.next()?; + Some((prefix, suffix)) +} From 90cb4bbbb99236f58000f972f4a4c8ada005b98c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 11:19:44 +0200 Subject: [PATCH 33/38] Remove usless pre-cache task We are not cleaning the rest of xtask artifacts, so this effectively does nothing. xtask is small and changes rarely, so this shouldn't really matter. --- .github/workflows/ci.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ea194a9442..fb077e28d5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -70,10 +70,6 @@ jobs: - name: Prepare cache run: cargo xtask pre-cache - - name: Prepare cache 2 - if: matrix.os == 'windows-latest' - run: Remove-Item ./target/debug/xtask.exe, ./target/debug/deps/xtask.exe - # Weird targets to catch non-portable code rust-cross: name: Rust Cross From 88adca766aca7c8e771b422c33f44afbfc6d0d81 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 10:49:18 +0200 Subject: [PATCH 34/38] :arrow_up: crates --- Cargo.lock | 16 ++++++++-------- xtask/tests/tidy.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 24dea133dd..7abfeaaebe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -765,9 +765,9 @@ dependencies = [ [[package]] name = "lsp-server" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53b4ace8ebe5d2aff3687ce0ed507f6020d6a47a7de2b0d3d664ea237ffb0c62" +checksum = "87fce8851309a325974ec76efe7c9d954d152c9ff4fded6520eb3c96d0aa3a96" dependencies = [ "crossbeam-channel", "log", @@ -971,9 +971,9 @@ checksum = "1ab52be62400ca80aa00285d25253d7f7c437b7375c4de678f5405d3afe82ca5" [[package]] name = "once_cell" -version = "1.4.0" +version = "1.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b631f7e854af39a1739f401cf34a8a013dfe09eac4fa4dba91e9768bd28168d" +checksum = "260e51e7efe62b592207e9e13a68e43692a7a279171d6ba57abd208bf23645ad" [[package]] name = "oorandom" @@ -1036,9 +1036,9 @@ dependencies = [ [[package]] name = "perf-event-open-sys" -version = "0.3.2" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83e7183862f36d10263d0a1ccaef50fef734ade948bf026afd1bd97355c78273" +checksum = "d9ebe2b9ef0cb884ef778c5a533144e348e9839a9fcf67f3d24e1890ac9088d6" dependencies = [ "libc", ] @@ -1578,9 +1578,9 @@ dependencies = [ [[package]] name = "tinyvec" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53953d2d3a5ad81d9f844a32f14ebb121f50b650cd59d0ee2a07cf13c617efed" +checksum = "238ce071d267c5710f9d31451efec16c5ee22de34df17cc05e56cbc92e967117" [[package]] name = "toolchain" diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index 76895aeca0..ebd42cef7e 100644 --- a/xtask/tests/tidy.rs +++ b/xtask/tests/tidy.rs @@ -82,7 +82,7 @@ MIT/Apache-2.0 MIT/Apache-2.0 AND BSD-2-Clause Unlicense OR MIT Unlicense/MIT -Zlib +Zlib OR Apache-2.0 OR MIT " .lines() .filter(|it| !it.is_empty()) From b56cfcca10994ec2bf1878f222afdb375459f8d3 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Tue, 18 Aug 2020 13:32:29 +0300 Subject: [PATCH 35/38] Make disabled diagnostics an argument of corresponding function --- crates/ide/src/diagnostics.rs | 36 +++++++++---------- crates/ide/src/lib.rs | 20 ++++------- .../rust-analyzer/src/cli/analysis_bench.rs | 2 +- crates/rust-analyzer/src/cli/diagnostics.rs | 2 +- crates/rust-analyzer/src/config.rs | 8 +++++ crates/rust-analyzer/src/global_state.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 12 +++++-- 7 files changed, 43 insertions(+), 39 deletions(-) diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index a54df37db5..606a6064b4 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -4,7 +4,7 @@ //! macro-expanded files, but we need to present them to the users in terms of //! original files. So we need to map the ranges. -use std::cell::RefCell; +use std::{cell::RefCell, collections::HashSet}; use base_db::SourceDatabase; use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; @@ -16,7 +16,7 @@ use syntax::{ }; use text_edit::TextEdit; -use crate::{AnalysisConfig, Diagnostic, FileId, Fix, SourceFileEdit}; +use crate::{Diagnostic, FileId, Fix, SourceFileEdit}; mod diagnostics_with_fix; use diagnostics_with_fix::DiagnosticWithFix; @@ -31,7 +31,7 @@ pub(crate) fn diagnostics( db: &RootDatabase, file_id: FileId, enable_experimental: bool, - analysis_config: &AnalysisConfig, + disabled_diagnostics: Option>, ) -> Vec { let _p = profile::span("diagnostics"); let sema = Semantics::new(db); @@ -68,10 +68,9 @@ pub(crate) fn diagnostics( // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental); - if !analysis_config.disabled_diagnostics.is_empty() { + if let Some(disabled_diagnostics) = disabled_diagnostics { // Do not collect disabled diagnostics. - sink_builder = - sink_builder.filter(|diag| !analysis_config.disabled_diagnostics.contains(diag.name())); + sink_builder = sink_builder.filter(move |diag| !disabled_diagnostics.contains(diag.name())); } // Finalize the `DiagnosticSink` building process. @@ -192,10 +191,7 @@ mod tests { use stdx::trim_indent; use test_utils::assert_eq_text; - use crate::{ - mock_analysis::{analysis_and_position, single_file, MockAnalysis}, - AnalysisConfig, - }; + use crate::mock_analysis::{analysis_and_position, single_file, MockAnalysis}; use expect::{expect, Expect}; /// Takes a multi-file input fixture with annotated cursor positions, @@ -207,7 +203,8 @@ mod tests { let after = trim_indent(ra_fixture_after); let (analysis, file_position) = analysis_and_position(ra_fixture_before); - let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap(); + let diagnostic = + analysis.diagnostics(file_position.file_id, true, None).unwrap().pop().unwrap(); let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap().edit; let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); @@ -233,7 +230,7 @@ mod tests { let ra_fixture_after = &trim_indent(ra_fixture_after); let (analysis, file_pos) = analysis_and_position(ra_fixture_before); let current_file_id = file_pos.file_id; - let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap(); + let diagnostic = analysis.diagnostics(current_file_id, true, None).unwrap().pop().unwrap(); let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap(); let changed_file_id = edit.file_id; @@ -254,7 +251,7 @@ mod tests { let analysis = mock.analysis(); let diagnostics = files .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) .collect::>(); assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); } @@ -267,13 +264,14 @@ mod tests { let mock = MockAnalysis::with_files(ra_fixture); let files = mock.files().map(|(it, _)| it).collect::>(); - let mut analysis = mock.analysis(); - analysis.set_config(AnalysisConfig { disabled_diagnostics: disabled_diagnostics.clone() }); + let analysis = mock.analysis(); let diagnostics = files .clone() .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .flat_map(|file_id| { + analysis.diagnostics(file_id, true, Some(disabled_diagnostics.clone())).unwrap() + }) .collect::>(); // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list. @@ -288,11 +286,9 @@ mod tests { // This is required for tests to not become outdated if e.g. diagnostics name changes: // without this additional run the test will pass simply because a diagnostic with an old name // will no longer exist. - analysis.set_config(AnalysisConfig { disabled_diagnostics: Default::default() }); - let diagnostics = files .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) .collect::>(); assert!( @@ -306,7 +302,7 @@ mod tests { fn check_expect(ra_fixture: &str, expect: Expect) { let (analysis, file_id) = single_file(ra_fixture); - let diagnostics = analysis.diagnostics(file_id, true).unwrap(); + let diagnostics = analysis.diagnostics(file_id, true, None).unwrap(); expect.assert_debug_eq(&diagnostics) } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index b762c994e4..a19a379c65 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -151,16 +151,11 @@ impl RangeInfo { #[derive(Debug)] pub struct AnalysisHost { db: RootDatabase, - config: AnalysisConfig, } impl AnalysisHost { pub fn new(lru_capacity: Option) -> Self { - Self::with_config(lru_capacity, AnalysisConfig::default()) - } - - pub fn with_config(lru_capacity: Option, config: AnalysisConfig) -> Self { - AnalysisHost { db: RootDatabase::new(lru_capacity), config } + AnalysisHost { db: RootDatabase::new(lru_capacity) } } pub fn update_lru_capacity(&mut self, lru_capacity: Option) { @@ -170,7 +165,7 @@ impl AnalysisHost { /// Returns a snapshot of the current state, which you can query for /// semantic information. pub fn analysis(&self) -> Analysis { - Analysis { db: self.db.snapshot(), config: self.config.clone() } + Analysis { db: self.db.snapshot() } } /// Applies changes to the current state of the world. If there are @@ -214,7 +209,6 @@ impl Default for AnalysisHost { #[derive(Debug)] pub struct Analysis { db: salsa::Snapshot, - config: AnalysisConfig, } // As a general design guideline, `Analysis` API are intended to be independent @@ -509,8 +503,11 @@ impl Analysis { &self, file_id: FileId, enable_experimental: bool, + disabled_diagnostics: Option>, ) -> Cancelable> { - self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental, &self.config)) + self.with_db(|db| { + diagnostics::diagnostics(db, file_id, enable_experimental, disabled_diagnostics) + }) } /// Returns the edit required to rename reference at the position to the new @@ -539,11 +536,6 @@ impl Analysis { }) } - /// Sets the provided config. - pub fn set_config(&mut self, config: AnalysisConfig) { - self.config = config; - } - /// Performs an operation on that may be Canceled. fn with_db(&self, f: F) -> Cancelable where diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs index 0f614f9e0c..43f0196afc 100644 --- a/crates/rust-analyzer/src/cli/analysis_bench.rs +++ b/crates/rust-analyzer/src/cli/analysis_bench.rs @@ -71,7 +71,7 @@ impl BenchCmd { match &self.what { BenchWhat::Highlight { .. } => { let res = do_work(&mut host, file_id, |analysis| { - analysis.diagnostics(file_id, true).unwrap(); + analysis.diagnostics(file_id, true, None).unwrap(); analysis.highlight_as_html(file_id, false).unwrap() }); if verbosity.is_verbose() { diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs index 3371c4fd30..31eb7ff3f8 100644 --- a/crates/rust-analyzer/src/cli/diagnostics.rs +++ b/crates/rust-analyzer/src/cli/diagnostics.rs @@ -47,7 +47,7 @@ pub fn diagnostics( String::from("unknown") }; println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id)); - for diagnostic in analysis.diagnostics(file_id, true).unwrap() { + for diagnostic in analysis.diagnostics(file_id, true, None).unwrap() { if matches!(diagnostic.severity, Severity::Error) { found_error = true; } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 9498244796..4e3ab05b2d 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -363,6 +363,14 @@ impl Config { self.client_caps.status_notification = get_bool("statusNotification"); } } + + pub fn disabled_diagnostics(&self) -> Option> { + if self.analysis.disabled_diagnostics.is_empty() { + None + } else { + Some(self.analysis.disabled_diagnostics.clone()) + } + } } #[derive(Deserialize)] diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 8fa31f59c7..212f98a300 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -108,7 +108,7 @@ impl GlobalState { Handle { handle, receiver } }; - let analysis_host = AnalysisHost::with_config(config.lru_capacity, config.analysis.clone()); + let analysis_host = AnalysisHost::new(config.lru_capacity); let (flycheck_sender, flycheck_receiver) = unbounded(); GlobalState { sender, diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 74f73655a4..067f5ff669 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -770,7 +770,11 @@ fn handle_fixes( None => {} }; - let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?; + let diagnostics = snap.analysis.diagnostics( + file_id, + snap.config.experimental_diagnostics, + snap.config.disabled_diagnostics(), + )?; for fix in diagnostics .into_iter() @@ -1044,7 +1048,11 @@ pub(crate) fn publish_diagnostics( let line_index = snap.analysis.file_line_index(file_id)?; let diagnostics: Vec = snap .analysis - .diagnostics(file_id, snap.config.experimental_diagnostics)? + .diagnostics( + file_id, + snap.config.experimental_diagnostics, + snap.config.disabled_diagnostics(), + )? .into_iter() .map(|d| Diagnostic { range: to_proto::range(&line_index, d.range), From 34847c8d96ddf98c40117ebacaecec38b9b758fc Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Tue, 18 Aug 2020 13:35:36 +0300 Subject: [PATCH 36/38] Move analysis config structure to the config.rs --- crates/ide/src/lib.rs | 6 ------ crates/rust-analyzer/src/config.rs | 8 +++++++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index a19a379c65..4b797f374c 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -99,12 +99,6 @@ pub use text_edit::{Indel, TextEdit}; pub type Cancelable = Result; -/// Configuration parameters for the analysis run. -#[derive(Debug, Default, Clone)] -pub struct AnalysisConfig { - pub disabled_diagnostics: HashSet, -} - #[derive(Debug)] pub struct Diagnostic { pub name: Option, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 4e3ab05b2d..44fd7c286f 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -10,7 +10,7 @@ use std::{collections::HashSet, ffi::OsString, path::PathBuf}; use flycheck::FlycheckConfig; -use ide::{AnalysisConfig, AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; +use ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; use lsp_types::ClientCapabilities; use project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest}; use serde::Deserialize; @@ -49,6 +49,12 @@ pub struct Config { pub analysis: AnalysisConfig, } +/// Configuration parameters for the analysis run. +#[derive(Debug, Default, Clone)] +pub struct AnalysisConfig { + pub disabled_diagnostics: HashSet, +} + #[derive(Debug, Clone, Eq, PartialEq)] pub enum LinkedProject { ProjectManifest(ProjectManifest), From 29e6238cb7330f7d29f33ff03a4ccc0a0cec9f4d Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Tue, 18 Aug 2020 20:39:55 +1000 Subject: [PATCH 37/38] SSR: A few small refactorings --- Cargo.lock | 1 + crates/ssr/Cargo.toml | 1 + crates/ssr/src/replacing.rs | 13 +++++++------ crates/ssr/src/tests.rs | 1 + 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2386c8f3a5..c1f1c07a2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1450,6 +1450,7 @@ dependencies = [ "expect", "hir", "ide_db", + "itertools", "rustc-hash", "syntax", "test_utils", diff --git a/crates/ssr/Cargo.toml b/crates/ssr/Cargo.toml index 56c1f77618..7c2090de3c 100644 --- a/crates/ssr/Cargo.toml +++ b/crates/ssr/Cargo.toml @@ -12,6 +12,7 @@ doctest = false [dependencies] rustc-hash = "1.1.0" +itertools = "0.9.0" text_edit = { path = "../text_edit" } syntax = { path = "../syntax" } diff --git a/crates/ssr/src/replacing.rs b/crates/ssr/src/replacing.rs index 21d0aa8a8c..29284e3f1c 100644 --- a/crates/ssr/src/replacing.rs +++ b/crates/ssr/src/replacing.rs @@ -1,9 +1,11 @@ //! Code for applying replacement templates for matches that have previously been found. use crate::{resolving::ResolvedRule, Match, SsrMatches}; +use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; use syntax::ast::{self, AstToken}; use syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize}; +use test_utils::mark; use text_edit::TextEdit; /// Returns a text edit that will replace each match in `matches` with its corresponding replacement @@ -127,6 +129,7 @@ impl ReplacementRenderer<'_> { && (placeholder_value.autoderef_count > 0 || placeholder_value.autoref_kind != ast::SelfParamKind::Owned) { + mark::hit!(replace_autoref_autoderef_capture); let ref_kind = match placeholder_value.autoref_kind { ast::SelfParamKind::Owned => "", ast::SelfParamKind::Ref => "&", @@ -206,18 +209,16 @@ fn token_is_method_call_receiver(token: &SyntaxToken) -> bool { use syntax::ast::AstNode; // Find the first method call among the ancestors of `token`, then check if the only token // within the receiver is `token`. - if let Some(receiver) = token - .ancestors() - .find(|node| node.kind() == SyntaxKind::METHOD_CALL_EXPR) - .and_then(|node| ast::MethodCallExpr::cast(node).unwrap().expr()) + if let Some(receiver) = + token.ancestors().find_map(ast::MethodCallExpr::cast).and_then(|call| call.expr()) { - let mut tokens = receiver.syntax().descendants_with_tokens().filter_map(|node_or_token| { + let tokens = receiver.syntax().descendants_with_tokens().filter_map(|node_or_token| { match node_or_token { SyntaxElement::Token(t) => Some(t), _ => None, } }); - if let (Some(only_token), None) = (tokens.next(), tokens.next()) { + if let Some((only_token,)) = tokens.collect_tuple() { return only_token == *token; } } diff --git a/crates/ssr/src/tests.rs b/crates/ssr/src/tests.rs index 0ad3512bad..e45c88864d 100644 --- a/crates/ssr/src/tests.rs +++ b/crates/ssr/src/tests.rs @@ -1179,6 +1179,7 @@ fn replace_autoref_autoderef_capture() { // second, we already have a reference, so it isn't. When $a is used in a context where autoref // doesn't apply, we need to prefix it with `&`. Finally, we have some cases where autoderef // needs to be applied. + mark::check!(replace_autoref_autoderef_capture); let code = r#" struct Foo {} impl Foo { From 9389e313d0f5f16e301acaf38fee1c00c76c5cce Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 13:20:17 +0200 Subject: [PATCH 38/38] Minor --- crates/stdx/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stdx/src/lib.rs b/crates/stdx/src/lib.rs index 3c5027fe57..265d192883 100644 --- a/crates/stdx/src/lib.rs +++ b/crates/stdx/src/lib.rs @@ -17,7 +17,7 @@ pub fn timeit(label: &'static str) -> impl Drop { impl Drop for Guard { fn drop(&mut self) { - eprintln!("{}: {:?}", self.label, self.start.elapsed()) + eprintln!("{}: {:.2?}", self.label, self.start.elapsed()) } }