Unnamed repository; edit this file 'description' to name the repository.
Merge #11145
11145: feat: add config to use reasonable default expression instead of todo! when filling missing fields r=Veykril a=bnjjj
Use `Default::default()` in struct fields when we ask to fill it instead of putting `todo!()` for every fields
before:
```rust
pub enum Other {
One,
Two,
}
pub struct Test {
text: String,
num: usize,
other: Other,
}
fn t_test() {
let test = Test {<|>};
}
```
after:
```rust
pub enum Other {
One,
Two,
}
pub struct Test {
text: String,
num: usize,
other: Other,
}
fn t_test() {
let test = Test {
text: String::new(),
num: 0,
other: todo!(),
};
}
```
Co-authored-by: Benjamin Coenen <[email protected]>
Co-authored-by: Coenen Benjamin <[email protected]>
| -rw-r--r-- | crates/hir/src/lib.rs | 29 | ||||
| -rw-r--r-- | crates/hir_expand/src/name.rs | 1 | ||||
| -rw-r--r-- | crates/hir_ty/src/method_resolution.rs | 1 | ||||
| -rw-r--r-- | crates/ide/src/lib.rs | 2 | ||||
| -rw-r--r-- | crates/ide_diagnostics/src/handlers/missing_fields.rs | 168 | ||||
| -rw-r--r-- | crates/ide_diagnostics/src/lib.rs | 12 | ||||
| -rw-r--r-- | crates/ide_diagnostics/src/tests.rs | 16 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/config.rs | 27 | ||||
| -rw-r--r-- | crates/syntax/src/ast/make.rs | 22 | ||||
| -rw-r--r-- | docs/user/generated_config.adoc | 5 | ||||
| -rw-r--r-- | editors/code/package.json | 13 |
11 files changed, 271 insertions, 25 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index c6d401a6d6..3b28b14668 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1689,6 +1689,26 @@ impl BuiltinType { pub fn name(self) -> Name { self.inner.as_name() } + + pub fn is_int(&self) -> bool { + matches!(self.inner, hir_def::builtin_type::BuiltinType::Int(_)) + } + + pub fn is_uint(&self) -> bool { + matches!(self.inner, hir_def::builtin_type::BuiltinType::Uint(_)) + } + + pub fn is_float(&self) -> bool { + matches!(self.inner, hir_def::builtin_type::BuiltinType::Float(_)) + } + + pub fn is_char(&self) -> bool { + matches!(self.inner, hir_def::builtin_type::BuiltinType::Char) + } + + pub fn is_str(&self) -> bool { + matches!(self.inner, hir_def::builtin_type::BuiltinType::Str) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -2615,6 +2635,10 @@ impl Type { matches!(&self.ty.kind(Interner), TyKind::FnDef(..) | TyKind::Function { .. }) } + pub fn is_array(&self) -> bool { + matches!(&self.ty.kind(Interner), TyKind::Array(..)) + } + pub fn is_packed(&self, db: &dyn HirDatabase) -> bool { let adt_id = match *self.ty.kind(Interner) { TyKind::Adt(hir_ty::AdtId(adt_id), ..) => adt_id, @@ -2713,7 +2737,7 @@ impl Type { // This would be nicer if it just returned an iterator, but that runs into // lifetime problems, because we need to borrow temp `CrateImplDefs`. pub fn iterate_assoc_items<T>( - self, + &self, db: &dyn HirDatabase, krate: Crate, mut callback: impl FnMut(AssocItem) -> Option<T>, @@ -2727,7 +2751,7 @@ impl Type { } fn iterate_assoc_items_dyn( - self, + &self, db: &dyn HirDatabase, krate: Crate, callback: &mut dyn FnMut(AssocItemId) -> bool, @@ -2769,6 +2793,7 @@ impl Type { ) -> Option<T> { let _p = profile::span("iterate_method_candidates"); let mut slot = None; + self.iterate_method_candidates_dyn( db, krate, diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs index 23f81d3c7f..3636ab6213 100644 --- a/crates/hir_expand/src/name.rs +++ b/crates/hir_expand/src/name.rs @@ -226,6 +226,7 @@ pub mod known { iter_mut, len, is_empty, + new, // Builtin macros asm, assert, diff --git a/crates/hir_ty/src/method_resolution.rs b/crates/hir_ty/src/method_resolution.rs index cb6b6ec39f..1a451ae79f 100644 --- a/crates/hir_ty/src/method_resolution.rs +++ b/crates/hir_ty/src/method_resolution.rs @@ -542,6 +542,7 @@ pub fn iterate_method_candidates_dyn( let deref_chain = autoderef_method_receiver(db, krate, ty); let mut deref_chains = stdx::slice_tails(&deref_chain); + deref_chains.try_for_each(|deref_chain| { iterate_method_candidates_with_autoref( deref_chain, diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 16a2b4a113..b8482d0222 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -118,7 +118,7 @@ pub use ide_db::{ symbol_index::Query, RootDatabase, SymbolKind, }; -pub use ide_diagnostics::{Diagnostic, DiagnosticsConfig, Severity}; +pub use ide_diagnostics::{Diagnostic, DiagnosticsConfig, ExprFillDefaultMode, Severity}; pub use ide_ssr::SsrError; pub use syntax::{TextRange, TextSize}; pub use text_edit::{Indel, TextEdit}; diff --git a/crates/ide_diagnostics/src/handlers/missing_fields.rs b/crates/ide_diagnostics/src/handlers/missing_fields.rs index 8d17a7e714..186bfc3d58 100644 --- a/crates/ide_diagnostics/src/handlers/missing_fields.rs +++ b/crates/ide_diagnostics/src/handlers/missing_fields.rs @@ -1,9 +1,16 @@ use either::Either; -use hir::{db::AstDatabase, InFile}; -use ide_db::{assists::Assist, source_change::SourceChange}; +use hir::{ + db::{AstDatabase, HirDatabase}, + known, AssocItem, HirDisplay, InFile, Type, +}; +use ide_db::{assists::Assist, helpers::FamousDefs, source_change::SourceChange}; use rustc_hash::FxHashMap; use stdx::format_to; -use syntax::{algo, ast::make, AstNode, SyntaxNodePtr}; +use syntax::{ + algo, + ast::{self, make}, + AstNode, SyntaxNodePtr, +}; use text_edit::TextEdit; use crate::{fix, Diagnostic, DiagnosticsContext}; @@ -63,6 +70,18 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass } }); let missing_fields = ctx.sema.record_literal_missing_fields(&field_list_parent); + + let generate_fill_expr = |ty: &Type| match ctx.config.expr_fill_default { + crate::ExprFillDefaultMode::Todo => Some(make::ext::expr_todo()), + crate::ExprFillDefaultMode::Default => { + let default_constr = get_default_constructor(ctx, d, ty); + match default_constr { + Some(default_constr) => Some(default_constr), + _ => Some(make::ext::expr_todo()), + } + } + }; + for (f, ty) in missing_fields.iter() { let field_expr = if let Some(local_candidate) = locals.get(&f.name(ctx.sema.db)) { cov_mark::hit!(field_shorthand); @@ -70,10 +89,10 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass if ty.could_unify_with(ctx.sema.db, &candidate_ty) { None } else { - Some(make::ext::expr_todo()) + generate_fill_expr(ty) } } else { - Some(make::ext::expr_todo()) + generate_fill_expr(ty) }; let field = make::record_expr_field(make::name_ref(&f.name(ctx.sema.db).to_smol_str()), field_expr) @@ -102,6 +121,68 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass )]) } +fn make_ty(ty: &hir::Type, db: &dyn HirDatabase, module: hir::Module) -> ast::Type { + let ty_str = match ty.as_adt() { + Some(adt) => adt.name(db).to_string(), + None => ty.display_source_code(db, module.into()).ok().unwrap_or_else(|| "_".to_string()), + }; + + make::ty(&ty_str) +} + +fn get_default_constructor( + ctx: &DiagnosticsContext<'_>, + d: &hir::MissingFields, + ty: &Type, +) -> Option<ast::Expr> { + if let Some(builtin_ty) = ty.as_builtin() { + if builtin_ty.is_int() || builtin_ty.is_uint() { + return Some(make::ext::zero_number()); + } + if builtin_ty.is_float() { + return Some(make::ext::zero_float()); + } + if builtin_ty.is_char() { + return Some(make::ext::empty_char()); + } + if builtin_ty.is_str() { + return Some(make::ext::empty_str()); + } + } + + let krate = ctx.sema.to_module_def(d.file.original_file(ctx.sema.db))?.krate(); + let module = krate.root_module(ctx.sema.db); + + // Look for a ::new() associated function + let has_new_func = ty + .iterate_assoc_items(ctx.sema.db, krate, |assoc_item| { + if let AssocItem::Function(func) = assoc_item { + if func.name(ctx.sema.db) == known::new + && func.assoc_fn_params(ctx.sema.db).is_empty() + { + return Some(()); + } + } + + None + }) + .is_some(); + + if has_new_func { + Some(make::ext::expr_ty_new(&make_ty(ty, ctx.sema.db, module))) + } else if !ty.is_array() + && ty.impls_trait( + ctx.sema.db, + FamousDefs(&ctx.sema, Some(krate)).core_default_Default()?, + &[], + ) + { + Some(make::ext::expr_ty_default(&make_ty(ty, ctx.sema.db, module))) + } else { + None + } +} + #[cfg(test)] mod tests { use crate::tests::{check_diagnostics, check_fix}; @@ -182,7 +263,7 @@ fn here() {} macro_rules! id { ($($tt:tt)*) => { $($tt)*}; } fn main() { - let _x = id![Foo {a:42, b: todo!() }]; + let _x = id![Foo {a:42, b: 0 }]; } pub struct Foo { pub a: i32, pub b: i32 } @@ -204,7 +285,7 @@ fn test_fn() { struct TestStruct { one: i32, two: i64 } fn test_fn() { - let s = TestStruct { one: todo!(), two: todo!() }; + let s = TestStruct { one: 0, two: 0 }; } "#, ); @@ -224,7 +305,7 @@ impl TestStruct { struct TestStruct { one: i32 } impl TestStruct { - fn test_fn() { let s = Self { one: todo!() }; } + fn test_fn() { let s = Self { one: 0 }; } } "#, ); @@ -272,7 +353,72 @@ fn test_fn() { struct TestStruct { one: i32, two: i64 } fn test_fn() { - let s = TestStruct{ two: 2, one: todo!() }; + let s = TestStruct{ two: 2, one: 0 }; +} +", + ); + } + + #[test] + fn test_fill_struct_fields_new() { + check_fix( + r#" +struct TestWithNew(usize); +impl TestWithNew { + pub fn new() -> Self { + Self(0) + } +} +struct TestStruct { one: i32, two: TestWithNew } + +fn test_fn() { + let s = TestStruct{ $0 }; +} +"#, + r" +struct TestWithNew(usize); +impl TestWithNew { + pub fn new() -> Self { + Self(0) + } +} +struct TestStruct { one: i32, two: TestWithNew } + +fn test_fn() { + let s = TestStruct{ one: 0, two: TestWithNew::new() }; +} +", + ); + } + + #[test] + fn test_fill_struct_fields_default() { + check_fix( + r#" +//- minicore: default +struct TestWithDefault(usize); +impl Default for TestWithDefault { + pub fn default() -> Self { + Self(0) + } +} +struct TestStruct { one: i32, two: TestWithDefault } + +fn test_fn() { + let s = TestStruct{ $0 }; +} +"#, + r" +struct TestWithDefault(usize); +impl Default for TestWithDefault { + pub fn default() -> Self { + Self(0) + } +} +struct TestStruct { one: i32, two: TestWithDefault } + +fn test_fn() { + let s = TestStruct{ one: 0, two: TestWithDefault::default() }; } ", ); @@ -292,7 +438,7 @@ fn test_fn() { struct TestStruct { r#type: u8 } fn test_fn() { - TestStruct { r#type: todo!() }; + TestStruct { r#type: 0 }; } ", ); @@ -403,7 +549,7 @@ fn f() { let b = 1usize; S { a, - b: todo!(), + b: 0, }; } "#, diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index c921d989bd..86d76751ad 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs @@ -129,10 +129,22 @@ pub enum Severity { WeakWarning, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ExprFillDefaultMode { + Todo, + Default, +} +impl Default for ExprFillDefaultMode { + fn default() -> Self { + Self::Todo + } +} + #[derive(Default, Debug, Clone)] pub struct DiagnosticsConfig { pub disable_experimental: bool, pub disabled: FxHashSet<String>, + pub expr_fill_default: ExprFillDefaultMode, } struct DiagnosticsContext<'a> { diff --git a/crates/ide_diagnostics/src/tests.rs b/crates/ide_diagnostics/src/tests.rs index a2b92c4ff9..7d06e9d36f 100644 --- a/crates/ide_diagnostics/src/tests.rs +++ b/crates/ide_diagnostics/src/tests.rs @@ -9,7 +9,7 @@ use ide_db::{ use stdx::trim_indent; use test_utils::{assert_eq_text, extract_annotations}; -use crate::{DiagnosticsConfig, Severity}; +use crate::{DiagnosticsConfig, ExprFillDefaultMode, Severity}; /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: @@ -36,14 +36,12 @@ fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) { let after = trim_indent(ra_fixture_after); let (db, file_position) = RootDatabase::with_position(ra_fixture_before); - let diagnostic = super::diagnostics( - &db, - &DiagnosticsConfig::default(), - &AssistResolveStrategy::All, - file_position.file_id, - ) - .pop() - .expect("no diagnostics"); + let mut conf = DiagnosticsConfig::default(); + conf.expr_fill_default = ExprFillDefaultMode::Default; + let diagnostic = + super::diagnostics(&db, &conf, &AssistResolveStrategy::All, file_position.file_id) + .pop() + .expect("no diagnostics"); let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth]; let actual = { let source_change = fix.source_change.as_ref().unwrap(); diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index de0aba0caa..88d86ef503 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -11,8 +11,8 @@ use std::{ffi::OsString, iter, path::PathBuf}; use flycheck::FlycheckConfig; use ide::{ - AssistConfig, CompletionConfig, DiagnosticsConfig, HighlightRelatedConfig, HoverConfig, - HoverDocFormat, InlayHintsConfig, JoinLinesConfig, Snippet, SnippetScope, + AssistConfig, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode, HighlightRelatedConfig, + HoverConfig, HoverDocFormat, InlayHintsConfig, JoinLinesConfig, Snippet, SnippetScope, }; use ide_db::helpers::{ insert_use::{ImportGranularity, InsertUseConfig, PrefixKind}, @@ -45,6 +45,8 @@ use crate::{ // parsing the old name. config_data! { struct ConfigData { + /// Placeholder for missing expressions in assists. + assist_exprFillDefault: ExprFillDefaultDef = "\"todo\"", /// How imports should be grouped into use statements. assist_importGranularity | assist_importMergeBehavior | @@ -708,6 +710,10 @@ impl Config { DiagnosticsConfig { disable_experimental: !self.data.diagnostics_enableExperimental, disabled: self.data.diagnostics_disabled.clone(), + expr_fill_default: match self.data.assist_exprFillDefault { + ExprFillDefaultDef::Todo => ExprFillDefaultMode::Todo, + ExprFillDefaultDef::Default => ExprFillDefaultMode::Default, + }, } } pub fn diagnostics_map(&self) -> DiagnosticsMapConfig { @@ -1081,6 +1087,15 @@ enum ManifestOrProjectJson { #[derive(Deserialize, Debug, Clone)] #[serde(rename_all = "snake_case")] +pub enum ExprFillDefaultDef { + #[serde(alias = "todo")] + Todo, + #[serde(alias = "default")] + Default, +} + +#[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "snake_case")] enum ImportGranularityDef { Preserve, #[serde(alias = "none")] @@ -1285,6 +1300,14 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json "Merge imports from the same module into a single `use` statement." ], }, + "ExprFillDefaultDef" => set! { + "type": "string", + "enum": ["todo", "default"], + "enumDescriptions": [ + "Fill missing expressions with the `todo` macro", + "Fill missing expressions with reasonable defaults, `new` or `default` constructors." + ], + }, "ImportGranularityDef" => set! { "type": "string", "enum": ["preserve", "crate", "module", "item"], diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index e1938307cf..3575020975 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -59,6 +59,28 @@ pub mod ext { pub fn expr_todo() -> ast::Expr { expr_from_text("todo!()") } + pub fn expr_ty_default(ty: &ast::Type) -> ast::Expr { + expr_from_text(&format!("{}::default()", ty)) + } + pub fn expr_ty_new(ty: &ast::Type) -> ast::Expr { + expr_from_text(&format!("{}::new()", ty)) + } + + pub fn zero_number() -> ast::Expr { + expr_from_text("0") + } + pub fn zero_float() -> ast::Expr { + expr_from_text("0.0") + } + pub fn empty_str() -> ast::Expr { + expr_from_text(r#""""#) + } + pub fn empty_char() -> ast::Expr { + expr_from_text("'\x00'") + } + pub fn default_bool() -> ast::Expr { + expr_from_text("false") + } pub fn empty_block_expr() -> ast::BlockExpr { block_expr(None, None) } diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index f46dda2351..88dcacfe83 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -1,3 +1,8 @@ +[[rust-analyzer.assist.exprFillDefault]]rust-analyzer.assist.exprFillDefault (default: `"todo"`):: ++ +-- +Placeholder for missing expressions in assists. +-- [[rust-analyzer.assist.importGranularity]]rust-analyzer.assist.importGranularity (default: `"crate"`):: + -- diff --git a/editors/code/package.json b/editors/code/package.json index d418868474..2c7d6c3773 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -378,6 +378,19 @@ "markdownDescription": "Optional settings passed to the debug engine. Example: `{ \"lldb\": { \"terminal\":\"external\"} }`" }, "$generated-start": {}, + "rust-analyzer.assist.exprFillDefault": { + "markdownDescription": "Placeholder for missing expressions in assists.", + "default": "todo", + "type": "string", + "enum": [ + "todo", + "default" + ], + "enumDescriptions": [ + "Fill missing expressions with the `todo` macro", + "Fill missing expressions with reasonable defaults, `new` or `default` constructors." + ] + }, "rust-analyzer.assist.importGranularity": { "markdownDescription": "How imports should be grouped into use statements.", "default": "crate", |