Repository: rust-analyzer/rust-analyzer
Pull request #3376 (2020-02-29 14:36:11)

Fix a common false-positive type mismatch
E.g. for `&{ some_string() }` in a context where a `&str` is expected, we
reported a mismatch inside the block. The problem is that we're passing an
expectation of `str` down, but the expectation is more of a hint in this case.
There's a long comment in rustc about this, which I just copied.

Also, fix reported location for type mismatches in macros.


Patch:
diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs
index 6e1d268dea..569d46cc38 100644
--- a/crates/ra_hir_ty/src/infer.rs
+++ b/crates/ra_hir_ty/src/infer.rs
@@ -583,21 +583,52 @@ impl InferTy {
 #[derive(Clone, PartialEq, Eq, Debug)]
 struct Expectation {
     ty: Ty,
-    // FIXME: In some cases, we need to be aware whether the expectation is that
-    // the type match exactly what we passed, or whether it just needs to be
-    // coercible to the expected type. See Expectation::rvalue_hint in rustc.
+    /// See the `rvalue_hint` method.
+    rvalue_hint: bool,
 }
 
 impl Expectation {
     /// The expectation that the type of the expression needs to equal the given
     /// type.
     fn has_type(ty: Ty) -> Self {
-        Expectation { ty }
+        Expectation { ty, rvalue_hint: false }
+    }
+
+    /// The following explanation is copied straight from rustc:
+    /// Provides an expectation for an rvalue expression given an *optional*
+    /// hint, which is not required for type safety (the resulting type might
+    /// be checked higher up, as is the case with `&expr` and `box expr`), but
+    /// is useful in determining the concrete type.
+    ///
+    /// The primary use case is where the expected type is a fat pointer,
+    /// like `&[isize]`. For example, consider the following statement:
+    ///
+    ///    let x: &[isize] = &[1, 2, 3];
+    ///
+    /// In this case, the expected type for the `&[1, 2, 3]` expression is
+    /// `&[isize]`. If however we were to say that `[1, 2, 3]` has the
+    /// expectation `ExpectHasType([isize])`, that would be too strong --
+    /// `[1, 2, 3]` does not have the type `[isize]` but rather `[isize; 3]`.
+    /// It is only the `&[1, 2, 3]` expression as a whole that can be coerced
+    /// to the type `&[isize]`. Therefore, we propagate this more limited hint,
+    /// which still is useful, because it informs integer literals and the like.
+    /// See the test case `test/ui/coerce-expect-unsized.rs` and #20169
+    /// for examples of where this comes up,.
+    fn rvalue_hint(ty: Ty) -> Self {
+        Expectation { ty, rvalue_hint: true }
     }
 
     /// This expresses no expectation on the type.
     fn none() -> Self {
-        Expectation { ty: Ty::Unknown }
+        Expectation { ty: Ty::Unknown, rvalue_hint: false }
+    }
+
+    fn coercion_target(&self) -> &Ty {
+        if self.rvalue_hint {
+            &Ty::Unknown
+        } else {
+            &self.ty
+        }
     }
 }
 
diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs
index 9d5f756257..3db5b2b515 100644
--- a/crates/ra_hir_ty/src/infer/expr.rs
+++ b/crates/ra_hir_ty/src/infer/expr.rs
@@ -42,14 +42,14 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
     /// Return the type after possible coercion.
     pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty {
         let ty = self.infer_expr_inner(expr, &expected);
-        let ty = if !self.coerce(&ty, &expected.ty) {
+        let ty = if !self.coerce(&ty, &expected.coercion_target()) {
             self.result
                 .type_mismatches
                 .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() });
             // Return actual type when type mismatch.
             // This is needed for diagnostic when return type mismatch.
             ty
-        } else if expected.ty == Ty::Unknown {
+        } else if expected.coercion_target() == &Ty::Unknown {
             ty
         } else {
             expected.ty.clone()
@@ -297,7 +297,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                             // FIXME: throw type error - expected mut reference but found shared ref,
                             // which cannot be coerced
                         }
-                        Expectation::has_type(Ty::clone(exp_inner))
+                        Expectation::rvalue_hint(Ty::clone(exp_inner))
                     } else {
                         Expectation::none()
                     };
@@ -542,7 +542,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         let ty = if let Some(expr) = tail {
             self.infer_expr_coerce(expr, expected)
         } else {
-            self.coerce(&Ty::unit(), &expected.ty);
+            self.coerce(&Ty::unit(), expected.coercion_target());
             Ty::unit()
         };
         if diverges {
diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs
index 4d59db1ee7..d70d34bdc3 100644
--- a/crates/rust-analyzer/src/cli/analysis_stats.rs
+++ b/crates/rust-analyzer/src/cli/analysis_stats.rs
@@ -4,8 +4,8 @@
 use std::{collections::HashSet, fmt::Write, path::Path, time::Instant};
 
 use hir::{
-    db::{DefDatabase, HirDatabase},
-    AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
+    db::{AstDatabase, DefDatabase, HirDatabase},
+    original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
 };
 use hir_def::FunctionId;
 use hir_ty::{Ty, TypeWalk};
@@ -188,13 +188,19 @@ pub fn analysis_stats(
                     let src = sm.expr_syntax(expr_id);
                     if let Some(src) = src {
                         // FIXME: it might be nice to have a function (on Analysis?) that goes from Source<T> -> (LineCol, LineCol) directly
-                        let original_file = src.file_id.original_file(db);
-                        let path = db.file_relative_path(original_file);
-                        let line_index = host.analysis().file_line_index(original_file).unwrap();
-                        let text_range = src.value.either(
-                            |it| it.syntax_node_ptr().range(),
-                            |it| it.syntax_node_ptr().range(),
-                        );
+                        // But also, we should just turn the type mismatches into diagnostics and provide these
+                        let root = db.parse_or_expand(src.file_id).unwrap();
+                        let node = src.map(|e| {
+                            e.either(
+                                |p| p.to_node(&root).syntax().clone(),
+                                |p| p.to_node(&root).syntax().clone(),
+                            )
+                        });
+                        let original_range = original_range(db, node.as_ref());
+                        let path = db.file_relative_path(original_range.file_id);
+                        let line_index =
+                            host.analysis().file_line_index(original_range.file_id).unwrap();
+                        let text_range = original_range.range;
                         let (start, end) = (
                             line_index.line_col(text_range.start()),
                             line_index.line_col(text_range.end()),


Test patch:
diff --git a/crates/ra_hir_ty/src/tests/coercion.rs b/crates/ra_hir_ty/src/tests/coercion.rs
index 60ad6e9be3..1e303f5ce4 100644
--- a/crates/ra_hir_ty/src/tests/coercion.rs
+++ b/crates/ra_hir_ty/src/tests/coercion.rs
@@ -457,6 +457,37 @@ fn test() {
     );
 }
 
+#[test]
+fn coerce_autoderef_block() {
+    assert_snapshot!(
+        infer_with_mismatches(r#"
+struct String {}
+#[lang = "deref"]
+trait Deref { type Target; }
+impl Deref for String { type Target = str; }
+fn takes_ref_str(x: &str) {}
+fn returns_string() -> String { loop {} }
+fn test() {
+    takes_ref_str(&{ returns_string() });
+}
+"#, true),
+        @r###"
+    [127; 128) 'x': &str
+    [136; 138) '{}': ()
+    [169; 180) '{ loop {} }': String
+    [171; 178) 'loop {}': !
+    [176; 178) '{}': ()
+    [191; 236) '{     ... }); }': ()
+    [197; 210) 'takes_ref_str': fn takes_ref_str(&str) -> ()
+    [197; 233) 'takes_...g() })': ()
+    [211; 232) '&{ ret...ng() }': &String
+    [212; 232) '{ retu...ng() }': String
+    [214; 228) 'returns_string': fn returns_string() -> String
+    [214; 230) 'return...ring()': String
+    "###
+    );
+}
+
 #[test]
 fn closure_return_coerce() {
     assert_snapshot!(