Repository: rust-analyzer/rust-analyzer
Pull request #4765 (2020-06-05 17:28:57)

Fix type parameter defaults
They should not be applied in expression or pattern contexts, unless there are other explicitly given type args.

(The existing tests about this were actually wrong.)


Patch:
diff --git a/crates/ra_assists/src/handlers/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs
index ab20c66493..90b06a6259 100644
--- a/crates/ra_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ra_assists/src/handlers/add_explicit_type.rs
@@ -195,7 +195,7 @@ struct Test<K, T = u8> {
 }
 
 fn main() {
-    let test<|> = Test { t: 23, k: 33 };
+    let test<|> = Test { t: 23u8, k: 33 };
 }"#,
             r#"
 struct Test<K, T = u8> {
@@ -204,7 +204,7 @@ struct Test<K, T = u8> {
 }
 
 fn main() {
-    let test: Test<i32> = Test { t: 23, k: 33 };
+    let test: Test<i32> = Test { t: 23u8, k: 33 };
 }"#,
         );
     }
diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs
index d9bf3c2f0f..2e16e5120d 100644
--- a/crates/ra_hir_ty/src/infer.rs
+++ b/crates/ra_hir_ty/src/infer.rs
@@ -439,13 +439,13 @@ impl<'a> InferenceContext<'a> {
             };
         return match resolution {
             TypeNs::AdtId(AdtId::StructId(strukt)) => {
-                let substs = Ty::substs_from_path(&ctx, path, strukt.into());
+                let substs = Ty::substs_from_path(&ctx, path, strukt.into(), true);
                 let ty = self.db.ty(strukt.into());
                 let ty = self.insert_type_vars(ty.subst(&substs));
                 forbid_unresolved_segments((ty, Some(strukt.into())), unresolved)
             }
             TypeNs::EnumVariantId(var) => {
-                let substs = Ty::substs_from_path(&ctx, path, var.into());
+                let substs = Ty::substs_from_path(&ctx, path, var.into(), true);
                 let ty = self.db.ty(var.parent.into());
                 let ty = self.insert_type_vars(ty.subst(&substs));
                 forbid_unresolved_segments((ty, Some(var.into())), unresolved)
diff --git a/crates/ra_hir_ty/src/infer/path.rs b/crates/ra_hir_ty/src/infer/path.rs
index 1c2e56fb0d..1ad0d83971 100644
--- a/crates/ra_hir_ty/src/infer/path.rs
+++ b/crates/ra_hir_ty/src/infer/path.rs
@@ -95,7 +95,7 @@ impl<'a> InferenceContext<'a> {
         // self_subst is just for the parent
         let parent_substs = self_subst.unwrap_or_else(Substs::empty);
         let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver);
-        let substs = Ty::substs_from_path(&ctx, path, typable);
+        let substs = Ty::substs_from_path(&ctx, path, typable, true);
         let full_substs = Substs::builder(substs.len())
             .use_parent_substs(&parent_substs)
             .fill(substs.0[parent_substs.len()..].iter().cloned())
@@ -141,6 +141,7 @@ impl<'a> InferenceContext<'a> {
                     def,
                     resolved_segment,
                     remaining_segments_for_ty,
+                    true,
                 );
                 if let Ty::Unknown = ty {
                     return None;
diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs
index a05cbd7fcf..42713928f4 100644
--- a/crates/ra_hir_ty/src/lower.rs
+++ b/crates/ra_hir_ty/src/lower.rs
@@ -323,6 +323,7 @@ impl Ty {
         resolution: TypeNs,
         resolved_segment: PathSegment<'_>,
         remaining_segments: PathSegments<'_>,
+        infer_args: bool,
     ) -> (Ty, Option<TypeNs>) {
         let ty = match resolution {
             TypeNs::TraitId(trait_) => {
@@ -400,9 +401,15 @@ impl Ty {
                 ctx.db.ty(adt.into()).subst(&substs)
             }
 
-            TypeNs::AdtId(it) => Ty::from_hir_path_inner(ctx, resolved_segment, it.into()),
-            TypeNs::BuiltinType(it) => Ty::from_hir_path_inner(ctx, resolved_segment, it.into()),
-            TypeNs::TypeAliasId(it) => Ty::from_hir_path_inner(ctx, resolved_segment, it.into()),
+            TypeNs::AdtId(it) => {
+                Ty::from_hir_path_inner(ctx, resolved_segment, it.into(), infer_args)
+            }
+            TypeNs::BuiltinType(it) => {
+                Ty::from_hir_path_inner(ctx, resolved_segment, it.into(), infer_args)
+            }
+            TypeNs::TypeAliasId(it) => {
+                Ty::from_hir_path_inner(ctx, resolved_segment, it.into(), infer_args)
+            }
             // FIXME: report error
             TypeNs::EnumVariantId(_) => return (Ty::Unknown, None),
         };
@@ -428,7 +435,13 @@ impl Ty {
             ),
             Some(i) => (path.segments().get(i - 1).unwrap(), path.segments().skip(i)),
         };
-        Ty::from_partly_resolved_hir_path(ctx, resolution, resolved_segment, remaining_segments)
+        Ty::from_partly_resolved_hir_path(
+            ctx,
+            resolution,
+            resolved_segment,
+            remaining_segments,
+            false,
+        )
     }
 
     fn select_associated_type(
@@ -474,13 +487,14 @@ impl Ty {
         ctx: &TyLoweringContext<'_>,
         segment: PathSegment<'_>,
         typable: TyDefId,
+        infer_args: bool,
     ) -> Ty {
         let generic_def = match typable {
             TyDefId::BuiltinType(_) => None,
             TyDefId::AdtId(it) => Some(it.into()),
             TyDefId::TypeAliasId(it) => Some(it.into()),
         };
-        let substs = substs_from_path_segment(ctx, segment, generic_def, false);
+        let substs = substs_from_path_segment(ctx, segment, generic_def, infer_args);
         ctx.db.ty(typable).subst(&substs)
     }
 
@@ -493,6 +507,7 @@ impl Ty {
         // `ValueTyDefId` is just a convenient way to pass generics and
         // special-case enum variants
         resolved: ValueTyDefId,
+        infer_args: bool,
     ) -> Substs {
         let last = path.segments().last().expect("path should have at least one segment");
         let (segment, generic_def) = match resolved {
@@ -515,22 +530,27 @@ impl Ty {
                 (segment, Some(var.parent.into()))
             }
         };
-        substs_from_path_segment(ctx, segment, generic_def, false)
+        substs_from_path_segment(ctx, segment, generic_def, infer_args)
     }
 }
 
-pub(super) fn substs_from_path_segment(
+fn substs_from_path_segment(
     ctx: &TyLoweringContext<'_>,
     segment: PathSegment<'_>,
     def_generic: Option<GenericDefId>,
-    _add_self_param: bool,
+    infer_args: bool,
 ) -> Substs {
     let mut substs = Vec::new();
     let def_generics = def_generic.map(|def| generics(ctx.db.upcast(), def));
 
     let (parent_params, self_params, type_params, impl_trait_params) =
         def_generics.map_or((0, 0, 0, 0), |g| g.provenance_split());
+    let total_len = parent_params + self_params + type_params + impl_trait_params;
+
     substs.extend(iter::repeat(Ty::Unknown).take(parent_params));
+
+    let mut had_explicit_args = false;
+
     if let Some(generic_args) = &segment.args_and_bindings {
         if !generic_args.has_self_type {
             substs.extend(iter::repeat(Ty::Unknown).take(self_params));
@@ -542,31 +562,35 @@ pub(super) fn substs_from_path_segment(
         for arg in generic_args.args.iter().skip(skip).take(expected_num) {
             match arg {
                 GenericArg::Type(type_ref) => {
+                    had_explicit_args = true;
                     let ty = Ty::from_hir(ctx, type_ref);
                     substs.push(ty);
                 }
             }
         }
     }
-    let total_len = parent_params + self_params + type_params + impl_trait_params;
-    // add placeholders for args that were not provided
-    for _ in substs.len()..total_len {
-        substs.push(Ty::Unknown);
-    }
-    assert_eq!(substs.len(), total_len);
 
-    // handle defaults
-    if let Some(def_generic) = def_generic {
-        let default_substs = ctx.db.generic_defaults(def_generic);
-        assert_eq!(substs.len(), default_substs.len());
+    // handle defaults. In expression or pattern path segments without
+    // explicitly specified type arguments, missing type arguments are inferred
+    // (i.e. defaults aren't used).
+    if !infer_args || had_explicit_args {
+        if let Some(def_generic) = def_generic {
+            let default_substs = ctx.db.generic_defaults(def_generic);
+            assert_eq!(total_len, default_substs.len());
 
-        for (i, default_ty) in default_substs.iter().enumerate() {
-            if substs[i] == Ty::Unknown {
-                substs[i] = default_ty.clone();
+            for default_ty in default_substs.iter().skip(substs.len()) {
+                substs.push(default_ty.clone());
             }
         }
     }
 
+    // add placeholders for args that were not provided
+    // FIXME: emit diagnostics in contexts where this is not allowed
+    for _ in substs.len()..total_len {
+        substs.push(Ty::Unknown);
+    }
+    assert_eq!(substs.len(), total_len);
+
     Substs(substs.into())
 }
 
@@ -615,9 +639,7 @@ impl TraitRef {
         segment: PathSegment<'_>,
         resolved: TraitId,
     ) -> Substs {
-        let has_self_param =
-            segment.args_and_bindings.as_ref().map(|a| a.has_self_type).unwrap_or(false);
-        substs_from_path_segment(ctx, segment, Some(resolved.into()), !has_self_param)
+        substs_from_path_segment(ctx, segment, Some(resolved.into()), false)
     }
 
     pub(crate) fn from_type_bound(
diff --git a/crates/ra_ide/src/hover.rs b/crates/ra_ide/src/hover.rs
index 62df074594..846d8c69ba 100644
--- a/crates/ra_ide/src/hover.rs
+++ b/crates/ra_ide/src/hover.rs
@@ -529,7 +529,7 @@ struct Test<K, T = u8> {
 }
 
 fn main() {
-    let zz<|> = Test { t: 23, k: 33 };
+    let zz<|> = Test { t: 23u8, k: 33 };
 }"#,
             &["Test<i32, u8>"],
         );
diff --git a/crates/ra_ide/src/inlay_hints.rs b/crates/ra_ide/src/inlay_hints.rs
index 49366de983..7eb2cef735 100644
--- a/crates/ra_ide/src/inlay_hints.rs
+++ b/crates/ra_ide/src/inlay_hints.rs
@@ -415,7 +415,7 @@ struct Test<K, T = u8> {
 }
 
 fn main() {
-    let zz = Test { t: 23, k: 33 };
+    let zz = Test { t: 23u8, k: 33 };
     let zz_ref = &zz;
 }"#,
         );
@@ -428,7 +428,7 @@ fn main() {
                 label: "Test<i32>",
             },
             InlayHint {
-                range: 105..111,
+                range: 107..113,
                 kind: TypeHint,
                 label: "&Test<i32>",
             },


Test patch:
diff --git a/crates/ra_hir_ty/src/tests/display_source_code.rs b/crates/ra_hir_ty/src/tests/display_source_code.rs
index 4088b1d22d..5dfa0a0145 100644
--- a/crates/ra_hir_ty/src/tests/display_source_code.rs
+++ b/crates/ra_hir_ty/src/tests/display_source_code.rs
@@ -29,7 +29,7 @@ fn omit_default_type_parameters() {
         //- /main.rs
         struct Foo<T = u8> { t: T }
         fn main() {
-            let foo = Foo { t: 5 };
+            let foo = Foo { t: 5u8 };
             foo<|>;
         }
         ",
@@ -41,7 +41,7 @@ fn omit_default_type_parameters() {
         //- /main.rs
         struct Foo<K, T = u8> { k: K, t: T }
         fn main() {
-            let foo = Foo { k: 400, t: 5 };
+            let foo = Foo { k: 400, t: 5u8 };
             foo<|>;
         }
         ",
diff --git a/crates/ra_hir_ty/src/tests/method_resolution.rs b/crates/ra_hir_ty/src/tests/method_resolution.rs
index 558a70022d..804297315c 100644
--- a/crates/ra_hir_ty/src/tests/method_resolution.rs
+++ b/crates/ra_hir_ty/src/tests/method_resolution.rs
@@ -183,60 +183,6 @@ fn test() {
     );
 }
 
-#[test]
-fn infer_associated_method_generics_with_default_param() {
-    assert_snapshot!(
-        infer(r#"
-struct Gen<T=u32> {
-    val: T
-}
-
-impl<T> Gen<T> {
-    pub fn make() -> Gen<T> {
-        loop { }
-    }
-}
-
-fn test() {
-    let a = Gen::make();
-}
-"#),
-        @r###"
-    80..104 '{     ...     }': Gen<T>
-    90..98 'loop { }': !
-    95..98 '{ }': ()
-    118..146 '{     ...e(); }': ()
-    128..129 'a': Gen<u32>
-    132..141 'Gen::make': fn make<u32>() -> Gen<u32>
-    132..143 'Gen::make()': Gen<u32>
-    "###
-    );
-}
-
-#[test]
-fn infer_associated_method_generics_with_default_tuple_param() {
-    let t = type_at(
-        r#"
-//- /main.rs
-struct Gen<T=()> {
-    val: T
-}
-
-impl<T> Gen<T> {
-    pub fn make() -> Gen<T> {
-        loop { }
-    }
-}
-
-fn test() {
-    let a = Gen::make();
-    a.val<|>;
-}
-"#,
-    );
-    assert_eq!(t, "()");
-}
-
 #[test]
 fn infer_associated_method_generics_without_args() {
     assert_snapshot!(
diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs
index 88309157b7..8a5031756f 100644
--- a/crates/ra_hir_ty/src/tests/simple.rs
+++ b/crates/ra_hir_ty/src/tests/simple.rs
@@ -1997,3 +1997,111 @@ fn foo() {
     "###
     );
 }
+
+#[test]
+fn generic_default() {
+    assert_snapshot!(
+        infer(r#"
+struct Thing<T = ()> { t: T }
+enum OtherThing<T = ()> {
+    One { t: T },
+    Two(T),
+}
+
+fn test(t1: Thing, t2: OtherThing, t3: Thing<i32>, t4: OtherThing<i32>) {
+    t1.t;
+    t3.t;
+    match t2 {
+        OtherThing::One { t } => { t; },
+        OtherThing::Two(t) => { t; },
+    }
+    match t4 {
+        OtherThing::One { t } => { t; },
+        OtherThing::Two(t) => { t; },
+    }
+}
+"#),
+        @r###"
+    98..100 't1': Thing<()>
+    109..111 't2': OtherThing<()>
+    125..127 't3': Thing<i32>
+    141..143 't4': OtherThing<i32>
+    162..385 '{     ...   } }': ()
+    168..170 't1': Thing<()>
+    168..172 't1.t': ()
+    178..180 't3': Thing<i32>
+    178..182 't3.t': i32
+    188..283 'match ...     }': ()
+    194..196 't2': OtherThing<()>
+    207..228 'OtherT... { t }': OtherThing<()>
+    225..226 't': ()
+    232..238 '{ t; }': ()
+    234..235 't': ()
+    248..266 'OtherT...Two(t)': OtherThing<()>
+    264..265 't': ()
+    270..276 '{ t; }': ()
+    272..273 't': ()
+    288..383 'match ...     }': ()
+    294..296 't4': OtherThing<i32>
+    307..328 'OtherT... { t }': OtherThing<i32>
+    325..326 't': i32
+    332..338 '{ t; }': ()
+    334..335 't': i32
+    348..366 'OtherT...Two(t)': OtherThing<i32>
+    364..365 't': i32
+    370..376 '{ t; }': ()
+    372..373 't': i32
+    "###
+    );
+}
+
+#[test]
+fn generic_default_in_struct_literal() {
+    assert_snapshot!(
+        infer(r#"
+struct Thing<T = ()> { t: T }
+enum OtherThing<T = ()> {
+    One { t: T },
+    Two(T),
+}
+
+fn test() {
+    let x = Thing { t: loop {} };
+    let y = Thing { t: () };
+    let z = Thing { t: 1i32 };
+    if let Thing { t } = z {
+        t;
+    }
+
+    let a = OtherThing::One { t: 1i32 };
+    let b = OtherThing::Two(1i32);
+}
+"#),
+        @r###"
+    100..320 '{     ...32); }': ()
+    110..111 'x': Thing<!>
+    114..134 'Thing ...p {} }': Thing<!>
+    125..132 'loop {}': !
+    130..132 '{}': ()
+    144..145 'y': Thing<()>
+    148..163 'Thing { t: () }': Thing<()>
+    159..161 '()': ()
+    173..174 'z': Thing<i32>
+    177..194 'Thing ...1i32 }': Thing<i32>
+    188..192 '1i32': i32
+    200..241 'if let...     }': ()
+    207..218 'Thing { t }': Thing<i32>
+    215..216 't': i32
+    221..222 'z': Thing<i32>
+    223..241 '{     ...     }': ()
+    233..234 't': i32
+    251..252 'a': OtherThing<i32>
+    255..282 'OtherT...1i32 }': OtherThing<i32>
+    276..280 '1i32': i32
+    292..293 'b': OtherThing<i32>
+    296..311 'OtherThing::Two': Two<i32>(i32) -> OtherThing<i32>
+    296..317 'OtherT...(1i32)': OtherThing<i32>
+    312..316 '1i32': i32
+    "###
+    );
+}
diff --git a/crates/ra_hir_ty/src/tests/traits.rs b/crates/ra_hir_ty/src/tests/traits.rs
index 0c538a62d9..133fb5f398 100644
--- a/crates/ra_hir_ty/src/tests/traits.rs
+++ b/crates/ra_hir_ty/src/tests/traits.rs
@@ -1806,33 +1806,33 @@ fn test() {
 }
 "#),
         @r###"
-65..69 'self': &Self
-166..170 'self': Self
-172..176 'args': Args
-240..244 'self': &Foo
-255..257 '{}': ()
-335..336 'f': F
-355..357 '{}': ()
-444..690 '{     ...o(); }': ()
-454..459 'lazy1': Lazy<Foo, fn() -> T>
-476..485 'Lazy::new': fn new<Foo, fn() -> T>(fn() -> T) -> Lazy<Foo, fn() -> T>
-476..493 'Lazy::...| Foo)': Lazy<Foo, fn() -> T>
-486..492 '|| Foo': || -> T
-489..492 'Foo': Foo
-503..505 'r1': {unknown}
-508..513 'lazy1': Lazy<Foo, fn() -> T>
-508..519 'lazy1.foo()': {unknown}
-561..576 'make_foo_fn_ptr': fn() -> Foo
-592..603 'make_foo_fn': fn make_foo_fn() -> Foo
-613..618 'lazy2': Lazy<Foo, fn() -> T>
-635..644 'Lazy::new': fn new<Foo, fn() -> T>(fn() -> T) -> Lazy<Foo, fn() -> T>
-635..661 'Lazy::...n_ptr)': Lazy<Foo, fn() -> T>
-645..660 'make_foo_fn_ptr': fn() -> Foo
-671..673 'r2': {unknown}
-676..681 'lazy2': Lazy<Foo, fn() -> T>
-676..687 'lazy2.foo()': {unknown}
-550..552 '{}': ()
-"###
+    65..69 'self': &Self
+    166..170 'self': Self
+    172..176 'args': Args
+    240..244 'self': &Foo
+    255..257 '{}': ()
+    335..336 'f': F
+    355..357 '{}': ()
+    444..690 '{     ...o(); }': ()
+    454..459 'lazy1': Lazy<Foo, || -> Foo>
+    476..485 'Lazy::new': fn new<Foo, || -> Foo>(|| -> Foo) -> Lazy<Foo, || -> Foo>
+    476..493 'Lazy::...| Foo)': Lazy<Foo, || -> Foo>
+    486..492 '|| Foo': || -> Foo
+    489..492 'Foo': Foo
+    503..505 'r1': usize
+    508..513 'lazy1': Lazy<Foo, || -> Foo>
+    508..519 'lazy1.foo()': usize
+    561..576 'make_foo_fn_ptr': fn() -> Foo
+    592..603 'make_foo_fn': fn make_foo_fn() -> Foo
+    613..618 'lazy2': Lazy<Foo, fn() -> Foo>
+    635..644 'Lazy::new': fn new<Foo, fn() -> Foo>(fn() -> Foo) -> Lazy<Foo, fn() -> Foo>
+    635..661 'Lazy::...n_ptr)': Lazy<Foo, fn() -> Foo>
+    645..660 'make_foo_fn_ptr': fn() -> Foo
+    671..673 'r2': {unknown}
+    676..681 'lazy2': Lazy<Foo, fn() -> Foo>
+    676..687 'lazy2.foo()': {unknown}
+    550..552 '{}': ()
+    "###
     );
 }