about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2022-01-07 14:10:11 +0000
committerGitHub <noreply@github.com>2022-01-07 14:10:11 +0000
commit40009e07d002bf676b4b32e90a858aed37ea4cc2 (patch)
tree03cbb6821b271c292ed6a206c83367d60292be21
parentefb9b89163e4a6080f5d87133028de28cbaf310c (diff)
parent8e0a05eb70d2a3506e1441fb491c0f8b6ae4ac59 (diff)
downloadrust-40009e07d002bf676b4b32e90a858aed37ea4cc2.tar.gz
rust-40009e07d002bf676b4b32e90a858aed37ea4cc2.zip
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 <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Coenen Benjamin <benjamin.coenen@hotmail.com>
-rw-r--r--crates/hir/src/lib.rs29
-rw-r--r--crates/hir_expand/src/name.rs1
-rw-r--r--crates/hir_ty/src/method_resolution.rs1
-rw-r--r--crates/ide/src/lib.rs2
-rw-r--r--crates/ide_diagnostics/src/handlers/missing_fields.rs168
-rw-r--r--crates/ide_diagnostics/src/lib.rs12
-rw-r--r--crates/ide_diagnostics/src/tests.rs16
-rw-r--r--crates/rust-analyzer/src/config.rs27
-rw-r--r--crates/syntax/src/ast/make.rs22
-rw-r--r--docs/user/generated_config.adoc5
-rw-r--r--editors/code/package.json13
11 files changed, 271 insertions, 25 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index c6d401a6d65..3b28b14668e 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 23f81d3c7f5..3636ab62131 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 cb6b6ec39fb..1a451ae79f5 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 16a2b4a1130..b8482d0222b 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 8d17a7e714a..186bfc3d584 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 c921d989bd3..86d76751ad6 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 a2b92c4ff91..7d06e9d36ff 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 de0aba0caa5..88d86ef5038 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 e1938307cf3..35750209754 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 f46dda2351f..88dcacfe833 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 d4188684748..2c7d6c3773e 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",