Merge pull request #1047 from sbillig/pat-binding-dup

Author: Y-Nak

Diff for: crates/hir-analysis/src/ty/diagnostics.rs

@@ -544,6 +544,11 @@ pub enum BodyDiag<'db> {
     TypeMismatch(DynLazySpan<'db>, String, String),
     InfiniteOccurrence(DynLazySpan<'db>),
 
+    DuplicatedBinding {
+        primary: DynLazySpan<'db>,
+        conflicat_with: DynLazySpan<'db>,
+        name: IdentId<'db>,
+    },
     DuplicatedRestPat(DynLazySpan<'db>),
 
     InvalidPathDomainInPat {
@@ -927,14 +932,18 @@ impl<'db> BodyDiag<'db> {
             Self::MethodNotFound { .. } => 29,
             Self::NotValue { .. } => 30,
             Self::TypeAnnotationNeeded { .. } => 31,
+            Self::DuplicatedBinding { .. } => 32,
         }
     }
 
     fn message(&self, db: &dyn HirDb) -> String {
         match self {
             Self::TypeMismatch(_, _, _) => "type mismatch".to_string(),
             Self::InfiniteOccurrence(_) => "infinite sized type found".to_string(),
-            Self::DuplicatedRestPat(_) => "duplicated `..` found".to_string(),
+            Self::DuplicatedBinding { name, .. } => {
+                format!("duplicate binding `{}` in pattern", name.data(db))
+            }
+            Self::DuplicatedRestPat(_) => "duplicate `..` in pattern".to_string(),
             Self::InvalidPathDomainInPat { .. } => "invalid item is given here".to_string(),
             Self::UnitVariantExpected { .. } => "expected unit variant".to_string(),
             Self::TupleVariantExpected { .. } => "expected tuple variant".to_string(),
@@ -943,7 +952,7 @@ impl<'db> BodyDiag<'db> {
             Self::DuplicatedRecordFieldBind { .. } => "duplicated record field binding".to_string(),
             Self::RecordFieldNotFound { .. } => "specified field not found".to_string(),
             Self::ExplicitLabelExpectedInRecord { .. } => "explicit label is required".to_string(),
-            Self::MissingRecordFields { .. } => "all fields are not given".to_string(),
+            Self::MissingRecordFields { .. } => "missing fields in record pattern".to_string(),
             Self::UndefinedVariable(..) => "undefined variable".to_string(),
             Self::ReturnedTypeMismatch { .. } => "returned type mismatch".to_string(),
             Self::TypeMustBeKnown(..) => "type must be known here".to_string(),
@@ -969,14 +978,14 @@ impl<'db> BodyDiag<'db> {
                 format!("`{}` needs to be implemented for {ty}", trait_name.data(db))
             }
 
-            Self::NotCallable(..) => "not callable type is given in call expression".to_string(),
+            Self::NotCallable(_, ty) => format!("expected function, found `{ty}`"),
 
             Self::CallGenericArgNumMismatch { .. } => {
                 "given generic argument number mismatch".to_string()
             }
 
-            Self::CallArgNumMismatch { .. } => "given argument number mismatch".to_string(),
-            Self::CallArgLabelMismatch { .. } => "given argument label mismatch".to_string(),
+            Self::CallArgNumMismatch { .. } => "argument number mismatch".to_string(),
+            Self::CallArgLabelMismatch { .. } => "argument label mismatch".to_string(),
 
             Self::AmbiguousInherentMethodCall { .. } => "ambiguous method call".to_string(),
 
@@ -1009,6 +1018,26 @@ impl<'db> BodyDiag<'db> {
                 span.resolve(db),
             )],
 
+            Self::DuplicatedBinding {
+                primary,
+                conflicat_with,
+                name,
+            } => {
+                let name = name.data(db.as_hir_db());
+                vec![
+                    SubDiagnostic::new(
+                        LabelStyle::Primary,
+                        format!("`{name}` is defined again here",),
+                        primary.resolve(db),
+                    ),
+                    SubDiagnostic::new(
+                        LabelStyle::Secondary,
+                        format!("first definition of `{name}` in this pattern"),
+                        conflicat_with.resolve(db),
+                    ),
+                ]
+            }
+
             Self::DuplicatedRestPat(span) => vec![SubDiagnostic::new(
                 LabelStyle::Primary,
                 "`..` can be used only once".to_string(),
@@ -1135,7 +1164,7 @@ impl<'db> BodyDiag<'db> {
                 vec![
                     SubDiagnostic::new(
                         LabelStyle::Primary,
-                        format!("duplicated field binding `{}`", name),
+                        format!("duplicate field binding `{}`", name),
                         primary.resolve(db),
                     ),
                     SubDiagnostic::new(
@@ -1285,7 +1314,7 @@ impl<'db> BodyDiag<'db> {
                 vec![
                     SubDiagnostic::new(
                         LabelStyle::Primary,
-                        format!("`{}` cant be applied to `{}`", op, ty),
+                        format!("`{}` can't be applied to `{}`", op, ty),
                         span.resolve(db),
                     ),
                     SubDiagnostic::new(
@@ -1352,7 +1381,7 @@ impl<'db> BodyDiag<'db> {
             Self::NotCallable(primary, ty) => {
                 vec![SubDiagnostic::new(
                     LabelStyle::Primary,
-                    format!("`{ty}` is not callable"),
+                    format!("call expression requires function; `{ty}` is not callable"),
                     primary.resolve(db),
                 )]
             }

Diff for: crates/hir-analysis/src/ty/ty_check/env.rs

@@ -177,17 +177,38 @@ impl<'db> TyCheckEnv<'db> {
         self.pat_ty.insert(pat, ty);
     }
 
-    /// Register a pending binding which will be added when `flush_pending_vars`
-    /// is called.
+    /// Registers a new pending binding.
+    ///
+    /// This function adds a binding to the list of pending variables. If a
+    /// binding with the same name already exists, it returns the existing
+    /// binding. Otherwise, it returns `None`.
+    ///
+    /// To flush pending bindings to the designated scope, call
+    /// [`flush_pending_bindings`] in the scope.
+    ///
+    /// # Arguments
+    ///
+    /// * `name` - The identifier of the variable.
+    /// * `binding` - The local binding to be registered.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(LocalBinding)` if a binding with the same name already exists.
+    /// * `None` if the binding was successfully registered.
     pub(super) fn register_pending_binding(
         &mut self,
         name: IdentId<'db>,
         binding: LocalBinding<'db>,
-    ) {
-        self.pending_vars.insert(name, binding);
+    ) -> Option<LocalBinding<'db>> {
+        self.pending_vars.insert(name, binding)
     }
 
-    /// Flush pending bindings to the current scope environment.
+    /// Flushes all pending variable bindings into the current variable
+    /// environment.
+    ///
+    /// This function moves all pending bindings from the `pending_vars` map
+    /// into the latest `BlockEnv` in `var_env`. After this operation, the
+    /// `pending_vars` map will be empty.
     pub(super) fn flush_pending_bindings(&mut self) {
         let var_env = self.var_env.last_mut().unwrap();
         for (name, binding) in self.pending_vars.drain() {
@@ -199,6 +220,25 @@ impl<'db> TyCheckEnv<'db> {
         self.pending_confirmations.push((inst, span))
     }
 
+    /// Completes the type checking environment by finalizing pending trait
+    /// confirmations, folding types with the unification table, and collecting
+    /// diagnostics.
+    ///
+    /// # Arguments
+    ///
+    /// * `table` - A mutable reference to the unification table used for type
+    ///   unification.
+    ///
+    /// # Returns
+    ///
+    /// * A tuple containing the `TypedBody` and a vector of `FuncBodyDiag`.
+    ///
+    /// The `TypedBody` includes the body of the function, pattern types,
+    /// expression types, and callables, all of which have been folded with
+    /// the unification table.
+    ///
+    /// The vector of `FuncBodyDiag` contains diagnostics related to function
+    /// bodies, such as ambiguous trait instances.
     pub(super) fn finish(
         mut self,
         table: &mut UnificationTable<'db>,
@@ -249,6 +289,22 @@ impl<'db> TyCheckEnv<'db> {
         &self.var_env[idx]
     }
 
+    /// Performs pending trait confirmations and collects diagnostics.
+    ///
+    /// This function attempts to satisfy all pending trait confirmations by
+    /// iteratively probing and unifying trait instances until a fixed point
+    /// is reached. If any trait instance remains ambiguous, a diagnostic is
+    /// generated and added to the diagnostics vector.
+    ///
+    /// # Arguments
+    ///
+    /// * `prober` - A mutable reference to the [`Prober`] used for type
+    ///   unification and probing.
+    ///
+    /// # Returns
+    ///
+    /// * A vector of `FuncBodyDiag` containing diagnostics related to ambiguous
+    ///   trait instances.
     fn perform_pending_confirmation(
         &self,
         prober: &mut Prober<'db, '_>,

Diff for: crates/hir-analysis/src/ty/ty_check/pat.rs

@@ -140,11 +140,19 @@ impl<'db> TyChecker<'db> {
                     }
                 }
                 _ => {
+                    let name = *path.ident(self.db.as_hir_db()).unwrap();
                     let binding = LocalBinding::local(pat, *is_mut);
-                    self.env.register_pending_binding(
-                        *path.ident(self.db.as_hir_db()).unwrap(),
-                        binding,
-                    );
+                    if let Some(LocalBinding::Local {
+                        pat: conflict_with, ..
+                    }) = self.env.register_pending_binding(name, binding)
+                    {
+                        let diag = BodyDiag::DuplicatedBinding {
+                            primary: span.into(),
+                            conflicat_with: conflict_with.lazy_span(self.body()).into(),
+                            name,
+                        };
+                        self.push_diag(diag);
+                    }
                     self.fresh_ty()
                 }
             }

Diff for: crates/uitest/fixtures/ty_check/aug_assign.snap

@@ -1,15 +1,15 @@
 ---
 source: crates/uitest/tests/ty_check.rs
 expression: diags
-input_file: crates/uitest/fixtures/ty_check/aug_assign.fe
+input_file: fixtures/ty_check/aug_assign.fe
 ---
 error[8-0016]: `std::ops::SubAssign` trait is not implemented
   ┌─ aug_assign.fe:6:5
   │
 6 │     f -= f
   │     ^^^^^^
   │     │
-  │     `-=` cant be applied to `Foo`
+  │     `-=` can't be applied to `Foo`
   │     Try implementing `std::ops::SubAssign` for `Foo`
 
 error[8-0018]: left-hand side of assignment is immutable
@@ -20,5 +20,3 @@ error[8-0018]: left-hand side of assignment is immutable
 6 │     f -= f
 7 │     f.x *= 1
   │     ^^^ immutable assignment
-
-

Diff for: crates/uitest/fixtures/ty_check/binary.snap

@@ -1,15 +1,15 @@
 ---
 source: crates/uitest/tests/ty_check.rs
 expression: diags
-input_file: crates/uitest/fixtures/ty_check/binary.fe
+input_file: fixtures/ty_check/binary.fe
 ---
 error[8-0016]: `std::ops::Add` trait is not implemented
   ┌─ binary.fe:4:5
   │
 4 │     f + f
   │     ^^^^^
   │     │
-  │     `+` cant be applied to `Foo`
+  │     `+` can't be applied to `Foo`
   │     Try implementing `std::ops::Add` for `Foo`
 
 error[8-0016]: `std::ops::And` trait is not implemented
@@ -18,7 +18,7 @@ error[8-0016]: `std::ops::And` trait is not implemented
 6 │     (f && f) || f
   │      ^^^^^^
   │      │
-  │      `&&` cant be applied to `Foo`
+  │      `&&` can't be applied to `Foo`
   │      Try implementing `std::ops::And` for `Foo`
 
 error[8-0016]: `std::ops::Eq` trait is not implemented
@@ -27,7 +27,7 @@ error[8-0016]: `std::ops::Eq` trait is not implemented
 7 │     f == f
   │     ^^^^^^
   │     │
-  │     `==` cant be applied to `Foo`
+  │     `==` can't be applied to `Foo`
   │     Try implementing `std::ops::Eq` for `Foo`
 
 error[8-0016]: `std::ops::Ord` trait is not implemented
@@ -36,7 +36,5 @@ error[8-0016]: `std::ops::Ord` trait is not implemented
 8 │     f < f
   │     ^^^^^
   │     │
-  │     `<` cant be applied to `Foo`
+  │     `<` can't be applied to `Foo`
   │     Try implementing `std::ops::Ord` for `Foo`
-
-

Diff for: crates/uitest/fixtures/ty_check/call.fe

@@ -15,4 +15,9 @@ pub fn make_tuple<T, U>(_ t: T, _ u: U) -> (T, U) {
 
 pub fn use_make_tuple() -> (i32, u32) {
     make_tuple<i32, u32>(false, 1)
-}
+}
+
+fn f() {
+    let x: u32 = 1
+    x(100)
+}

Diff for: crates/uitest/fixtures/ty_check/call.snap

@@ -1,7 +1,7 @@
 ---
 source: crates/uitest/tests/ty_check.rs
 expression: diags
-input_file: crates/uitest/fixtures/ty_check/call.fe
+input_file: fixtures/ty_check/call.fe
 ---
 error[8-0000]: type mismatch
   ┌─ call.fe:9:12
@@ -15,7 +15,13 @@ error[8-0000]: type mismatch
 17 │     make_tuple<i32, u32>(false, 1)
    │                          ^^^^^ expected `i32`, but `bool` is given
 
-error[8-0024]: given argument label mismatch
+error[8-0021]: expected function, found `u32`
+   ┌─ call.fe:22:5
+   │
+22 │     x(100)
+   │     ^ call expression requires function; `u32` is not callable
+
+error[8-0024]: argument label mismatch
   ┌─ call.fe:6:9
   │
 1 │ pub fn add(x: i32, y: i32) -> i32 {
@@ -24,7 +30,7 @@ error[8-0024]: given argument label mismatch
 6 │     add(1, 2)
   │         ^ expected `x` label
 
-error[8-0024]: given argument label mismatch
+error[8-0024]: argument label mismatch
   ┌─ call.fe:6:12
   │
 1 │ pub fn add(x: i32, y: i32) -> i32 {
@@ -33,7 +39,7 @@ error[8-0024]: given argument label mismatch
 6 │     add(1, 2)
   │            ^ expected `y` label
 
-error[8-0024]: given argument label mismatch
+error[8-0024]: argument label mismatch
   ┌─ call.fe:7:9
   │
 1 │ pub fn add(x: i32, y: i32) -> i32 {
@@ -42,7 +48,7 @@ error[8-0024]: given argument label mismatch
 7 │     add(y: 1, x: 2)
   │         ^ expected `x` label, but `y` given
 
-error[8-0024]: given argument label mismatch
+error[8-0024]: argument label mismatch
   ┌─ call.fe:7:15
   │
 1 │ pub fn add(x: i32, y: i32) -> i32 {
@@ -51,13 +57,11 @@ error[8-0024]: given argument label mismatch
 7 │     add(y: 1, x: 2)
   │               ^ expected `y` label, but `x` given
 
-error[8-0024]: given argument label mismatch
+error[8-0024]: argument label mismatch
   ┌─ call.fe:8:15
   │
 1 │ pub fn add(x: i32, y: i32) -> i32 {
   │        --- function defined here
   ·
 8 │     add(x: 1, z: 2)
   │               ^ expected `y` label, but `z` given
-
-

Diff for: crates/uitest/fixtures/ty_check/index.snap

@@ -1,7 +1,7 @@
 ---
 source: crates/uitest/tests/ty_check.rs
 expression: diags
-input_file: crates/uitest/fixtures/ty_check/index.fe
+input_file: fixtures/ty_check/index.fe
 ---
 error[8-0000]: type mismatch
   ┌─ index.fe:4:7
@@ -15,7 +15,5 @@ error[8-0016]: `std::ops::Index` trait is not implemented
 6 │     f[1]
   │     ^^^^
   │     │
-  │     `[]` cant be applied to `Foo`
+  │     `[]` can't be applied to `Foo`
   │     Try implementing `std::ops::Index` for `Foo`
-
-