From 1d74ef1d989b3d37f3f08d32e88670ee0f1f7ab6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 8 Jun 2021 16:50:10 +0200 Subject: Don't complete values in type position --- crates/hir/src/lib.rs | 12 ++++++ crates/hir/src/semantics.rs | 3 +- crates/ide_completion/src/completions.rs | 48 ++++++++++------------ crates/ide_completion/src/completions/attribute.rs | 4 +- crates/ide_completion/src/completions/flyimport.rs | 3 +- .../src/completions/unqualified_path.rs | 4 -- crates/ide_completion/src/context.rs | 42 +++++++++++++------ crates/ide_completion/src/render.rs | 15 +++---- 8 files changed, 76 insertions(+), 55 deletions(-) (limited to 'crates') diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index c2b68a853..589641760 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2496,6 +2496,18 @@ impl ScopeDef { items } + + pub fn is_value_def(&self) -> bool { + matches!( + self, + ScopeDef::ModuleDef(ModuleDef::Function(_)) + | ScopeDef::ModuleDef(ModuleDef::Variant(_)) + | ScopeDef::ModuleDef(ModuleDef::Const(_)) + | ScopeDef::ModuleDef(ModuleDef::Static(_)) + | ScopeDef::GenericParam(GenericParam::ConstParam(_)) + | ScopeDef::Local(_) + ) + } } impl From for ScopeDef { diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 2d08a7704..827e23e2b 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -35,8 +35,9 @@ pub enum PathResolution { Def(ModuleDef), /// A local binding (only value namespace) Local(Local), - /// A generic parameter + /// A type parameter TypeParam(TypeParam), + /// A const parameter ConstParam(ConstParam), SelfType(Impl), Macro(MacroDef), diff --git a/crates/ide_completion/src/completions.rs b/crates/ide_completion/src/completions.rs index 7a4d71e91..e07a4c403 100644 --- a/crates/ide_completion/src/completions.rs +++ b/crates/ide_completion/src/completions.rs @@ -56,10 +56,16 @@ impl Builder { } impl Completions { - pub(crate) fn add(&mut self, item: CompletionItem) { + fn add(&mut self, item: CompletionItem) { self.buf.push(item) } + fn add_opt(&mut self, item: Option) { + if let Some(item) = item { + self.buf.push(item) + } + } + pub(crate) fn add_all(&mut self, items: I) where I: IntoIterator, @@ -103,9 +109,10 @@ impl Completions { local_name: hir::Name, resolution: &hir::ScopeDef, ) { - if let Some(item) = render_resolution(RenderContext::new(ctx), local_name, resolution) { - self.add(item); + if ctx.expects_type() && resolution.is_value_def() { + return; } + self.add_opt(render_resolution(RenderContext::new(ctx), local_name, resolution)); } pub(crate) fn add_macro( @@ -118,9 +125,7 @@ impl Completions { Some(it) => it, None => return, }; - if let Some(item) = render_macro(RenderContext::new(ctx), None, name, macro_) { - self.add(item); - } + self.add_opt(render_macro(RenderContext::new(ctx), None, name, macro_)); } pub(crate) fn add_function( @@ -129,9 +134,10 @@ impl Completions { func: hir::Function, local_name: Option, ) { - if let Some(item) = render_fn(RenderContext::new(ctx), None, local_name, func) { - self.add(item) + if ctx.expects_type() { + return; } + self.add_opt(render_fn(RenderContext::new(ctx), None, local_name, func)); } pub(crate) fn add_method( @@ -141,10 +147,7 @@ impl Completions { receiver: Option, local_name: Option, ) { - if let Some(item) = render_method(RenderContext::new(ctx), None, receiver, local_name, func) - { - self.add(item) - } + self.add_opt(render_method(RenderContext::new(ctx), None, receiver, local_name, func)); } pub(crate) fn add_variant_pat( @@ -153,9 +156,7 @@ impl Completions { variant: hir::Variant, local_name: Option, ) { - if let Some(item) = render_variant_pat(RenderContext::new(ctx), variant, local_name, None) { - self.add(item); - } + self.add_opt(render_variant_pat(RenderContext::new(ctx), variant, local_name, None)); } pub(crate) fn add_qualified_variant_pat( @@ -164,9 +165,7 @@ impl Completions { variant: hir::Variant, path: hir::ModPath, ) { - if let Some(item) = render_variant_pat(RenderContext::new(ctx), variant, None, Some(path)) { - self.add(item); - } + self.add_opt(render_variant_pat(RenderContext::new(ctx), variant, None, Some(path))); } pub(crate) fn add_struct_pat( @@ -175,21 +174,18 @@ impl Completions { strukt: hir::Struct, local_name: Option, ) { - if let Some(item) = render_struct_pat(RenderContext::new(ctx), strukt, local_name) { - self.add(item); - } + self.add_opt(render_struct_pat(RenderContext::new(ctx), strukt, local_name)); } pub(crate) fn add_const(&mut self, ctx: &CompletionContext, constant: hir::Const) { - if let Some(item) = render_const(RenderContext::new(ctx), constant) { - self.add(item); + if ctx.expects_type() { + return; } + self.add_opt(render_const(RenderContext::new(ctx), constant)); } pub(crate) fn add_type_alias(&mut self, ctx: &CompletionContext, type_alias: hir::TypeAlias) { - if let Some(item) = render_type_alias(RenderContext::new(ctx), type_alias) { - self.add(item) - } + self.add_opt(render_type_alias(RenderContext::new(ctx), type_alias)); } pub(crate) fn add_qualified_enum_variant( diff --git a/crates/ide_completion/src/completions/attribute.rs b/crates/ide_completion/src/completions/attribute.rs index d3392100d..7f76e357e 100644 --- a/crates/ide_completion/src/completions/attribute.rs +++ b/crates/ide_completion/src/completions/attribute.rs @@ -69,7 +69,7 @@ fn complete_new_attribute(acc: &mut Completions, ctx: &CompletionContext, attrib } if is_inner || !attr_completion.prefer_inner { - acc.add(item.build()); + item.add_to(acc); } }; @@ -96,7 +96,7 @@ fn complete_new_attribute(acc: &mut Completions, ctx: &CompletionContext, attrib if let Some(docs) = mac.docs(ctx.sema.db) { item.documentation(docs); } - acc.add(item.build()); + item.add_to(acc); } } }); diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index 7bf47bf75..c010cbbca 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -90,7 +90,6 @@ //! Note that having this flag set to `true` does not guarantee that the feature is enabled: your client needs to have the corredponding //! capability enabled. -use hir::ModPath; use ide_db::helpers::{ import_assets::{ImportAssets, ImportCandidate}, insert_use::ImportScope, @@ -208,7 +207,7 @@ fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option usize { cov_mark::hit!(certain_fuzzy_order_test); diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs index 8b22933e0..f370dbdf0 100644 --- a/crates/ide_completion/src/completions/unqualified_path.rs +++ b/crates/ide_completion/src/completions/unqualified_path.rs @@ -339,7 +339,6 @@ fn x() -> $0 "#, expect![[r#" st Foo - fn x() fn() "#]], ); } @@ -391,7 +390,6 @@ pub mod prelude { } "#, expect![[r#" - fn foo() fn() md std st Option "#]], @@ -448,7 +446,6 @@ pub mod prelude { } "#, expect![[r#" - fn foo() fn() md std md core st String @@ -509,7 +506,6 @@ macro_rules! foo { () => {} } fn main() { let x: $0 } "#, expect![[r#" - fn main() fn() ma foo!(…) macro_rules! foo "#]], ); diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs index 6177caa12..2c2a4aa6b 100644 --- a/crates/ide_completion/src/context.rs +++ b/crates/ide_completion/src/context.rs @@ -29,6 +29,12 @@ pub(crate) enum PatternRefutability { Irrefutable, } +#[derive(Debug)] +pub(super) enum PathKind { + Expr, + Type, +} + #[derive(Debug)] pub(crate) struct PathCompletionContext { /// If this is a call with () already there @@ -36,13 +42,12 @@ pub(crate) struct PathCompletionContext { /// A single-indent path, like `foo`. `::foo` should not be considered a trivial path. pub(super) is_trivial_path: bool, /// If not a trivial path, the prefix (qualifier). - pub(super) path_qual: Option, - pub(super) is_path_type: bool, + pub(super) qualifier: Option, + pub(super) kind: Option, + /// Whether the path segment has type args or not. pub(super) has_type_args: bool, /// `true` if we are a statement or a last expr in the block. pub(super) can_be_stmt: bool, - /// `true` if we expect an expression at the cursor position. - pub(super) is_expr: bool, pub(super) in_loop_body: bool, } @@ -308,7 +313,11 @@ impl<'a> CompletionContext<'a> { } pub(crate) fn expects_expression(&self) -> bool { - self.path_context.as_ref().map_or(false, |it| it.is_expr) + matches!(self.path_context, Some(PathCompletionContext { kind: Some(PathKind::Expr), .. })) + } + + pub(crate) fn expects_type(&self) -> bool { + matches!(self.path_context, Some(PathCompletionContext { kind: Some(PathKind::Type), .. })) } pub(crate) fn path_call_kind(&self) -> Option { @@ -316,11 +325,11 @@ impl<'a> CompletionContext<'a> { } pub(crate) fn is_trivial_path(&self) -> bool { - self.path_context.as_ref().map_or(false, |it| it.is_trivial_path) + matches!(self.path_context, Some(PathCompletionContext { is_trivial_path: true, .. })) } pub(crate) fn path_qual(&self) -> Option<&ast::Path> { - self.path_context.as_ref().and_then(|it| it.path_qual.as_ref()) + self.path_context.as_ref().and_then(|it| it.qualifier.as_ref()) } fn fill_impl_def(&mut self) { @@ -573,12 +582,11 @@ impl<'a> CompletionContext<'a> { let path_ctx = self.path_context.get_or_insert(PathCompletionContext { call_kind: None, is_trivial_path: false, - path_qual: None, + qualifier: None, has_type_args: false, - is_path_type: false, can_be_stmt: false, - is_expr: false, in_loop_body: false, + kind: None, }); path_ctx.in_loop_body = is_in_loop_body(name_ref.syntax()); let path = segment.parent_path(); @@ -593,11 +601,20 @@ impl<'a> CompletionContext<'a> { } }; } - path_ctx.is_path_type = path.syntax().parent().and_then(ast::PathType::cast).is_some(); + + if let Some(parent) = path.syntax().parent() { + path_ctx.kind = match_ast! { + match parent { + ast::PathType(_it) => Some(PathKind::Type), + ast::PathExpr(_it) => Some(PathKind::Expr), + _ => None, + } + }; + } path_ctx.has_type_args = segment.generic_arg_list().is_some(); if let Some(path) = path_or_use_tree_qualifier(&path) { - path_ctx.path_qual = path + path_ctx.qualifier = path .segment() .and_then(|it| { find_node_with_range::( @@ -635,7 +652,6 @@ impl<'a> CompletionContext<'a> { None }) .unwrap_or(false); - path_ctx.is_expr = path.syntax().parent().and_then(ast::PathExpr::cast).is_some(); } } } diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 750694432..7118183fe 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -18,6 +18,7 @@ use ide_db::{ use syntax::TextRange; use crate::{ + context::{PathCompletionContext, PathKind}, item::{CompletionRelevanceTypeMatch, ImportEdit}, render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, @@ -54,6 +55,9 @@ pub(crate) fn render_resolution_with_import<'a>( import_edit: ImportEdit, ) -> Option { let resolution = hir::ScopeDef::from(import_edit.import.original_item); + if ctx.completion.expects_type() && resolution.is_value_def() { + return None; + } let local_name = match resolution { hir::ScopeDef::ModuleDef(hir::ModuleDef::Function(f)) => f.name(ctx.completion.db), hir::ScopeDef::ModuleDef(hir::ModuleDef::Const(c)) => c.name(ctx.completion.db)?, @@ -275,13 +279,10 @@ impl<'a> Render<'a> { }; // Add `<>` for generic types - if self - .ctx - .completion - .path_context - .as_ref() - .map_or(false, |it| it.is_path_type && !it.has_type_args) - && self.ctx.completion.config.add_call_parenthesis + if matches!( + self.ctx.completion.path_context, + Some(PathCompletionContext { kind: Some(PathKind::Type), has_type_args: false, .. }) + ) && self.ctx.completion.config.add_call_parenthesis { if let Some(cap) = self.ctx.snippet_cap() { let has_non_default_type_params = match resolution { -- cgit v1.2.3 From ee374ff1ee7ad1af6bc0624f98b2028cef6ed4a8 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 8 Jun 2021 17:31:47 +0200 Subject: fix: don't complete derive macros as fn-like macros --- crates/hir/src/lib.rs | 7 +++ crates/ide_completion/src/completions/pattern.rs | 24 ++++++++++- .../src/completions/qualified_path.rs | 11 ++++- .../src/completions/unqualified_path.rs | 50 +++++++++++++++++++++- 4 files changed, 88 insertions(+), 4 deletions(-) (limited to 'crates') diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index c2b68a853..b5209546c 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1351,6 +1351,13 @@ impl MacroDef { MacroDefKind::ProcMacro(_, base_db::ProcMacroKind::FuncLike, _) => MacroKind::ProcMacro, } } + + pub fn is_fn_like(&self) -> bool { + match self.kind() { + MacroKind::Declarative | MacroKind::BuiltIn | MacroKind::ProcMacro => true, + MacroKind::Attr | MacroKind::Derive => false, + } + } } /// Invariant: `inner.as_assoc_item(db).is_some()` diff --git a/crates/ide_completion/src/completions/pattern.rs b/crates/ide_completion/src/completions/pattern.rs index 8a728c67e..1daa8595a 100644 --- a/crates/ide_completion/src/completions/pattern.rs +++ b/crates/ide_completion/src/completions/pattern.rs @@ -39,7 +39,7 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) { | hir::ModuleDef::Module(..) => refutable, _ => false, }, - hir::ScopeDef::MacroDef(_) => true, + hir::ScopeDef::MacroDef(mac) => mac.is_fn_like(), hir::ScopeDef::ImplSelfType(impl_) => match impl_.self_ty(ctx.db).as_adt() { Some(hir::Adt::Struct(strukt)) => { acc.add_struct_pat(ctx, strukt, Some(name.clone())); @@ -101,6 +101,28 @@ fn foo() { ); } + #[test] + fn does_not_complete_non_fn_macros() { + check( + r#" +macro_rules! m { ($e:expr) => { $e } } +enum E { X } + +#[rustc_builtin_macro] +macro Clone {} + +fn foo() { + match E::X { $0 } +} +"#, + expect![[r#" + ev E::X () + en E + ma m!(…) macro_rules! m + "#]], + ); + } + #[test] fn completes_in_simple_macro_call() { check( diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs index d58745fb4..4dfdc5ced 100644 --- a/crates/ide_completion/src/completions/qualified_path.rs +++ b/crates/ide_completion/src/completions/qualified_path.rs @@ -26,7 +26,9 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon let module_scope = module.scope(ctx.db, context_module); for (name, def) in module_scope { if let hir::ScopeDef::MacroDef(macro_def) = def { - acc.add_macro(ctx, Some(name.clone()), macro_def); + if macro_def.is_fn_like() { + acc.add_macro(ctx, Some(name.clone()), macro_def); + } } if let hir::ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = def { acc.add_resolution(ctx, name, &def); @@ -58,6 +60,13 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon } } + if let hir::ScopeDef::MacroDef(macro_def) = def { + if !macro_def.is_fn_like() { + // Don't suggest attribute macros and derives. + continue; + } + } + acc.add_resolution(ctx, name, &def); } } diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs index 8b22933e0..52f40d496 100644 --- a/crates/ide_completion/src/completions/unqualified_path.rs +++ b/crates/ide_completion/src/completions/unqualified_path.rs @@ -13,7 +13,9 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC // only show macros in {Assoc}ItemList ctx.scope.process_all_names(&mut |name, res| { if let hir::ScopeDef::MacroDef(mac) = res { - acc.add_macro(ctx, Some(name.clone()), mac); + if mac.is_fn_like() { + acc.add_macro(ctx, Some(name.clone()), mac); + } } if let hir::ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = res { acc.add_resolution(ctx, name, &res); @@ -46,7 +48,13 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC cov_mark::hit!(skip_lifetime_completion); return; } - acc.add_resolution(ctx, name, &res); + let add_resolution = match res { + ScopeDef::MacroDef(mac) => mac.is_fn_like(), + _ => true, + }; + if add_resolution { + acc.add_resolution(ctx, name, &res); + } }); } @@ -426,6 +434,44 @@ mod macros { ); } + #[test] + fn does_not_complete_non_fn_macros() { + check( + r#" +#[rustc_builtin_macro] +pub macro Clone {} + +fn f() {$0} +"#, + expect![[r#" + fn f() fn() + "#]], + ); + check( + r#" +#[rustc_builtin_macro] +pub macro Clone {} + +struct S; +impl S { + $0 +} +"#, + expect![[r#""#]], + ); + check( + r#" +mod m { + #[rustc_builtin_macro] + pub macro Clone {} +} + +fn f() {m::$0} +"#, + expect![[r#""#]], + ); + } + #[test] fn completes_std_prelude_if_core_is_defined() { check( -- cgit v1.2.3 From 1a26af15ef190bf605d3a04005aea02664815fb0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 8 Jun 2021 20:27:25 +0200 Subject: Add tests checking no value completion in type pos --- crates/ide_completion/src/completions.rs | 3 +++ .../src/completions/qualified_path.rs | 30 ++++++++++++++++++++++ .../src/completions/unqualified_path.rs | 22 ++++++++++++++++ 3 files changed, 55 insertions(+) (limited to 'crates') diff --git a/crates/ide_completion/src/completions.rs b/crates/ide_completion/src/completions.rs index e07a4c403..fbd499900 100644 --- a/crates/ide_completion/src/completions.rs +++ b/crates/ide_completion/src/completions.rs @@ -204,6 +204,9 @@ impl Completions { variant: hir::Variant, local_name: Option, ) { + if ctx.expects_type() { + return; + } let item = render_variant(RenderContext::new(ctx), None, local_name, variant, None); self.add(item); } diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs index d58745fb4..a5cba5938 100644 --- a/crates/ide_completion/src/completions/qualified_path.rs +++ b/crates/ide_completion/src/completions/qualified_path.rs @@ -198,6 +198,36 @@ mod tests { check(r#"use self::foo$0;"#, expect![[""]]); } + #[test] + fn dont_complete_values_in_type_pos() { + check( + r#" +const FOO: () = (); +static BAR: () = (); +struct Baz; +fn foo() { + let _: self::$0; +} +"#, + expect![[r#" + st Baz + "#]], + ); + } + + #[test] + fn dont_complete_enum_variants_in_type_pos() { + check( + r#" +enum Foo { Bar } +fn foo() { + let _: Foo::$0; +} +"#, + expect![[r#""#]], + ); + } + #[test] fn dont_complete_current_use_in_braces_with_glob() { check( diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs index f370dbdf0..2105bb428 100644 --- a/crates/ide_completion/src/completions/unqualified_path.rs +++ b/crates/ide_completion/src/completions/unqualified_path.rs @@ -68,6 +68,28 @@ mod tests { expect.assert_eq(&actual) } + #[test] + fn dont_complete_values_in_type_pos() { + check( + r#" +const FOO: () = (); +static BAR: () = (); +enum Foo { + Bar +} +struct Baz; +fn foo() { + let local = (); + let _: $0; +} +"#, + expect![[r#" + en Foo + st Baz + "#]], + ); + } + #[test] fn only_completes_modules_in_import() { cov_mark::check!(only_completes_modules_in_import); -- cgit v1.2.3