Merge #4765

4765: Fix type parameter defaults r=matklad a=flodiebold

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.)

Co-authored-by: Florian Diebold <florian.diebold@freiheit.com>

Author: bors[bot]

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 };
 }"#,
         );
     }

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)

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;

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(

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<|>;
         }
         ",

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!(

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
+    "###
+    );
+}