feat: Deprioritize completions of private but editable definitions

This commit is contained in:
Lukas Wirth 2022-02-02 02:05:49 +01:00
parent d7a544e69a
commit 5c41f5d165
12 changed files with 120 additions and 58 deletions

View file

@ -22,6 +22,7 @@ use hir::known;
use ide_db::SymbolKind; use ide_db::SymbolKind;
use crate::{ use crate::{
context::Visible,
item::Builder, item::Builder,
render::{ render::{
const_::render_const, const_::render_const,
@ -91,7 +92,7 @@ impl Completions {
cov_mark::hit!(qualified_path_doc_hidden); cov_mark::hit!(qualified_path_doc_hidden);
return; return;
} }
self.add(render_resolution(RenderContext::new(ctx), local_name, resolution)); self.add(render_resolution(RenderContext::new(ctx, false), local_name, resolution));
} }
pub(crate) fn add_macro( pub(crate) fn add_macro(
@ -104,7 +105,7 @@ impl Completions {
Some(it) => it, Some(it) => it,
None => return, None => return,
}; };
self.add(render_macro(RenderContext::new(ctx), None, name, macro_)); self.add(render_macro(RenderContext::new(ctx, false), None, name, macro_));
} }
pub(crate) fn add_function( pub(crate) fn add_function(
@ -113,10 +114,12 @@ impl Completions {
func: hir::Function, func: hir::Function,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
) { ) {
if !ctx.is_visible(&func) { let is_private_editable = match ctx.is_visible(&func) {
return; Visible::Yes => false,
} Visible::Editable => true,
self.add(render_fn(RenderContext::new(ctx), None, local_name, func)); Visible::No => return,
};
self.add(render_fn(RenderContext::new(ctx, is_private_editable), None, local_name, func));
} }
pub(crate) fn add_method( pub(crate) fn add_method(
@ -126,24 +129,36 @@ impl Completions {
receiver: Option<hir::Name>, receiver: Option<hir::Name>,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
) { ) {
if !ctx.is_visible(&func) { let is_private_editable = match ctx.is_visible(&func) {
return; Visible::Yes => false,
} Visible::Editable => true,
self.add(render_method(RenderContext::new(ctx), None, receiver, local_name, func)); Visible::No => return,
};
self.add(render_method(
RenderContext::new(ctx, is_private_editable),
None,
receiver,
local_name,
func,
));
} }
pub(crate) fn add_const(&mut self, ctx: &CompletionContext, konst: hir::Const) { pub(crate) fn add_const(&mut self, ctx: &CompletionContext, konst: hir::Const) {
if !ctx.is_visible(&konst) { let is_private_editable = match ctx.is_visible(&konst) {
return; Visible::Yes => false,
} Visible::Editable => true,
self.add_opt(render_const(RenderContext::new(ctx), konst)); Visible::No => return,
};
self.add_opt(render_const(RenderContext::new(ctx, is_private_editable), konst));
} }
pub(crate) fn add_type_alias(&mut self, ctx: &CompletionContext, type_alias: hir::TypeAlias) { pub(crate) fn add_type_alias(&mut self, ctx: &CompletionContext, type_alias: hir::TypeAlias) {
if !ctx.is_visible(&type_alias) { let is_private_editable = match ctx.is_visible(&type_alias) {
return; Visible::Yes => false,
} Visible::Editable => true,
self.add_opt(render_type_alias(RenderContext::new(ctx), type_alias)); Visible::No => return,
};
self.add_opt(render_type_alias(RenderContext::new(ctx, is_private_editable), type_alias));
} }
pub(crate) fn add_type_alias_with_eq( pub(crate) fn add_type_alias_with_eq(
@ -151,7 +166,7 @@ impl Completions {
ctx: &CompletionContext, ctx: &CompletionContext,
type_alias: hir::TypeAlias, type_alias: hir::TypeAlias,
) { ) {
self.add_opt(render_type_alias_with_eq(RenderContext::new(ctx), type_alias)); self.add_opt(render_type_alias_with_eq(RenderContext::new(ctx, false), type_alias));
} }
pub(crate) fn add_qualified_enum_variant( pub(crate) fn add_qualified_enum_variant(
@ -160,7 +175,7 @@ impl Completions {
variant: hir::Variant, variant: hir::Variant,
path: hir::ModPath, path: hir::ModPath,
) { ) {
let item = render_variant(RenderContext::new(ctx), None, None, variant, Some(path)); let item = render_variant(RenderContext::new(ctx, false), None, None, variant, Some(path));
self.add(item); self.add(item);
} }
@ -170,7 +185,7 @@ impl Completions {
variant: hir::Variant, variant: hir::Variant,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
) { ) {
let item = render_variant(RenderContext::new(ctx), None, local_name, variant, None); let item = render_variant(RenderContext::new(ctx, false), None, local_name, variant, None);
self.add(item); self.add(item);
} }
@ -181,10 +196,12 @@ impl Completions {
field: hir::Field, field: hir::Field,
ty: &hir::Type, ty: &hir::Type,
) { ) {
if !ctx.is_visible(&field) { let is_private_editable = match ctx.is_visible(&field) {
return; Visible::Yes => false,
} Visible::Editable => true,
let item = render_field(RenderContext::new(ctx), receiver, field, ty); Visible::No => return,
};
let item = render_field(RenderContext::new(ctx, is_private_editable), receiver, field, ty);
self.add(item); self.add(item);
} }
@ -195,7 +212,7 @@ impl Completions {
path: Option<hir::ModPath>, path: Option<hir::ModPath>,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
) { ) {
let item = render_struct_literal(RenderContext::new(ctx), strukt, path, local_name); let item = render_struct_literal(RenderContext::new(ctx, false), strukt, path, local_name);
self.add_opt(item); self.add_opt(item);
} }
@ -206,7 +223,7 @@ impl Completions {
field: usize, field: usize,
ty: &hir::Type, ty: &hir::Type,
) { ) {
let item = render_tuple_field(RenderContext::new(ctx), receiver, field, ty); let item = render_tuple_field(RenderContext::new(ctx, false), receiver, field, ty);
self.add(item); self.add(item);
} }
@ -221,7 +238,7 @@ impl Completions {
variant: hir::Variant, variant: hir::Variant,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
) { ) {
self.add_opt(render_variant_pat(RenderContext::new(ctx), variant, local_name, None)); self.add_opt(render_variant_pat(RenderContext::new(ctx, false), variant, local_name, None));
} }
pub(crate) fn add_qualified_variant_pat( pub(crate) fn add_qualified_variant_pat(
@ -230,7 +247,7 @@ impl Completions {
variant: hir::Variant, variant: hir::Variant,
path: hir::ModPath, path: hir::ModPath,
) { ) {
self.add_opt(render_variant_pat(RenderContext::new(ctx), variant, None, Some(path))); self.add_opt(render_variant_pat(RenderContext::new(ctx, false), variant, None, Some(path)));
} }
pub(crate) fn add_struct_pat( pub(crate) fn add_struct_pat(
@ -239,7 +256,7 @@ impl Completions {
strukt: hir::Struct, strukt: hir::Struct,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
) { ) {
self.add_opt(render_struct_pat(RenderContext::new(ctx), strukt, local_name)); self.add_opt(render_struct_pat(RenderContext::new(ctx, false), strukt, local_name));
} }
} }

View file

@ -193,7 +193,7 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
}) })
.filter_map(|import| { .filter_map(|import| {
render_resolution_with_import( render_resolution_with_import(
RenderContext::new(ctx), RenderContext::new(ctx, false),
ImportEdit { import, scope: import_scope.clone() }, ImportEdit { import, scope: import_scope.clone() },
) )
}), }),

View file

@ -34,6 +34,11 @@ pub(crate) enum PatternRefutability {
Refutable, Refutable,
Irrefutable, Irrefutable,
} }
pub enum Visible {
Yes,
Editable,
No,
}
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
pub(super) enum PathKind { pub(super) enum PathKind {
@ -285,7 +290,7 @@ impl<'a> CompletionContext<'a> {
} }
/// Checks if an item is visible and not `doc(hidden)` at the completion site. /// Checks if an item is visible and not `doc(hidden)` at the completion site.
pub(crate) fn is_visible<I>(&self, item: &I) -> bool pub(crate) fn is_visible<I>(&self, item: &I) -> Visible
where where
I: hir::HasVisibility + hir::HasAttrs + hir::HasCrate + Copy, I: hir::HasVisibility + hir::HasAttrs + hir::HasCrate + Copy,
{ {
@ -339,20 +344,24 @@ impl<'a> CompletionContext<'a> {
vis: &hir::Visibility, vis: &hir::Visibility,
attrs: &hir::Attrs, attrs: &hir::Attrs,
defining_crate: hir::Crate, defining_crate: hir::Crate,
) -> bool { ) -> Visible {
let module = match self.module { let module = match self.module {
Some(it) => it, Some(it) => it,
None => return false, None => return Visible::No,
}; };
if !vis.is_visible_from(self.db, module.into()) { if !vis.is_visible_from(self.db, module.into()) {
// If the definition location is editable, also show private items // If the definition location is editable, also show private items
let root_file = defining_crate.root_file(self.db); let root_file = defining_crate.root_file(self.db);
let source_root_id = self.db.file_source_root(root_file); let source_root_id = self.db.file_source_root(root_file);
let is_editable = !self.db.source_root(source_root_id).is_library; let is_editable = !self.db.source_root(source_root_id).is_library;
return is_editable; return if is_editable { Visible::Editable } else { Visible::No };
} }
!self.is_doc_hidden(attrs, defining_crate) if self.is_doc_hidden(attrs, defining_crate) {
Visible::No
} else {
Visible::Yes
}
} }
fn is_doc_hidden(&self, attrs: &hir::Attrs, defining_crate: hir::Crate) -> bool { fn is_doc_hidden(&self, attrs: &hir::Attrs, defining_crate: hir::Crate) -> bool {

View file

@ -141,6 +141,8 @@ pub struct CompletionRelevance {
pub is_local: bool, pub is_local: bool,
/// Set for method completions of the `core::ops` and `core::cmp` family. /// Set for method completions of the `core::ops` and `core::cmp` family.
pub is_op_method: bool, pub is_op_method: bool,
/// Set for item completions that are private but in the workspace.
pub is_private_editable: bool,
/// This is set in cases like these: /// This is set in cases like these:
/// ///
/// ``` /// ```
@ -177,7 +179,7 @@ pub enum CompletionRelevanceTypeMatch {
} }
impl CompletionRelevance { impl CompletionRelevance {
const BASE_LINE: u32 = 1; const BASE_LINE: u32 = 2;
/// Provides a relevance score. Higher values are more relevant. /// Provides a relevance score. Higher values are more relevant.
/// ///
/// The absolute value of the relevance score is not meaningful, for /// The absolute value of the relevance score is not meaningful, for
@ -190,6 +192,15 @@ impl CompletionRelevance {
pub fn score(&self) -> u32 { pub fn score(&self) -> u32 {
let mut score = Self::BASE_LINE; let mut score = Self::BASE_LINE;
// score decreases
if self.is_op_method {
score -= 1;
}
if self.is_private_editable {
score -= 1;
}
// score increases
if self.exact_name_match { if self.exact_name_match {
score += 1; score += 1;
} }
@ -201,9 +212,6 @@ impl CompletionRelevance {
if self.is_local { if self.is_local {
score += 1; score += 1;
} }
if self.is_op_method {
score -= 1;
}
if self.exact_postfix_snippet_match { if self.exact_postfix_snippet_match {
score += 100; score += 100;
} }
@ -214,7 +222,7 @@ impl CompletionRelevance {
/// some threshold such that we think it is especially likely /// some threshold such that we think it is especially likely
/// to be relevant. /// to be relevant.
pub fn is_relevant(&self) -> bool { pub fn is_relevant(&self) -> bool {
self.score() > (Self::BASE_LINE + 1) self.score() > Self::BASE_LINE
} }
} }
@ -564,7 +572,15 @@ mod tests {
// This test asserts that the relevance score for these items is ascending, and // This test asserts that the relevance score for these items is ascending, and
// that any items in the same vec have the same score. // that any items in the same vec have the same score.
let expected_relevance_order = vec![ let expected_relevance_order = vec![
vec![CompletionRelevance { is_op_method: true, ..CompletionRelevance::default() }], vec![CompletionRelevance {
is_op_method: true,
is_private_editable: true,
..CompletionRelevance::default()
}],
vec![
CompletionRelevance { is_private_editable: true, ..CompletionRelevance::default() },
CompletionRelevance { is_op_method: true, ..CompletionRelevance::default() },
],
vec![CompletionRelevance::default()], vec![CompletionRelevance::default()],
vec![ vec![
CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() }, CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() },

View file

@ -28,11 +28,15 @@ use crate::{
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct RenderContext<'a> { pub(crate) struct RenderContext<'a> {
completion: &'a CompletionContext<'a>, completion: &'a CompletionContext<'a>,
is_private_editable: bool,
} }
impl<'a> RenderContext<'a> { impl<'a> RenderContext<'a> {
pub(crate) fn new(completion: &'a CompletionContext<'a>) -> RenderContext<'a> { pub(crate) fn new(
RenderContext { completion } completion: &'a CompletionContext<'a>,
is_private_editable: bool,
) -> RenderContext<'a> {
RenderContext { completion, is_private_editable }
} }
fn snippet_cap(&self) -> Option<SnippetCap> { fn snippet_cap(&self) -> Option<SnippetCap> {
@ -47,6 +51,10 @@ impl<'a> RenderContext<'a> {
self.completion.source_range() self.completion.source_range()
} }
fn completion_relevance(&self) -> CompletionRelevance {
CompletionRelevance { is_private_editable: self.is_private_editable, ..Default::default() }
}
fn is_deprecated(&self, def: impl HasAttrs) -> bool { fn is_deprecated(&self, def: impl HasAttrs) -> bool {
let attrs = def.attrs(self.db()); let attrs = def.attrs(self.db());
attrs.by_key("deprecated").exists() || attrs.by_key("rustc_deprecated").exists() attrs.by_key("deprecated").exists() || attrs.by_key("rustc_deprecated").exists()
@ -582,6 +590,7 @@ fn main() { let _: m::Spam = S$0 }
), ),
is_local: false, is_local: false,
is_op_method: false, is_op_method: false,
is_private_editable: false,
exact_postfix_snippet_match: false, exact_postfix_snippet_match: false,
}, },
trigger_call_info: true, trigger_call_info: true,
@ -603,6 +612,7 @@ fn main() { let _: m::Spam = S$0 }
), ),
is_local: false, is_local: false,
is_op_method: false, is_op_method: false,
is_private_editable: false,
exact_postfix_snippet_match: false, exact_postfix_snippet_match: false,
}, },
}, },
@ -689,6 +699,7 @@ fn foo() { A { the$0 } }
), ),
is_local: false, is_local: false,
is_op_method: false, is_op_method: false,
is_private_editable: false,
exact_postfix_snippet_match: false, exact_postfix_snippet_match: false,
}, },
}, },

View file

@ -18,7 +18,8 @@ fn render(ctx: RenderContext<'_>, const_: hir::Const) -> Option<CompletionItem>
let mut item = CompletionItem::new(SymbolKind::Const, ctx.source_range(), name.clone()); let mut item = CompletionItem::new(SymbolKind::Const, ctx.source_range(), name.clone());
item.set_documentation(ctx.docs(const_)) item.set_documentation(ctx.docs(const_))
.set_deprecated(ctx.is_deprecated(const_) || ctx.is_deprecated_assoc_item(const_)) .set_deprecated(ctx.is_deprecated(const_) || ctx.is_deprecated_assoc_item(const_))
.detail(detail); .detail(detail)
.set_relevance(ctx.completion_relevance());
if let Some(actm) = const_.as_assoc_item(db) { if let Some(actm) = const_.as_assoc_item(db) {
if let Some(trt) = actm.containing_trait_or_trait_impl(db) { if let Some(trt) = actm.containing_trait_or_trait_impl(db) {

View file

@ -23,7 +23,7 @@ pub(crate) fn render_variant(
} }
fn render( fn render(
ctx @ RenderContext { completion }: RenderContext<'_>, ctx @ RenderContext { completion, .. }: RenderContext<'_>,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
variant: hir::Variant, variant: hir::Variant,
path: Option<hir::ModPath>, path: Option<hir::ModPath>,
@ -58,18 +58,18 @@ fn render(
if variant_kind == hir::StructKind::Tuple { if variant_kind == hir::StructKind::Tuple {
cov_mark::hit!(inserts_parens_for_tuple_enums); cov_mark::hit!(inserts_parens_for_tuple_enums);
let params = Params::Anonymous(variant.fields(db).len()); let params = Params::Anonymous(variant.fields(db).len());
item.add_call_parens(ctx.completion, short_qualified_name, params); item.add_call_parens(completion, short_qualified_name, params);
} else if qualified { } else if qualified {
item.lookup_by(short_qualified_name); item.lookup_by(short_qualified_name);
} }
let ty = variant.parent_enum(ctx.completion.db).ty(ctx.completion.db); let ty = variant.parent_enum(completion.db).ty(completion.db);
item.set_relevance(CompletionRelevance { item.set_relevance(CompletionRelevance {
type_match: compute_type_match(ctx.completion, &ty), type_match: compute_type_match(completion, &ty),
..CompletionRelevance::default() ..ctx.completion_relevance()
}); });
if let Some(ref_match) = compute_ref_match(ctx.completion, &ty) { if let Some(ref_match) = compute_ref_match(completion, &ty) {
item.ref_match(ref_match); item.ref_match(ref_match);
} }

View file

@ -41,7 +41,7 @@ pub(crate) fn render_method(
} }
fn render( fn render(
ctx @ RenderContext { completion }: RenderContext<'_>, ctx @ RenderContext { completion, .. }: RenderContext<'_>,
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
func: hir::Function, func: hir::Function,
func_type: FuncType, func_type: FuncType,
@ -75,7 +75,7 @@ fn render(
type_match: compute_type_match(completion, &ret_type), type_match: compute_type_match(completion, &ret_type),
exact_name_match: compute_exact_name_match(completion, &call), exact_name_match: compute_exact_name_match(completion, &call),
is_op_method, is_op_method,
..CompletionRelevance::default() ..ctx.completion_relevance()
}); });
if let Some(ref_match) = compute_ref_match(completion, &ret_type) { if let Some(ref_match) = compute_ref_match(completion, &ret_type) {

View file

@ -25,7 +25,7 @@ pub(crate) fn render_macro(
} }
fn render( fn render(
ctx @ RenderContext { completion }: RenderContext<'_>, ctx @ RenderContext { completion, .. }: RenderContext<'_>,
name: hir::Name, name: hir::Name,
macro_: hir::MacroDef, macro_: hir::MacroDef,
import_to_add: Option<ImportEdit>, import_to_add: Option<ImportEdit>,
@ -53,7 +53,8 @@ fn render(
); );
item.set_deprecated(ctx.is_deprecated(macro_)) item.set_deprecated(ctx.is_deprecated(macro_))
.set_detail(detail(&completion.sema, macro_)) .set_detail(detail(&completion.sema, macro_))
.set_documentation(docs); .set_documentation(docs)
.set_relevance(ctx.completion_relevance());
if let Some(import_to_add) = import_to_add { if let Some(import_to_add) = import_to_add {
item.add_import(import_to_add); item.add_import(import_to_add);

View file

@ -59,7 +59,10 @@ fn build_completion(
def: impl HasAttrs + Copy, def: impl HasAttrs + Copy,
) -> CompletionItem { ) -> CompletionItem {
let mut item = CompletionItem::new(CompletionItemKind::Binding, ctx.source_range(), name); let mut item = CompletionItem::new(CompletionItemKind::Binding, ctx.source_range(), name);
item.set_documentation(ctx.docs(def)).set_deprecated(ctx.is_deprecated(def)).detail(&pat); item.set_documentation(ctx.docs(def))
.set_deprecated(ctx.is_deprecated(def))
.detail(&pat)
.set_relevance(ctx.completion_relevance());
match ctx.snippet_cap() { match ctx.snippet_cap() {
Some(snippet_cap) => item.insert_snippet(snippet_cap, pat), Some(snippet_cap) => item.insert_snippet(snippet_cap, pat),
None => item.insert_text(pat), None => item.insert_text(pat),

View file

@ -41,7 +41,10 @@ fn build_completion(
ctx.source_range(), ctx.source_range(),
SmolStr::from_iter([&name, " {…}"]), SmolStr::from_iter([&name, " {…}"]),
); );
item.set_documentation(ctx.docs(def)).set_deprecated(ctx.is_deprecated(def)).detail(&literal); item.set_documentation(ctx.docs(def))
.set_deprecated(ctx.is_deprecated(def))
.detail(&literal)
.set_relevance(ctx.completion_relevance());
match ctx.snippet_cap() { match ctx.snippet_cap() {
Some(snippet_cap) => item.insert_snippet(snippet_cap, literal), Some(snippet_cap) => item.insert_snippet(snippet_cap, literal),
None => item.insert_text(literal), None => item.insert_text(literal),

View file

@ -39,7 +39,8 @@ fn render(
let mut item = CompletionItem::new(SymbolKind::TypeAlias, ctx.source_range(), name.clone()); let mut item = CompletionItem::new(SymbolKind::TypeAlias, ctx.source_range(), name.clone());
item.set_documentation(ctx.docs(type_alias)) item.set_documentation(ctx.docs(type_alias))
.set_deprecated(ctx.is_deprecated(type_alias) || ctx.is_deprecated_assoc_item(type_alias)) .set_deprecated(ctx.is_deprecated(type_alias) || ctx.is_deprecated_assoc_item(type_alias))
.detail(detail); .detail(detail)
.set_relevance(ctx.completion_relevance());
if let Some(actm) = type_alias.as_assoc_item(db) { if let Some(actm) = type_alias.as_assoc_item(db) {
if let Some(trt) = actm.containing_trait_or_trait_impl(db) { if let Some(trt) = actm.containing_trait_or_trait_impl(db) {