Skip to content

Commit

Permalink
Auto merge of #17516 - kilpkonn:master, r=kilpkonn
Browse files Browse the repository at this point in the history
Quality of life improvements to term search

Basically two things:
- Allow optionally disabling "borrow checking" restrictions on term search code assists. Sometimes it is better to get invalid suggestions and fix borrow checking issues later...
- Remove explicit generics in generated expressions. I find it quite rare that one writes `None::<T>` instead of `None`.
  • Loading branch information
bors committed Jun 30, 2024
2 parents 7e8f9c8 + c194582 commit 56ef240
Show file tree
Hide file tree
Showing 12 changed files with 51 additions and 77 deletions.
78 changes: 8 additions & 70 deletions crates/hir/src/term_search/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use hir_ty::{
use itertools::Itertools;

use crate::{
Adt, AsAssocItem, AssocItemContainer, Const, ConstParam, Field, Function, GenericDef, Local,
ModuleDef, SemanticsScope, Static, Struct, StructKind, Trait, Type, Variant,
Adt, AsAssocItem, AssocItemContainer, Const, ConstParam, Field, Function, Local, ModuleDef,
SemanticsScope, Static, Struct, StructKind, Trait, Type, Variant,
};

/// Helper function to get path to `ModuleDef`
Expand All @@ -35,43 +35,6 @@ fn mod_item_path_str(
.ok_or(DisplaySourceCodeError::PathNotFound)
}

/// Helper function to get path to `Type`
fn type_path(
sema_scope: &SemanticsScope<'_>,
ty: &Type,
cfg: ImportPathConfig,
) -> Result<String, DisplaySourceCodeError> {
let db = sema_scope.db;
let m = sema_scope.module();

match ty.as_adt() {
Some(adt) => {
let ty_name = ty.display_source_code(db, m.id, true)?;

let mut path = mod_item_path(sema_scope, &ModuleDef::Adt(adt), cfg).unwrap();
path.pop_segment();
let path = path.display(db.upcast()).to_string();
let res = match path.is_empty() {
true => ty_name,
false => format!("{path}::{ty_name}"),
};
Ok(res)
}
None => ty.display_source_code(db, m.id, true),
}
}

/// Helper function to filter out generic parameters that are default
fn non_default_generics(db: &dyn HirDatabase, def: GenericDef, generics: &[Type]) -> Vec<Type> {
def.type_or_const_params(db)
.into_iter()
.filter_map(|it| it.as_type_param(db))
.zip(generics)
.filter(|(tp, arg)| tp.default(db).as_ref() != Some(arg))
.map(|(_, arg)| arg.clone())
.collect()
}

/// Type tree shows how can we get from set of types to some type.
///
/// Consider the following code as an example
Expand Down Expand Up @@ -208,20 +171,7 @@ impl Expr {
None => Ok(format!("{target_str}.{func_name}({args})")),
}
}
Expr::Variant { variant, generics, params } => {
let generics = non_default_generics(db, variant.parent_enum(db).into(), generics);
let generics_str = match generics.is_empty() {
true => String::new(),
false => {
let generics = generics
.iter()
.map(|it| type_path(sema_scope, it, cfg))
.collect::<Result<Vec<String>, DisplaySourceCodeError>>()?
.into_iter()
.join(", ");
format!("::<{generics}>")
}
};
Expr::Variant { variant, params, .. } => {
let inner = match variant.kind(db) {
StructKind::Tuple => {
let args = params
Expand All @@ -230,7 +180,7 @@ impl Expr {
.collect::<Result<Vec<String>, DisplaySourceCodeError>>()?
.into_iter()
.join(", ");
format!("{generics_str}({args})")
format!("({args})")
}
StructKind::Record => {
let fields = variant.fields(db);
Expand All @@ -248,16 +198,15 @@ impl Expr {
.collect::<Result<Vec<String>, DisplaySourceCodeError>>()?
.into_iter()
.join(", ");
format!("{generics_str}{{ {args} }}")
format!("{{ {args} }}")
}
StructKind::Unit => generics_str,
StructKind::Unit => String::new(),
};

let prefix = mod_item_path_str(sema_scope, &ModuleDef::Variant(*variant))?;
Ok(format!("{prefix}{inner}"))
}
Expr::Struct { strukt, generics, params } => {
let generics = non_default_generics(db, (*strukt).into(), generics);
Expr::Struct { strukt, params, .. } => {
let inner = match strukt.kind(db) {
StructKind::Tuple => {
let args = params
Expand Down Expand Up @@ -286,18 +235,7 @@ impl Expr {
.join(", ");
format!(" {{ {args} }}")
}
StructKind::Unit => match generics.is_empty() {
true => String::new(),
false => {
let generics = generics
.iter()
.map(|it| type_path(sema_scope, it, cfg))
.collect::<Result<Vec<String>, DisplaySourceCodeError>>()?
.into_iter()
.join(", ");
format!("::<{generics}>")
}
},
StructKind::Unit => String::new(),
};

let prefix = mod_item_path_str(sema_scope, &ModuleDef::Adt(Adt::Struct(*strukt)))?;
Expand Down
1 change: 1 addition & 0 deletions crates/ide-assists/src/assist_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ pub struct AssistConfig {
pub prefer_prelude: bool,
pub assist_emit_must_use: bool,
pub term_search_fuel: u64,
pub term_search_borrowck: bool,
}
14 changes: 9 additions & 5 deletions crates/ide-assists/src/handlers/term_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ pub(crate) fn term_search(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<
sema: &ctx.sema,
scope: &scope,
goal: target_ty,
config: TermSearchConfig { fuel: ctx.config.term_search_fuel, ..Default::default() },
config: TermSearchConfig {
fuel: ctx.config.term_search_fuel,
enable_borrowcheck: ctx.config.term_search_borrowck,
..Default::default()
},
};
let paths = hir::term_search::term_search(&term_search_ctx);

Expand Down Expand Up @@ -144,7 +148,7 @@ fn f() { let a = A { x: 1, y: true }; let b: i32 = a.x; }"#,
term_search,
r#"//- minicore: todo, unimplemented, option
fn f() { let a: i32 = 1; let b: Option<i32> = todo$0!(); }"#,
r#"fn f() { let a: i32 = 1; let b: Option<i32> = Some::<i32>(a); }"#,
r#"fn f() { let a: i32 = 1; let b: Option<i32> = Some(a); }"#,
)
}

Expand All @@ -156,7 +160,7 @@ fn f() { let a: i32 = 1; let b: Option<i32> = todo$0!(); }"#,
enum Option<T> { None, Some(T) }
fn f() { let a: i32 = 1; let b: Option<i32> = todo$0!(); }"#,
r#"enum Option<T> { None, Some(T) }
fn f() { let a: i32 = 1; let b: Option<i32> = Option::Some::<i32>(a); }"#,
fn f() { let a: i32 = 1; let b: Option<i32> = Option::Some(a); }"#,
)
}

Expand All @@ -168,7 +172,7 @@ fn f() { let a: i32 = 1; let b: Option<i32> = Option::Some::<i32>(a); }"#,
enum Option<T> { None, Some(T) }
fn f() { let a: Option<i32> = Option::None; let b: Option<Option<i32>> = todo$0!(); }"#,
r#"enum Option<T> { None, Some(T) }
fn f() { let a: Option<i32> = Option::None; let b: Option<Option<i32>> = Option::Some::<Option<i32>>(a); }"#,
fn f() { let a: Option<i32> = Option::None; let b: Option<Option<i32>> = Option::Some(a); }"#,
)
}

Expand All @@ -180,7 +184,7 @@ fn f() { let a: Option<i32> = Option::None; let b: Option<Option<i32>> = Option:
enum Foo<T = i32> { Foo(T) }
fn f() { let a = 0; let b: Foo = todo$0!(); }"#,
r#"enum Foo<T = i32> { Foo(T) }
fn f() { let a = 0; let b: Foo = Foo::Foo::<i32>(a); }"#,
fn f() { let a = 0; let b: Foo = Foo::Foo(a); }"#,
);

check_assist(
Expand Down
3 changes: 3 additions & 0 deletions crates/ide-assists/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig {
prefer_prelude: true,
assist_emit_must_use: false,
term_search_fuel: 400,
term_search_borrowck: true,
};

pub(crate) const TEST_CONFIG_NO_SNIPPET_CAP: AssistConfig = AssistConfig {
Expand All @@ -48,6 +49,7 @@ pub(crate) const TEST_CONFIG_NO_SNIPPET_CAP: AssistConfig = AssistConfig {
prefer_prelude: true,
assist_emit_must_use: false,
term_search_fuel: 400,
term_search_borrowck: true,
};

pub(crate) const TEST_CONFIG_IMPORT_ONE: AssistConfig = AssistConfig {
Expand All @@ -64,6 +66,7 @@ pub(crate) const TEST_CONFIG_IMPORT_ONE: AssistConfig = AssistConfig {
prefer_prelude: true,
assist_emit_must_use: false,
term_search_fuel: 400,
term_search_borrowck: true,
};

pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) {
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-completion/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2641,7 +2641,7 @@ fn foo() {
expect![[r#"
lc foo [type+local]
ex foo [type]
ex Foo::B::<u32> [type]
ex Foo::B [type]
ev Foo::A(…) [type_could_unify]
ev Foo::B [type_could_unify]
en Foo [type_could_unify]
Expand Down
7 changes: 6 additions & 1 deletion crates/ide-diagnostics/src/handlers/typed_hole.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypedHole) -> Option<Vec<Assist>
sema: &ctx.sema,
scope: &scope,
goal: d.expected.clone(),
config: TermSearchConfig { fuel: ctx.config.term_search_fuel, ..Default::default() },
config: TermSearchConfig {
fuel: ctx.config.term_search_fuel,
enable_borrowcheck: ctx.config.term_search_borrowck,

..Default::default()
},
};
let paths = term_search(&term_search_ctx);

Expand Down
2 changes: 2 additions & 0 deletions crates/ide-diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ pub struct DiagnosticsConfig {
pub prefer_no_std: bool,
pub prefer_prelude: bool,
pub term_search_fuel: u64,
pub term_search_borrowck: bool,
}

impl DiagnosticsConfig {
Expand All @@ -260,6 +261,7 @@ impl DiagnosticsConfig {
prefer_no_std: false,
prefer_prelude: true,
term_search_fuel: 400,
term_search_borrowck: true,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/cli/analysis_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,7 @@ impl flags::AnalysisStats {
prefer_prelude: true,
style_lints: false,
term_search_fuel: 400,
term_search_borrowck: true,
},
ide::AssistResolveStrategy::All,
file_id,
Expand Down
4 changes: 4 additions & 0 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ config_data! {
assist_emitMustUse: bool = false,
/// Placeholder expression to use for missing expressions in assists.
assist_expressionFillDefault: ExprFillDefaultDef = ExprFillDefaultDef::Todo,
/// Enable borrow checking for term search code assists. If set to false, also there will be more suggestions, but some of them may not borrow-check.
assist_termSearch_borrowcheck: bool = true,
/// Term search fuel in "units of work" for assists (Defaults to 1800).
assist_termSearch_fuel: usize = 1800,

Expand Down Expand Up @@ -1269,6 +1271,7 @@ impl Config {
assist_emit_must_use: self.assist_emitMustUse(source_root).to_owned(),
prefer_prelude: self.imports_preferPrelude(source_root).to_owned(),
term_search_fuel: self.assist_termSearch_fuel(source_root).to_owned() as u64,
term_search_borrowck: self.assist_termSearch_borrowcheck(source_root).to_owned(),
}
}

Expand Down Expand Up @@ -1328,6 +1331,7 @@ impl Config {
prefer_prelude: self.imports_preferPrelude(source_root).to_owned(),
style_lints: self.diagnostics_styleLints_enable().to_owned(),
term_search_fuel: self.assist_termSearch_fuel(source_root).to_owned() as u64,
term_search_borrowck: self.assist_termSearch_borrowcheck(source_root).to_owned(),
}
}
pub fn expand_proc_attr_macros(&self) -> bool {
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/integrated_benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ fn integrated_diagnostics_benchmark() {
prefer_no_std: false,
prefer_prelude: false,
term_search_fuel: 400,
term_search_borrowck: true,
};
host.analysis()
.diagnostics(&diagnostics_config, ide::AssistResolveStrategy::None, file_id)
Expand Down
5 changes: 5 additions & 0 deletions docs/user/generated_config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ for enum variants.
--
Placeholder expression to use for missing expressions in assists.
--
[[rust-analyzer.assist.termSearch.borrowcheck]]rust-analyzer.assist.termSearch.borrowcheck (default: `true`)::
+
--
Enable borrow checking for term search code assists. If set to false, also there will be more suggestions, but some of them may not borrow-check.
--
[[rust-analyzer.assist.termSearch.fuel]]rust-analyzer.assist.termSearch.fuel (default: `1800`)::
+
--
Expand Down
10 changes: 10 additions & 0 deletions editors/code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,16 @@
}
}
},
{
"title": "assist",
"properties": {
"rust-analyzer.assist.termSearch.borrowcheck": {
"markdownDescription": "Enable borrow checking for term search code assists. If set to false, also there will be more suggestions, but some of them may not borrow-check.",
"default": true,
"type": "boolean"
}
}
},
{
"title": "assist",
"properties": {
Expand Down

0 comments on commit 56ef240

Please sign in to comment.