diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2deb009ceff..fb077e28d58 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 @@ -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 diff --git a/Cargo.lock b/Cargo.lock index 2386c8f3a5d..ffa3851068c 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", @@ -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", @@ -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", ] @@ -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" @@ -1259,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", ] @@ -1450,6 +1455,7 @@ dependencies = [ "expect", "hir", "ide_db", + "itertools", "rustc-hash", "syntax", "test_utils", @@ -1573,9 +1579,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/crates/assists/src/ast_transform.rs b/crates/assists/src/ast_transform.rs index 4c41c16d86c..5216862ba52 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/assists/src/handlers/add_missing_impl_members.rs b/crates/assists/src/handlers/add_missing_impl_members.rs index 81b61ebf8e4..83a2ada9a28 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/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs index cce789972e5..b9ec3f10b65 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 f690ec343bc..81d0af2f353 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 4bcdae7ba0e..d62e06b4a81 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 011bf1106d1..470e5f8ff75 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/lib.rs b/crates/assists/src/lib.rs index ae90d68a35f..c589b08dc4b 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/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index d16e6fb0a63..17356700373 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!()} } - } "#####, ) diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs index 50a62ee8296..49096a67c7a 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 5dc3ae3b19e..c442654dd0f 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 4ae2bd0855b..8961ba8fd6c 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 3953017c3be..c693176fa48 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 8750584f946..1d13c4f1d3b 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, diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index 2e38a978f82..c7723de0067 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -15,6 +15,9 @@ pub struct UnresolvedModule { } impl Diagnostic for UnresolvedModule { + fn name(&self) -> &'static str { + "unresolved-module" + } fn message(&self) -> String { "unresolved module".to_string() } diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs index 74d26f08b37..99395667def 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 { diff --git a/crates/hir_expand/src/diagnostics.rs b/crates/hir_expand/src/diagnostics.rs index 59d35debe33..6c81b2501a5 100644 --- a/crates/hir_expand/src/diagnostics.rs +++ b/crates/hir_expand/src/diagnostics.rs @@ -21,6 +21,7 @@ use syntax::SyntaxNodePtr; use crate::InFile; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { + fn name(&self) -> &'static str; fn message(&self) -> String; /// Used in highlighting and related purposes fn display_source(&self) -> InFile; diff --git a/crates/hir_ty/Cargo.toml b/crates/hir_ty/Cargo.toml index 83b5013a902..a319b0ce8e3 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/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index ae0cf8d09b9..38fa24ee0a2 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs @@ -32,6 +32,10 @@ pub struct NoSuchField { } impl Diagnostic for NoSuchField { + fn name(&self) -> &'static str { + "no-such-field" + } + fn message(&self) -> String { "no such field".to_string() } @@ -54,6 +58,9 @@ pub struct MissingFields { } impl Diagnostic for MissingFields { + fn name(&self) -> &'static str { + "missing-structure-fields" + } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); for field in &self.missed_fields { @@ -87,6 +94,9 @@ pub struct MissingPatFields { } impl Diagnostic for MissingPatFields { + fn name(&self) -> &'static str { + "missing-pat-fields" + } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); for field in &self.missed_fields { @@ -117,6 +127,9 @@ pub struct MissingMatchArms { } impl Diagnostic for MissingMatchArms { + fn name(&self) -> &'static str { + "missing-match-arm" + } fn message(&self) -> String { String::from("Missing match arm") } @@ -135,6 +148,9 @@ pub struct MissingOkInTailExpr { } impl Diagnostic for MissingOkInTailExpr { + fn name(&self) -> &'static str { + "missing-ok-in-tail-expr" + } fn message(&self) -> String { "wrap return expression in Ok".to_string() } @@ -153,6 +169,9 @@ pub struct BreakOutsideOfLoop { } impl Diagnostic for BreakOutsideOfLoop { + fn name(&self) -> &'static str { + "break-outside-of-loop" + } fn message(&self) -> String { "break outside of loop".to_string() } @@ -171,6 +190,9 @@ pub struct MissingUnsafe { } impl Diagnostic for MissingUnsafe { + fn name(&self) -> &'static str { + "missing-unsafe" + } fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") } @@ -191,6 +213,9 @@ pub struct MismatchedArgCount { } impl Diagnostic for MismatchedArgCount { + fn name(&self) -> &'static str { + "mismatched-arg-count" + } fn message(&self) -> String { let s = if self.expected == 1 { "" } else { "s" }; format!("Expected {} argument{}, found {}", self.expected, s, self.found) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index fb76e2e4ecf..278a4b94726 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 7f007f1d652..5bd03f2ac00 100644 --- a/crates/hir_ty/src/diagnostics/match_check.rs +++ b/crates/hir_ty/src/diagnostics/match_check.rs @@ -1335,6 +1335,23 @@ fn panic(a: Category, b: Category) { ); } + #[test] + fn unknown_type() { + check_diagnostics( + r#" +enum Option { Some(T), None } + +fn main() { + // `Never` is deliberately not defined so that it's an uninferred type. + match Option::::None { + None => (), + Some(never) => match never {}, + } +} +"#, + ); + } + 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 diff --git a/crates/hir_ty/src/traits.rs b/crates/hir_ty/src/traits.rs index 1c3abb18f04..14cd3a2b446 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) } }; diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index a3ec98178a1..606a6064b42 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}; @@ -31,6 +31,7 @@ pub(crate) fn diagnostics( db: &RootDatabase, file_id: FileId, enable_experimental: bool, + disabled_diagnostics: Option>, ) -> Vec { let _p = profile::span("diagnostics"); let sema = Semantics::new(db); @@ -39,6 +40,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, @@ -50,7 +52,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| { res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) @@ -64,10 +66,19 @@ pub(crate) fn diagnostics( res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) // Only collect experimental diagnostics when they're enabled. - .filter(|diag| !diag.is_experimental() || enable_experimental) + .filter(|diag| !diag.is_experimental() || enable_experimental); + + if let Some(disabled_diagnostics) = disabled_diagnostics { + // Do not collect disabled diagnostics. + sink_builder = sink_builder.filter(move |diag| !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().into()), message: d.message(), range: sema.diagnostics_display_range(d).range, severity: Severity::Error, @@ -84,6 +95,7 @@ pub(crate) fn diagnostics( fn diagnostic_with_fix(d: &D, sema: &Semantics) -> Diagnostic { Diagnostic { + name: Some(d.name().into()), range: sema.diagnostics_display_range(d).range, message: d.message(), severity: Severity::Error, @@ -110,6 +122,7 @@ fn check_unnecessary_braces_in_use_statement( }); acc.push(Diagnostic { + name: None, range: use_range, message: "Unnecessary braces in use statement".to_string(), severity: Severity::WeakWarning, @@ -156,6 +169,7 @@ fn check_struct_shorthand_initialization( let field_range = record_field.syntax().text_range(); acc.push(Diagnostic { + name: None, range: field_range, message: "Shorthand struct initialization".to_string(), severity: Severity::WeakWarning, @@ -173,6 +187,7 @@ fn check_struct_shorthand_initialization( #[cfg(test)] mod tests { + use std::collections::HashSet; use stdx::trim_indent; use test_utils::assert_eq_text; @@ -188,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(); @@ -214,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; @@ -235,14 +251,58 @@ 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); } + /// 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: &[&'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::>(); + let analysis = mock.analysis(); + + let diagnostics = files + .clone() + .into_iter() + .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. + 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. + let diagnostics = files + .into_iter() + .flat_map(|file_id| analysis.diagnostics(file_id, true, None).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(); + let diagnostics = analysis.diagnostics(file_id, true, None).unwrap(); expect.assert_debug_eq(&diagnostics) } @@ -502,6 +562,9 @@ fn test_fn() { expect![[r#" [ Diagnostic { + name: Some( + "unresolved-module", + ), message: "unresolved module", range: 0..8, severity: Error, @@ -675,4 +738,9 @@ struct Foo { ", ) } + + #[test] + fn test_disabled_diagnostics() { + check_disabled_diagnostics(r#"mod foo;"#, &["unresolved-module"]); + } } diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 002adf9159f..596bc872dba 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 diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index eb638952970..4b797f374c0 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -44,7 +44,7 @@ mod syntax_highlighting; mod syntax_tree; mod typing; -use std::sync::Arc; +use std::{collections::HashSet, sync::Arc}; use base_db::{ salsa::{self, ParallelDatabase}, @@ -101,6 +101,7 @@ pub type Cancelable = Result; #[derive(Debug)] pub struct Diagnostic { + pub name: Option, pub message: String, pub range: TextRange, pub severity: Severity, @@ -147,7 +148,7 @@ pub struct AnalysisHost { } impl AnalysisHost { - pub fn new(lru_capacity: Option) -> AnalysisHost { + pub fn new(lru_capacity: Option) -> Self { AnalysisHost { db: RootDatabase::new(lru_capacity) } } @@ -496,8 +497,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.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 diff --git a/crates/proc_macro_api/src/lib.rs b/crates/proc_macro_api/src/lib.rs index 15db57eb28b..d5e87cf7d59 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() diff --git a/crates/proc_macro_srv/Cargo.toml b/crates/proc_macro_srv/Cargo.toml index 7171f08084e..a468b556098 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 8e6f28abdf5..1a827cbd76c 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 5828512d6e9..36942147d9e 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 00000000000..7b0f64f318c --- /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 00000000000..ec2a114a35d --- /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() +} diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs index 0f614f9e0c5..43f0196afc6 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 3371c4fd301..31eb7ff3f88 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 33fb5e9c225..44fd7c286fd 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -7,7 +7,7 @@ //! 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 ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; @@ -45,6 +45,14 @@ pub struct Config { pub with_sysroot: bool, pub linked_projects: Vec, pub root_path: AbsPathBuf, + + 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)] @@ -176,6 +184,8 @@ impl Config { hover: HoverConfig::default(), linked_projects: Vec::new(), root_path, + + analysis: AnalysisConfig::default(), } } @@ -293,6 +303,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); } @@ -357,6 +369,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)] @@ -444,5 +464,7 @@ config_data! { rustfmt_overrideCommand: Option> = None, withSysroot: bool = true, + + analysis_disabledDiagnostics: HashSet = HashSet::new(), } } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 74f73655a4e..4f77b1b4d26 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -272,19 +272,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 { @@ -770,7 +775,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() @@ -859,10 +868,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) } @@ -1044,7 +1053,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), diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 3976b6529ee..e1a28b1b4ba 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 a2cfb4e0d5c..505505a7796 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -13,6 +13,7 @@ use crate::{ lsp_ext, main_loop::Task, }; +use lsp_ext::StatusParams; impl GlobalState { pub(crate) fn update_configuration(&mut self, config: Config) { @@ -85,7 +86,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/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 8a2cfa2aee1..535de2f71ab 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/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 7370505f8bf..fa315ff8eef 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/crates/ssr/Cargo.toml b/crates/ssr/Cargo.toml index 56c1f776187..7c2090de3cc 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/lib.rs b/crates/ssr/src/lib.rs index 292bd5b9a73..ba669fd56c9 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 ffc7202ae56..8bb5ced9007 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 { @@ -69,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)] @@ -173,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)?; } @@ -183,8 +183,8 @@ impl<'db, 'sema> Matcher<'db, 'sema> { // 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), + placeholder.ident.clone(), + PlaceholderMatch::new(Some(code), original_range), ); } return Ok(()); @@ -487,7 +487,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 @@ -536,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 { @@ -575,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> { @@ -676,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/parsing.rs b/crates/ssr/src/parsing.rs index 9570e96e36e..05b66dcd780 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 8f8fe6149a5..29284e3f1c2 100644 --- a/crates/ssr/src/replacing.rs +++ b/crates/ssr/src/replacing.rs @@ -1,10 +1,11 @@ //! Code for applying replacement templates for matches that have previously been found. -use crate::matching::Var; 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 @@ -114,11 +115,33 @@ 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 = 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) + { + mark::hit!(replace_autoref_autoderef_capture); + 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, @@ -179,6 +202,29 @@ 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_map(ast::MethodCallExpr::cast).and_then(|call| call.expr()) + { + 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,)) = tokens.collect_tuple() { + 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 0d0a0009064..e45c88864d7 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" ); } @@ -1172,3 +1172,110 @@ 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. + mark::check!(replace_autoref_autoderef_capture); + 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); + } + "#]], + ); +} diff --git a/crates/stdx/src/lib.rs b/crates/stdx/src/lib.rs index 3c5027fe570..265d192883e 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()) } } diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml index 47e351f9d1a..ec3132da8d7 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" diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index 190746e09e6..060b209663e 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs @@ -91,29 +91,52 @@ 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 { diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index 1be01fd8842..2e3133449fb 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/docs/dev/style.md b/docs/dev/style.md index 963a6d73d05..8effddcda55 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. diff --git a/editors/code/.eslintignore b/editors/code/.eslintignore new file mode 100644 index 00000000000..3df5c860bd5 --- /dev/null +++ b/editors/code/.eslintignore @@ -0,0 +1,3 @@ +node_modules +.eslintrc.js +rollup.config.js \ No newline at end of file diff --git a/editors/code/package.json b/editors/code/package.json index ee5f96bf324..429ff5def41 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -609,6 +609,15 @@ }, "description": "List of 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": [] } } }, diff --git a/editors/code/src/ctx.ts b/editors/code/src/ctx.ts index 6e767babf44..543f7e02e38 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 494d51c83a1..8663737a684 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"); diff --git a/xtask/src/codegen.rs b/xtask/src/codegen.rs index 4b2b614fae6..c468468de1b 100644 --- a/xtask/src/codegen.rs +++ b/xtask/src/codegen.rs @@ -16,7 +16,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}, @@ -71,6 +75,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 526941f73ac..4f49685944f 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 31bc3839d07..3f0013e82ab 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 dd1f4d6a2c4..df3ec22c8d4 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 807ef587ce3..e790d995fb3 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}; @@ -62,17 +63,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") { @@ -119,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 71caff2484c..c4a15f4bdfe 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<()> { @@ -101,7 +102,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 00000000000..47ba6ba246c --- /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)) +} diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index ca9749ed470..bec3c630b76 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())