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 <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Coenen Benjamin <benjamin.coenen@hotmail.com>
This commit is contained in:
bors[bot] 2022-01-07 14:10:11 +00:00 committed by GitHub
commit 40009e07d0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 271 additions and 25 deletions

View file

@ -1689,6 +1689,26 @@ impl BuiltinType {
pub fn name(self) -> Name { pub fn name(self) -> Name {
self.inner.as_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)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -2615,6 +2635,10 @@ impl Type {
matches!(&self.ty.kind(Interner), TyKind::FnDef(..) | TyKind::Function { .. }) 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 { pub fn is_packed(&self, db: &dyn HirDatabase) -> bool {
let adt_id = match *self.ty.kind(Interner) { let adt_id = match *self.ty.kind(Interner) {
TyKind::Adt(hir_ty::AdtId(adt_id), ..) => adt_id, 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 // This would be nicer if it just returned an iterator, but that runs into
// lifetime problems, because we need to borrow temp `CrateImplDefs`. // lifetime problems, because we need to borrow temp `CrateImplDefs`.
pub fn iterate_assoc_items<T>( pub fn iterate_assoc_items<T>(
self, &self,
db: &dyn HirDatabase, db: &dyn HirDatabase,
krate: Crate, krate: Crate,
mut callback: impl FnMut(AssocItem) -> Option<T>, mut callback: impl FnMut(AssocItem) -> Option<T>,
@ -2727,7 +2751,7 @@ impl Type {
} }
fn iterate_assoc_items_dyn( fn iterate_assoc_items_dyn(
self, &self,
db: &dyn HirDatabase, db: &dyn HirDatabase,
krate: Crate, krate: Crate,
callback: &mut dyn FnMut(AssocItemId) -> bool, callback: &mut dyn FnMut(AssocItemId) -> bool,
@ -2769,6 +2793,7 @@ impl Type {
) -> Option<T> { ) -> Option<T> {
let _p = profile::span("iterate_method_candidates"); let _p = profile::span("iterate_method_candidates");
let mut slot = None; let mut slot = None;
self.iterate_method_candidates_dyn( self.iterate_method_candidates_dyn(
db, db,
krate, krate,

View file

@ -226,6 +226,7 @@ pub mod known {
iter_mut, iter_mut,
len, len,
is_empty, is_empty,
new,
// Builtin macros // Builtin macros
asm, asm,
assert, assert,

View file

@ -542,6 +542,7 @@ pub fn iterate_method_candidates_dyn(
let deref_chain = autoderef_method_receiver(db, krate, ty); let deref_chain = autoderef_method_receiver(db, krate, ty);
let mut deref_chains = stdx::slice_tails(&deref_chain); let mut deref_chains = stdx::slice_tails(&deref_chain);
deref_chains.try_for_each(|deref_chain| { deref_chains.try_for_each(|deref_chain| {
iterate_method_candidates_with_autoref( iterate_method_candidates_with_autoref(
deref_chain, deref_chain,

View file

@ -118,7 +118,7 @@ pub use ide_db::{
symbol_index::Query, symbol_index::Query,
RootDatabase, SymbolKind, RootDatabase, SymbolKind,
}; };
pub use ide_diagnostics::{Diagnostic, DiagnosticsConfig, Severity}; pub use ide_diagnostics::{Diagnostic, DiagnosticsConfig, ExprFillDefaultMode, Severity};
pub use ide_ssr::SsrError; pub use ide_ssr::SsrError;
pub use syntax::{TextRange, TextSize}; pub use syntax::{TextRange, TextSize};
pub use text_edit::{Indel, TextEdit}; pub use text_edit::{Indel, TextEdit};

View file

@ -1,9 +1,16 @@
use either::Either; use either::Either;
use hir::{db::AstDatabase, InFile}; use hir::{
use ide_db::{assists::Assist, source_change::SourceChange}; db::{AstDatabase, HirDatabase},
known, AssocItem, HirDisplay, InFile, Type,
};
use ide_db::{assists::Assist, helpers::FamousDefs, source_change::SourceChange};
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use stdx::format_to; use stdx::format_to;
use syntax::{algo, ast::make, AstNode, SyntaxNodePtr}; use syntax::{
algo,
ast::{self, make},
AstNode, SyntaxNodePtr,
};
use text_edit::TextEdit; use text_edit::TextEdit;
use crate::{fix, Diagnostic, DiagnosticsContext}; 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 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() { for (f, ty) in missing_fields.iter() {
let field_expr = if let Some(local_candidate) = locals.get(&f.name(ctx.sema.db)) { let field_expr = if let Some(local_candidate) = locals.get(&f.name(ctx.sema.db)) {
cov_mark::hit!(field_shorthand); 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) { if ty.could_unify_with(ctx.sema.db, &candidate_ty) {
None None
} else { } else {
Some(make::ext::expr_todo()) generate_fill_expr(ty)
} }
} else { } else {
Some(make::ext::expr_todo()) generate_fill_expr(ty)
}; };
let field = let field =
make::record_expr_field(make::name_ref(&f.name(ctx.sema.db).to_smol_str()), field_expr) 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)] #[cfg(test)]
mod tests { mod tests {
use crate::tests::{check_diagnostics, check_fix}; use crate::tests::{check_diagnostics, check_fix};
@ -182,7 +263,7 @@ fn here() {}
macro_rules! id { ($($tt:tt)*) => { $($tt)*}; } macro_rules! id { ($($tt:tt)*) => { $($tt)*}; }
fn main() { 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 } pub struct Foo { pub a: i32, pub b: i32 }
@ -204,7 +285,7 @@ fn test_fn() {
struct TestStruct { one: i32, two: i64 } struct TestStruct { one: i32, two: i64 }
fn test_fn() { 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 } struct TestStruct { one: i32 }
impl TestStruct { 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 } struct TestStruct { one: i32, two: i64 }
fn test_fn() { 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 } struct TestStruct { r#type: u8 }
fn test_fn() { fn test_fn() {
TestStruct { r#type: todo!() }; TestStruct { r#type: 0 };
} }
", ",
); );
@ -403,7 +549,7 @@ fn f() {
let b = 1usize; let b = 1usize;
S { S {
a, a,
b: todo!(), b: 0,
}; };
} }
"#, "#,

View file

@ -129,10 +129,22 @@ pub enum Severity {
WeakWarning, WeakWarning,
} }
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ExprFillDefaultMode {
Todo,
Default,
}
impl Default for ExprFillDefaultMode {
fn default() -> Self {
Self::Todo
}
}
#[derive(Default, Debug, Clone)] #[derive(Default, Debug, Clone)]
pub struct DiagnosticsConfig { pub struct DiagnosticsConfig {
pub disable_experimental: bool, pub disable_experimental: bool,
pub disabled: FxHashSet<String>, pub disabled: FxHashSet<String>,
pub expr_fill_default: ExprFillDefaultMode,
} }
struct DiagnosticsContext<'a> { struct DiagnosticsContext<'a> {

View file

@ -9,7 +9,7 @@ use ide_db::{
use stdx::trim_indent; use stdx::trim_indent;
use test_utils::{assert_eq_text, extract_annotations}; 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, /// Takes a multi-file input fixture with annotated cursor positions,
/// and checks that: /// and checks that:
@ -36,12 +36,10 @@ fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) {
let after = trim_indent(ra_fixture_after); let after = trim_indent(ra_fixture_after);
let (db, file_position) = RootDatabase::with_position(ra_fixture_before); let (db, file_position) = RootDatabase::with_position(ra_fixture_before);
let diagnostic = super::diagnostics( let mut conf = DiagnosticsConfig::default();
&db, conf.expr_fill_default = ExprFillDefaultMode::Default;
&DiagnosticsConfig::default(), let diagnostic =
&AssistResolveStrategy::All, super::diagnostics(&db, &conf, &AssistResolveStrategy::All, file_position.file_id)
file_position.file_id,
)
.pop() .pop()
.expect("no diagnostics"); .expect("no diagnostics");
let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth]; let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth];

View file

@ -11,8 +11,8 @@ use std::{ffi::OsString, iter, path::PathBuf};
use flycheck::FlycheckConfig; use flycheck::FlycheckConfig;
use ide::{ use ide::{
AssistConfig, CompletionConfig, DiagnosticsConfig, HighlightRelatedConfig, HoverConfig, AssistConfig, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode, HighlightRelatedConfig,
HoverDocFormat, InlayHintsConfig, JoinLinesConfig, Snippet, SnippetScope, HoverConfig, HoverDocFormat, InlayHintsConfig, JoinLinesConfig, Snippet, SnippetScope,
}; };
use ide_db::helpers::{ use ide_db::helpers::{
insert_use::{ImportGranularity, InsertUseConfig, PrefixKind}, insert_use::{ImportGranularity, InsertUseConfig, PrefixKind},
@ -45,6 +45,8 @@ use crate::{
// parsing the old name. // parsing the old name.
config_data! { config_data! {
struct ConfigData { struct ConfigData {
/// Placeholder for missing expressions in assists.
assist_exprFillDefault: ExprFillDefaultDef = "\"todo\"",
/// How imports should be grouped into use statements. /// How imports should be grouped into use statements.
assist_importGranularity | assist_importGranularity |
assist_importMergeBehavior | assist_importMergeBehavior |
@ -708,6 +710,10 @@ impl Config {
DiagnosticsConfig { DiagnosticsConfig {
disable_experimental: !self.data.diagnostics_enableExperimental, disable_experimental: !self.data.diagnostics_enableExperimental,
disabled: self.data.diagnostics_disabled.clone(), 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 { pub fn diagnostics_map(&self) -> DiagnosticsMapConfig {
@ -1079,6 +1085,15 @@ enum ManifestOrProjectJson {
ProjectJson(ProjectJsonData), ProjectJson(ProjectJsonData),
} }
#[derive(Deserialize, Debug, Clone)]
#[serde(rename_all = "snake_case")]
pub enum ExprFillDefaultDef {
#[serde(alias = "todo")]
Todo,
#[serde(alias = "default")]
Default,
}
#[derive(Deserialize, Debug, Clone)] #[derive(Deserialize, Debug, Clone)]
#[serde(rename_all = "snake_case")] #[serde(rename_all = "snake_case")]
enum ImportGranularityDef { enum ImportGranularityDef {
@ -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." "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! { "ImportGranularityDef" => set! {
"type": "string", "type": "string",
"enum": ["preserve", "crate", "module", "item"], "enum": ["preserve", "crate", "module", "item"],

View file

@ -59,6 +59,28 @@ pub mod ext {
pub fn expr_todo() -> ast::Expr { pub fn expr_todo() -> ast::Expr {
expr_from_text("todo!()") 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 { pub fn empty_block_expr() -> ast::BlockExpr {
block_expr(None, None) block_expr(None, None)
} }

View file

@ -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"`):: [[rust-analyzer.assist.importGranularity]]rust-analyzer.assist.importGranularity (default: `"crate"`)::
+ +
-- --

View file

@ -378,6 +378,19 @@
"markdownDescription": "Optional settings passed to the debug engine. Example: `{ \"lldb\": { \"terminal\":\"external\"} }`" "markdownDescription": "Optional settings passed to the debug engine. Example: `{ \"lldb\": { \"terminal\":\"external\"} }`"
}, },
"$generated-start": {}, "$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": { "rust-analyzer.assist.importGranularity": {
"markdownDescription": "How imports should be grouped into use statements.", "markdownDescription": "How imports should be grouped into use statements.",
"default": "crate", "default": "crate",