Merge #11842

11842: Fix duplicate type mismatches with blocks r=flodiebold a=flodiebold

E.g. when there's a type mismatch on the return value of a function. To fix this, we have to return the expected type as the type of the block when there's a mismatch. That meant some IDE code that expected otherwise had to be adapted, in particular the "add return type" assist. For the "wrap in Ok/Some" quickfix, this sadly means it usually can't be applied in all branches of an if expression at the same time anymore, because there's a type mismatch for each branch that has the wrong type.

Co-authored-by: Florian Diebold <flodiebold@gmail.com>

diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs
index 238aff75c45ed8ad780baf5425f0b3b449aa4f1b..0b67f2c32e58d7e2c0336054f29d1c9d97999d30 100644 (file)
--- a/crates/hir_ty/src/infer/expr.rs
+++ b/crates/hir_ty/src/infer/expr.rs
@@ -69,12 +69,11 @@ pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation)
             match self.coerce(Some(expr), &ty, &target) {
                 Ok(res) => res,
                 Err(_) => {
-                    self.result
-                        .type_mismatches
-                        .insert(expr.into(), TypeMismatch { expected: target, actual: ty.clone() });
-                    // Return actual type when type mismatch.
-                    // This is needed for diagnostic when return type mismatch.
-                    ty
+                    self.result.type_mismatches.insert(
+                        expr.into(),
+                        TypeMismatch { expected: target.clone(), actual: ty.clone() },
+                    );
+                    target
                 }
             }
         } else {
@@ -914,9 +913,16 @@ fn infer_block(
                 self.table.new_maybe_never_var()
             } else {
                 if let Some(t) = expected.only_has_type(&mut self.table) {
-                    let _ = self.coerce(Some(expr), &TyBuilder::unit(), &t);
+                    if self.coerce(Some(expr), &TyBuilder::unit(), &t).is_err() {
+                        self.result.type_mismatches.insert(
+                            expr.into(),
+                            TypeMismatch { expected: t.clone(), actual: TyBuilder::unit() },
+                        );
+                    }
+                    t
+                } else {
+                    TyBuilder::unit()
                 }
-                TyBuilder::unit()
             }
         }
     }

diff --git a/crates/hir_ty/src/tests.rs b/crates/hir_ty/src/tests.rs
index 18523d2db599d01e065bbc52b63ca343bdf19e9f..d2f13e4351c73b17cca725446ddfcbd772988e4f 100644 (file)
--- a/crates/hir_ty/src/tests.rs
+++ b/crates/hir_ty/src/tests.rs
@@ -8,6 +8,7 @@
 mod macros;
 mod display_source_code;
 mod incremental;
+mod diagnostics;
 
 use std::{collections::HashMap, env, sync::Arc};
 

diff --git a/crates/hir_ty/src/tests/coercion.rs b/crates/hir_ty/src/tests/coercion.rs
index c0dddb608ea394e75c4c057aa01c9db2c00e280d..268faf8cb3a0643a7fb842f604af28213673393b 100644 (file)
--- a/crates/hir_ty/src/tests/coercion.rs
+++ b/crates/hir_ty/src/tests/coercion.rs
@@ -2,12 +2,10 @@
 
 #[test]
 fn block_expr_type_mismatch() {
-    // FIXME fix double type mismatch
     check(
         r"
 fn test() {
     let a: i32 = { 1i64 };
-              // ^^^^^^^^ expected i32, got i64
                 // ^^^^ expected i32, got i64
 }
         ",

diff --git a/crates/hir_ty/src/tests/diagnostics.rs b/crates/hir_ty/src/tests/diagnostics.rs
new file mode 100644 (file)
index 0000000..f00fa97
--- /dev/null
+++ b/crates/hir_ty/src/tests/diagnostics.rs
@@ -0,0 +1,75 @@
+use super::check;
+
+#[test]
+fn function_return_type_mismatch_1() {
+    check(
+        r#"
+fn test() -> &'static str {
+    5
+  //^ expected &str, got i32
+}
+"#,
+    );
+}
+
+#[test]
+fn function_return_type_mismatch_2() {
+    check(
+        r#"
+fn test(x: bool) -> &'static str {
+    if x {
+        return 1;
+             //^ expected &str, got i32
+    }
+    "ok"
+}
+"#,
+    );
+}
+
+#[test]
+fn function_return_type_mismatch_3() {
+    check(
+        r#"
+fn test(x: bool) -> &'static str {
+    if x {
+        return "ok";
+    }
+    1
+  //^ expected &str, got i32
+}
+"#,
+    );
+}
+
+#[test]
+fn function_return_type_mismatch_4() {
+    check(
+        r#"
+fn test(x: bool) -> &'static str {
+    if x {
+        "ok"
+    } else {
+        1
+      //^ expected &str, got i32
+    }
+}
+"#,
+    );
+}
+
+#[test]
+fn function_return_type_mismatch_5() {
+    check(
+        r#"
+fn test(x: bool) -> &'static str {
+    if x {
+        1
+      //^ expected &str, got i32
+    } else {
+        "ok"
+    }
+}
+"#,
+    );
+}

diff --git a/crates/hir_ty/src/tests/never_type.rs b/crates/hir_ty/src/tests/never_type.rs
index 8be2b8ec1088df44e63aea3f55a10b1ae2d9ac5d..3ca0a5b391d060e42b0fcacf77238c5bfdacfd5d 100644 (file)
--- a/crates/hir_ty/src/tests/never_type.rs
+++ b/crates/hir_ty/src/tests/never_type.rs
@@ -314,11 +314,10 @@ fn test1() {
         expect![[r#"
             11..84 '{     ..." }; }': ()
             54..55 'x': u32
-            63..81 '{ loop...foo" }': &str
+            63..81 '{ loop...foo" }': u32
             65..72 'loop {}': !
             70..72 '{}': ()
             74..79 '"foo"': &str
-            63..81: expected u32, got &str
             74..79: expected u32, got &str
         "#]],
     );
@@ -350,31 +349,30 @@ fn test3() {
             let x: u32 = { while true { return; }; };
         }
         ",
-        expect![[r"
+        expect![[r#"
             11..85 '{     ...} }; }': ()
             54..55 'x': u32
-            63..82 '{ loop...k; } }': ()
+            63..82 '{ loop...k; } }': u32
             65..80 'loop { break; }': ()
             70..80 '{ break; }': ()
             72..77 'break': !
-            63..82: expected u32, got ()
             65..80: expected u32, got ()
             97..343 '{     ...; }; }': ()
             140..141 'x': u32
-            149..175 '{ for ...; }; }': ()
+            149..175 '{ for ...; }; }': u32
             151..172 'for a ...eak; }': ()
             155..156 'a': {unknown}
             160..161 'b': {unknown}
             162..172 '{ break; }': ()
             164..169 'break': !
             226..227 'x': u32
-            235..253 '{ for ... {}; }': ()
+            235..253 '{ for ... {}; }': u32
             237..250 'for a in b {}': ()
             241..242 'a': {unknown}
             246..247 'b': {unknown}
             248..250 '{}': ()
             304..305 'x': u32
-            313..340 '{ for ...; }; }': ()
+            313..340 '{ for ...; }; }': u32
             315..337 'for a ...urn; }': ()
             319..320 'a': {unknown}
             324..325 'b': {unknown}
@@ -385,18 +383,18 @@ fn test3() {
             313..340: expected u32, got ()
             355..654 '{     ...; }; }': ()
             398..399 'x': u32
-            407..433 '{ whil...; }; }': ()
+            407..433 '{ whil...; }; }': u32
             409..430 'while ...eak; }': ()
             415..419 'true': bool
             420..430 '{ break; }': ()
             422..427 'break': !
             537..538 'x': u32
-            546..564 '{ whil... {}; }': ()
+            546..564 '{ whil... {}; }': u32
             548..561 'while true {}': ()
             554..558 'true': bool
             559..561 '{}': ()
             615..616 'x': u32
-            624..651 '{ whil...; }; }': ()
+            624..651 '{ whil...; }; }': u32
             626..648 'while ...urn; }': ()
             632..636 'true': bool
             637..648 '{ return; }': ()
@@ -404,7 +402,7 @@ fn test3() {
             407..433: expected u32, got ()
             546..564: expected u32, got ()
             624..651: expected u32, got ()
-        "]],
+        "#]],
     );
 }
 
@@ -438,7 +436,7 @@ fn f() {
             17..18 '1': i32
             17..18 '1': i32
             21..22 '2': i32
-            28..30 '{}': ()
+            28..30 '{}': !
             28..30: expected !, got ()
         "#]],
     );

diff --git a/crates/hir_ty/src/tests/regression.rs b/crates/hir_ty/src/tests/regression.rs
index 14c6af943abcd827a7ead2041c5d37667ce8fb84..2809b1e912a8dd409983aeb85a1fa4cc4cd4d275 100644 (file)
--- a/crates/hir_ty/src/tests/regression.rs
+++ b/crates/hir_ty/src/tests/regression.rs
@@ -367,7 +367,7 @@ pub fn main_loop() {
         }
         "#,
         expect![[r#"
-            143..145 '{}': ()
+            143..145 '{}': HashSet<T, H>
             168..197 '{     ...t(); }': ()
             174..192 'FxHash...efault': fn default<{unknown}, FxHasher>() -> HashSet<{unknown}, FxHasher>
             174..194 'FxHash...ault()': HashSet<{unknown}, FxHasher>
@@ -831,7 +831,7 @@ fn main() {
         "#,
         expect![[r#"
             225..229 'iter': T
-            244..246 '{}': ()
+            244..246 '{}': Vec<A>
             258..402 '{     ...r(); }': ()
             268..273 'inner': Map<|&f64| -> f64>
             276..300 'Map { ... 0.0 }': Map<|&f64| -> f64>
@@ -914,7 +914,7 @@ fn flush(&self) {
 "#,
         expect![[r#"
             123..127 'self': &Mutex<T>
-            150..152 '{}': ()
+            150..152 '{}': MutexGuard<T>
             234..238 'self': &{unknown}
             240..290 '{     ...()); }': ()
             250..251 'w': &Mutex<BufWriter>
@@ -1039,18 +1039,18 @@ fn no_actual_tail(){
         }
         "#,
         expect![[r#"
-            14..53 '{     ...)] 9 }': &str
-            20..31 '{ "first" }': &str
+            14..53 '{     ...)] 9 }': ()
+            20..31 '{ "first" }': ()
             22..29 '"first"': &str
-            72..190 '{     ...] 13 }': &str
+            72..190 '{     ...] 13 }': ()
             78..88 '{ "fake" }': &str
             80..86 '"fake"': &str
             93..103 '{ "fake" }': &str
             95..101 '"fake"': &str
-            108..120 '{ "second" }': &str
+            108..120 '{ "second" }': ()
             110..118 '"second"': &str
-            210..273 '{     ... 15; }': &str
-            216..227 '{ "third" }': &str
+            210..273 '{     ... 15; }': ()
+            216..227 '{ "third" }': ()
             218..225 '"third"': &str
             293..357 '{     ...] 15 }': ()
             299..311 '{ "fourth" }': &str

diff --git a/crates/hir_ty/src/tests/simple.rs b/crates/hir_ty/src/tests/simple.rs
index 675f9038f0961f616f63d33293fb7dd00d7efb9b..df7b3df3d5b2f1da8c9a0797a3d28ffbf89cffe7 100644 (file)
--- a/crates/hir_ty/src/tests/simple.rs
+++ b/crates/hir_ty/src/tests/simple.rs
@@ -996,9 +996,9 @@ fn main(foo: Foo) {
             50..106 'if tru...     }': ()
             53..57 'true': bool
             58..66 '{      }': ()
-            72..106 'if fal...     }': i32
+            72..106 'if fal...     }': ()
             75..80 'false': bool
-            81..106 '{     ...     }': i32
+            81..106 '{     ...     }': ()
             91..94 'foo': Foo
             91..100 'foo.field': i32
         "#]],
@@ -1094,10 +1094,10 @@ fn test(a: A) {
         expect![[r#"
             31..35 'self': A
             37..38 'x': u32
-            52..54 '{}': ()
+            52..54 '{}': i32
             106..110 'self': &A
             112..113 'x': u64
-            127..129 '{}': ()
+            127..129 '{}': i64
             147..148 'a': A
             153..201 '{     ...(1); }': ()
             159..160 'a': A
@@ -1129,7 +1129,7 @@ fn test() {
         "#,
         expect![[r#"
             39..43 'self': &str
-            52..54 '{}': ()
+            52..54 '{}': i32
             68..88 '{     ...o(); }': ()
             74..79 '"foo"': &str
             74..85 '"foo".foo()': i32
@@ -1419,7 +1419,7 @@ fn test() -> i128 {
             206..210 'self': A<X, Y>
             206..212 'self.y': Y
             214..215 't': T
-            244..341 '{     ...(1); }': ()
+            244..341 '{     ...(1); }': i128
             254..255 'a': A<u64, i64>
             258..280 'A { x:...1i64 }': A<u64, i64>
             265..269 '1u64': u64
@@ -1456,7 +1456,7 @@ fn test(o: Option<u32>) {
         "#,
         expect![[r#"
             77..81 'self': &Option<T>
-            97..99 '{}': ()
+            97..99 '{}': Option<&T>
             110..111 'o': Option<u32>
             126..164 '{     ...f(); }': ()
             132..145 '(&o).as_ref()': Option<&u32>
@@ -1852,7 +1852,7 @@ fn foo() -> u32 {
         }
         "#,
         expect![[r#"
-            16..58 '{     ...; }; }': ()
+            16..58 '{     ...; }; }': u32
             26..27 'x': || -> usize
             30..55 '|| -> ...n 1; }': || -> usize
             42..55 '{ return 1; }': usize
@@ -1871,7 +1871,7 @@ fn foo() -> u32 {
         }
         "#,
         expect![[r#"
-            16..47 '{     ...; }; }': ()
+            16..47 '{     ...; }; }': u32
             26..27 'x': || -> ()
             30..44 '|| { return; }': || -> ()
             33..44 '{ return; }': ()
@@ -1889,7 +1889,7 @@ fn foo() -> u32 {
         }
         "#,
         expect![[r#"
-            16..46 '{     ..." }; }': ()
+            16..46 '{     ..." }; }': u32
             26..27 'x': || -> &str
             30..43 '|| { "test" }': || -> &str
             33..43 '{ "test" }': &str
@@ -2628,11 +2628,11 @@ fn main() {
         expect![[r#"
             104..108 'self': &Box<T>
             188..192 'self': &Box<Foo<T>>
-            218..220 '{}': ()
+            218..220 '{}': &T
             242..246 'self': &Box<Foo<T>>
-            275..277 '{}': ()
+            275..277 '{}': &Foo<T>
             297..301 'self': Box<Foo<T>>
-            322..324 '{}': ()
+            322..324 '{}': Foo<T>
             338..559 '{     ...r(); }': ()
             348..353 'boxed': Box<Foo<i32>>
             356..359 'Box': Box<Foo<i32>>(Foo<i32>) -> Box<Foo<i32>>

diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs
index a82b8cb466fd41f54a07a6bd46fc9dfa55ce5303..5e58d5ad8381cb7325bf531f43d6e7722b2be334 100644 (file)
--- a/crates/hir_ty/src/tests/traits.rs
+++ b/crates/hir_ty/src/tests/traits.rs
@@ -1280,7 +1280,7 @@ fn test(x: dyn Trait<u64>, y: &dyn Trait<u64>) {
         expect![[r#"
             29..33 'self': &Self
             54..58 'self': &Self
-            97..99 '{}': ()
+            97..99 '{}': dyn Trait<u64>
             109..110 'x': dyn Trait<u64>
             128..129 'y': &dyn Trait<u64>
             148..265 '{     ...2(); }': ()
@@ -1361,10 +1361,10 @@ fn test(x: Trait, y: &Trait) -> u64 {
 }"#,
         expect![[r#"
             26..30 'self': &Self
-            60..62 '{}': ()
+            60..62 '{}': dyn Trait
             72..73 'x': dyn Trait
             82..83 'y': &dyn Trait
-            100..175 '{     ...o(); }': ()
+            100..175 '{     ...o(); }': u64
             106..107 'x': dyn Trait
             113..114 'y': &dyn Trait
             124..125 'z': dyn Trait
@@ -1449,9 +1449,9 @@ fn test<T: Trait<Type = u32>>(x: T, y: impl Trait<Type = i64>) {
 }"#,
         expect![[r#"
             49..50 't': T
-            77..79 '{}': ()
+            77..79 '{}': Trait::Type<T>
             111..112 't': T
-            122..124 '{}': ()
+            122..124 '{}': U
             154..155 't': T
             165..168 '{t}': T
             166..167 't': T
@@ -1575,7 +1575,7 @@ fn test<T: Trait1, U: Trait2>(x: T, y: U) {
 }"#,
         expect![[r#"
             49..53 'self': &Self
-            62..64 '{}': ()
+            62..64 '{}': u32
             181..182 'x': T
             187..188 'y': U
             193..222 '{     ...o(); }': ()
@@ -1604,7 +1604,7 @@ fn test(x: &impl Trait1) {
 }"#,
         expect![[r#"
             49..53 'self': &Self
-            62..64 '{}': ()
+            62..64 '{}': u32
             115..116 'x': &impl Trait1
             132..148 '{     ...o(); }': ()
             138..139 'x': &impl Trait1
@@ -1653,7 +1653,7 @@ fn test() {
 }"#,
         expect![[r#"
             102..103 't': T
-            113..115 '{}': ()
+            113..115 '{}': U
             145..146 't': T
             156..159 '{t}': T
             157..158 't': T
@@ -1786,9 +1786,9 @@ fn make_foo_fn() -> Foo {}
 }"#,
         expect![[r#"
             36..40 'self': &Foo
-            51..53 '{}': ()
+            51..53 '{}': usize
             131..132 'f': F
-            151..153 '{}': ()
+            151..153 '{}': Lazy<T, F>
             251..497 '{     ...o(); }': ()
             261..266 'lazy1': Lazy<Foo, || -> Foo>
             283..292 'Lazy::new': fn new<Foo, || -> Foo>(|| -> Foo) -> Lazy<Foo, || -> Foo>
@@ -1807,7 +1807,7 @@ fn make_foo_fn() -> Foo {}
             478..480 'r2': usize
             483..488 'lazy2': Lazy<Foo, fn() -> Foo>
             483..494 'lazy2.foo()': usize
-            357..359 '{}': ()
+            357..359 '{}': Foo
         "#]],
     );
 }
@@ -2738,7 +2738,7 @@ fn test() {
         expect![[r#"
             9..11 '{}': ()
             28..29 'T': {unknown}
-            36..38 '{}': ()
+            36..38 '{}': T
             36..38: expected T, got ()
             113..117 'self': &Self
             169..249 '{     ...t(); }': ()
@@ -3167,7 +3167,7 @@ fn inner() -> S {
 }"#,
         expect![[r#"
             17..73 '{     ...   } }': ()
-            39..71 '{     ...     }': ()
+            39..71 '{     ...     }': S
             53..54 's': S
             57..62 'inner': fn inner() -> S
             57..64 'inner()': S

diff --git a/crates/ide_assists/src/handlers/add_return_type.rs b/crates/ide_assists/src/handlers/add_return_type.rs
index 2c5b61eddb769c06ddfa3c2eb4ff9ad477723433..c7172741e46de527b0f111336768fe7286bd5a6a 100644 (file)
--- a/crates/ide_assists/src/handlers/add_return_type.rs
+++ b/crates/ide_assists/src/handlers/add_return_type.rs
@@ -1,5 +1,5 @@
 use hir::HirDisplay;
-use syntax::{ast, AstNode, SyntaxKind, SyntaxToken, TextRange, TextSize};
+use syntax::{ast, match_ast, AstNode, SyntaxKind, SyntaxToken, TextRange, TextSize};
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
 
@@ -18,7 +18,7 @@
 pub(crate) fn add_return_type(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let (fn_type, tail_expr, builder_edit_pos) = extract_tail(ctx)?;
     let module = ctx.sema.scope(tail_expr.syntax()).module()?;
-    let ty = ctx.sema.type_of_expr(&tail_expr)?.adjusted();
+    let ty = ctx.sema.type_of_expr(&peel_blocks(tail_expr.clone()))?.original();
     if ty.is_unit() {
         return None;
     }
@@ -93,6 +93,45 @@ enum FnType {
     Closure { wrap_expr: bool },
 }
 
+/// If we're looking at a block that is supposed to return `()`, type inference
+/// will just tell us it has type `()`. We have to look at the tail expression
+/// to see the mismatched actual type. This 'unpeels' the various blocks to
+/// hopefully let us see the type the user intends. (This still doesn't handle
+/// all situations fully correctly; the 'ideal' way to handle this would be to
+/// run type inference on the function again, but with a variable as the return
+/// type.)
+fn peel_blocks(mut expr: ast::Expr) -> ast::Expr {
+    loop {
+        match_ast! {
+            match (expr.syntax()) {
+                ast::BlockExpr(it) => {
+                    if let Some(tail) = it.tail_expr() {
+                        expr = tail.clone();
+                    } else {
+                        break;
+                    }
+                },
+                ast::IfExpr(it) => {
+                    if let Some(then_branch) = it.then_branch() {
+                        expr = ast::Expr::BlockExpr(then_branch.clone());
+                    } else {
+                        break;
+                    }
+                },
+                ast::MatchExpr(it) => {
+                    if let Some(arm_expr) = it.match_arm_list().and_then(|l| l.arms().next()).and_then(|a| a.expr()) {
+                        expr = arm_expr;
+                    } else {
+                        break;
+                    }
+                },
+                _ => break,
+            }
+        }
+    }
+    expr
+}
+
 fn extract_tail(ctx: &AssistContext) -> Option<(FnType, ast::Expr, InsertOrReplace)> {
     let (fn_type, tail_expr, return_type_range, action) =
         if let Some(closure) = ctx.find_node_at_offset::<ast::ClosureExpr>() {
@@ -248,6 +287,25 @@ fn infer_return_type_nested() {
         );
     }
 
+    #[test]
+    fn infer_return_type_nested_match() {
+        check_assist(
+            add_return_type,
+            r#"fn foo() {
+    match true {
+        true => { 3$0 },
+        false => { 5 },
+    }
+}"#,
+            r#"fn foo() -> i32 {
+    match true {
+        true => { 3 },
+        false => { 5 },
+    }
+}"#,
+        );
+    }
+
     #[test]
     fn not_applicable_ret_type_specified() {
         cov_mark::check!(existing_ret_type);

diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs
index 8352774be4265f210afd7432ff9905a5907cbee4..fcb4aaf065d4bab0269d46b4aa9f2e678c0da7b4 100644 (file)
--- a/crates/ide_assists/src/handlers/extract_function.rs
+++ b/crates/ide_assists/src/handlers/extract_function.rs
@@ -4162,7 +4162,7 @@ fn main() {
     match 6 {
         100 => $0{ 100 }$0
         _ => 0,
-    }
+    };
 }
 "#,
             r#"
@@ -4170,7 +4170,7 @@ fn main() {
     match 6 {
         100 => fun_name(),
         _ => 0,
-    }
+    };
 }
 
 fn $0fun_name() -> i32 {
@@ -4185,7 +4185,7 @@ fn main() {
     match 6 {
         100 => $0{ 100 }$0,
         _ => 0,
-    }
+    };
 }
 "#,
             r#"
@@ -4193,7 +4193,7 @@ fn main() {
     match 6 {
         100 => fun_name(),
         _ => 0,
-    }
+    };
 }
 
 fn $0fun_name() -> i32 {

diff --git a/crates/ide_diagnostics/src/handlers/type_mismatch.rs b/crates/ide_diagnostics/src/handlers/type_mismatch.rs
index 040dcbd1d958cdb9128be2bdd1579825e444127b..8ae9a0be5380499b9caec9bb83d909c629b4d346 100644 (file)
--- a/crates/ide_diagnostics/src/handlers/type_mismatch.rs
+++ b/crates/ide_diagnostics/src/handlers/type_mismatch.rs
@@ -1,8 +1,5 @@
-use hir::{db::AstDatabase, HirDisplay, Type, TypeInfo};
-use ide_db::{
-    famous_defs::FamousDefs, source_change::SourceChange,
-    syntax_helpers::node_ext::for_each_tail_expr,
-};
+use hir::{db::AstDatabase, HirDisplay, Type};
+use ide_db::{famous_defs::FamousDefs, source_change::SourceChange};
 use syntax::{
     ast::{BlockExpr, ExprStmt},
     AstNode,
@@ -77,9 +74,9 @@ fn add_missing_ok_or_some(
     acc: &mut Vec<Assist>,
 ) -> Option<()> {
     let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
-    let tail_expr = d.expr.value.to_node(&root);
-    let tail_expr_range = tail_expr.syntax().text_range();
-    let scope = ctx.sema.scope(tail_expr.syntax());
+    let expr = d.expr.value.to_node(&root);
+    let expr_range = expr.syntax().text_range();
+    let scope = ctx.sema.scope(expr.syntax());
 
     let expected_adt = d.expected.as_adt()?;
     let expected_enum = expected_adt.as_enum()?;
@@ -101,16 +98,12 @@ fn add_missing_ok_or_some(
     }
 
     let mut builder = TextEdit::builder();
-    for_each_tail_expr(&tail_expr, &mut |expr| {
-        if ctx.sema.type_of_expr(expr).map(TypeInfo::adjusted).as_ref() != Some(&d.expected) {
-            builder.insert(expr.syntax().text_range().start(), format!("{}(", variant_name));
-            builder.insert(expr.syntax().text_range().end(), ")".to_string());
-        }
-    });
+    builder.insert(expr.syntax().text_range().start(), format!("{}(", variant_name));
+    builder.insert(expr.syntax().text_range().end(), ")".to_string());
     let source_change =
         SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), builder.finish());
     let name = format!("Wrap in {}", variant_name);
-    acc.push(fix("wrap_tail_expr", &name, source_change, tail_expr_range));
+    acc.push(fix("wrap_in_constructor", &name, source_change, expr_range));
     Some(())
 }
 
@@ -330,12 +323,12 @@ fn test_wrap_return_type_option_tails() {
 //- minicore: option, result
 fn div(x: i32, y: i32) -> Option<i32> {
     if y == 0 {
-        0
+        Some(0)
     } else if true {
-        100
+        100$0
     } else {
         None
-    }$0
+    }
 }
 "#,
             r#"

diff --git a/crates/ide_diagnostics/src/tests.rs b/crates/ide_diagnostics/src/tests.rs
index 7d06e9d36fffeb6dfb1b94506ce152dee1a9e1f1..7cd79c7ceef58f703cd61f172d28fc91e96c3c85 100644 (file)
--- a/crates/ide_diagnostics/src/tests.rs
+++ b/crates/ide_diagnostics/src/tests.rs
@@ -54,13 +54,13 @@ fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) {
         actual
     };
 
-    assert_eq_text!(&after, &actual);
     assert!(
         fix.target.contains_inclusive(file_position.offset),
         "diagnostic fix range {:?} does not touch cursor position {:?}",
         fix.target,
         file_position.offset
     );
+    assert_eq_text!(&after, &actual);
 }
 
 /// Checks that there's a diagnostic *without* fix at `$0`.