Skip to content

Commit

Permalink
Auto merge of rust-lang#13789 - feniljain:fix_enum_completion, r=Veykril
Browse files Browse the repository at this point in the history
feat: show only missing variant suggestion for enums in patterns completion and bump them in list too

Fixes rust-lang#12438

### Points to help in review:

- This PR can be reviewed commit wise, first commit is about bumping enum variant completions up in the list of completions and second commit is about only showing enum variants which are not complete
- I am calculating missing variants in analysis.rs by firstly locating the enum and then comparing each of it's variant's name and checking if arm string already contains that name, this is kinda hacky but I didn't want to implement complete missing_arms assist here as that would have been too bulky to run on each completion cycle ( if we can improve this somehow would appreciate some inputs on it )

### Output:

https://user-images.githubusercontent.com/49019259/208245540-57d7321b-b275-477e-bef0-b3a1ff8b7040.mov

Relevant Zulip Discussion: https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Frust-analyzer/topic/Issue.20.2312438
  • Loading branch information
bors committed Mar 18, 2023
2 parents 924d277 + a79a76a commit 7c05f55
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 13 deletions.
19 changes: 11 additions & 8 deletions crates/ide-completion/src/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) mod env_vars;

use std::iter;

use hir::{known, ScopeDef};
use hir::{known, ScopeDef, Variant};
use ide_db::{imports::import_assets::LocatedImport, SymbolKind};
use syntax::ast;

Expand Down Expand Up @@ -537,17 +537,20 @@ fn enum_variants_with_paths(
impl_: &Option<ast::Impl>,
cb: impl Fn(&mut Completions, &CompletionContext<'_>, hir::Variant, hir::ModPath),
) {
let mut process_variant = |variant: Variant| {
let self_path = hir::ModPath::from_segments(
hir::PathKind::Plain,
iter::once(known::SELF_TYPE).chain(iter::once(variant.name(ctx.db))),
);

cb(acc, ctx, variant, self_path);
};

let variants = enum_.variants(ctx.db);

if let Some(impl_) = impl_.as_ref().and_then(|impl_| ctx.sema.to_def(impl_)) {
if impl_.self_ty(ctx.db).as_adt() == Some(hir::Adt::Enum(enum_)) {
for &variant in &variants {
let self_path = hir::ModPath::from_segments(
hir::PathKind::Plain,
iter::once(known::SELF_TYPE).chain(iter::once(variant.name(ctx.db))),
);
cb(acc, ctx, variant, self_path);
}
variants.iter().for_each(|variant| process_variant(*variant));
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/ide-completion/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ pub(super) struct PatternContext {
/// The record pattern this name or ref is a field of
pub(super) record_pat: Option<ast::RecordPat>,
pub(super) impl_: Option<ast::Impl>,
/// List of missing variants in a match expr
pub(super) missing_variants: Vec<hir::Variant>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down
53 changes: 51 additions & 2 deletions crates/ide-completion/src/context/analysis.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Module responsible for analyzing the code surrounding the cursor for completion.
use std::iter;

use hir::{Semantics, Type, TypeInfo};
use hir::{Semantics, Type, TypeInfo, Variant};
use ide_db::{active_parameter::ActiveParameter, RootDatabase};
use syntax::{
algo::{find_node_at_offset, non_trivia_sibling},
Expand Down Expand Up @@ -1132,6 +1132,9 @@ fn pattern_context_for(
pat: ast::Pat,
) -> PatternContext {
let mut param_ctx = None;

let mut missing_variants = vec![];

let (refutability, has_type_ascription) =
pat
.syntax()
Expand Down Expand Up @@ -1161,7 +1164,52 @@ fn pattern_context_for(
})();
return (PatternRefutability::Irrefutable, has_type_ascription)
},
ast::MatchArm(_) => PatternRefutability::Refutable,
ast::MatchArm(match_arm) => {
let missing_variants_opt = match_arm
.syntax()
.parent()
.and_then(ast::MatchArmList::cast)
.and_then(|match_arm_list| {
match_arm_list
.syntax()
.parent()
.and_then(ast::MatchExpr::cast)
.and_then(|match_expr| {
let expr_opt = find_opt_node_in_file(&original_file, match_expr.expr());

expr_opt.and_then(|expr| {
sema.type_of_expr(&expr)?
.adjusted()
.autoderef(sema.db)
.find_map(|ty| match ty.as_adt() {
Some(hir::Adt::Enum(e)) => Some(e),
_ => None,
}).and_then(|enum_| {
Some(enum_.variants(sema.db))
})
})
}).and_then(|variants| {
Some(variants.iter().filter_map(|variant| {
let variant_name = variant.name(sema.db).to_string();

let variant_already_present = match_arm_list.arms().any(|arm| {
arm.pat().and_then(|pat| {
let pat_already_present = pat.syntax().to_string().contains(&variant_name);
pat_already_present.then(|| pat_already_present)
}).is_some()
});

(!variant_already_present).then_some(variant.clone())
}).collect::<Vec<Variant>>())
})
});

if let Some(missing_variants_) = missing_variants_opt {
missing_variants = missing_variants_;
};

PatternRefutability::Refutable
},
ast::LetExpr(_) => PatternRefutability::Refutable,
ast::ForExpr(_) => PatternRefutability::Irrefutable,
_ => PatternRefutability::Irrefutable,
Expand All @@ -1183,6 +1231,7 @@ fn pattern_context_for(
ref_token,
record_pat: None,
impl_: fetch_immediate_impl(sema, original_file, pat.syntax()),
missing_variants,
}
}

Expand Down
26 changes: 23 additions & 3 deletions crates/ide-completion/src/render/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ pub(crate) fn render_struct_pat(
let lookup = format_literal_lookup(name.as_str(), kind);
let pat = render_pat(&ctx, pattern_ctx, &escaped_name, kind, &visible_fields, fields_omitted)?;

Some(build_completion(ctx, label, lookup, pat, strukt))
let db = ctx.db();

Some(build_completion(ctx, label, lookup, pat, strukt, strukt.ty(db), false))
}

pub(crate) fn render_variant_pat(
Expand All @@ -52,6 +54,7 @@ pub(crate) fn render_variant_pat(

let fields = variant.fields(ctx.db());
let (visible_fields, fields_omitted) = visible_fields(ctx.completion, &fields, variant)?;
let enum_ty = variant.parent_enum(ctx.db()).ty(ctx.db());

let (name, escaped_name) = match path {
Some(path) => (path.unescaped().to_string().into(), path.to_string().into()),
Expand Down Expand Up @@ -81,7 +84,15 @@ pub(crate) fn render_variant_pat(
}
};

Some(build_completion(ctx, label, lookup, pat, variant))
Some(build_completion(
ctx,
label,
lookup,
pat,
variant,
enum_ty,
pattern_ctx.missing_variants.contains(&variant),
))
}

fn build_completion(
Expand All @@ -90,13 +101,22 @@ fn build_completion(
lookup: SmolStr,
pat: String,
def: impl HasAttrs + Copy,
adt_ty: hir::Type,
// Missing in context of match statement completions
is_variant_missing: bool,
) -> CompletionItem {
let mut relevance = ctx.completion_relevance();

if is_variant_missing {
relevance.type_match = super::compute_type_match(ctx.completion, &adt_ty);
}

let mut item = CompletionItem::new(CompletionItemKind::Binding, ctx.source_range(), label);
item.set_documentation(ctx.docs(def))
.set_deprecated(ctx.is_deprecated(def))
.detail(&pat)
.lookup_by(lookup)
.set_relevance(ctx.completion_relevance());
.set_relevance(relevance);
match ctx.snippet_cap() {
Some(snippet_cap) => item.insert_snippet(snippet_cap, pat),
None => item.insert_text(pat),
Expand Down
60 changes: 60 additions & 0 deletions crates/ide-completion/src/tests/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,66 @@ fn foo(s: Struct) {
);
}

#[test]
fn record_pattern_field_enum() {
check(
r#"
//- minicore:result
enum Baz { Foo, Bar }
fn foo(baz: Baz) {
match baz {
Baz::Foo => (),
$0
}
}
"#,
expect![[r#"
en Baz
en Result
md core
ev Err
ev Ok
bn Baz::Bar Baz::Bar$0
bn Baz::Foo Baz::Foo$0
bn Err(…) Err($1)$0
bn Ok(…) Ok($1)$0
kw mut
kw ref
"#]],
);

check(
r#"
//- minicore:result
enum Baz { Foo, Bar }
fn foo(baz: Baz) {
use Baz::*;
match baz {
Foo => (),
$0
}
}
"#,
expect![[r#"
en Baz
en Result
md core
ev Bar
ev Err
ev Foo
ev Ok
bn Bar Bar$0
bn Err(…) Err($1)$0
bn Foo Foo$0
bn Ok(…) Ok($1)$0
kw mut
kw ref
"#]],
);
}

#[test]
fn pattern_enum_variant() {
check(
Expand Down

0 comments on commit 7c05f55

Please sign in to comment.