Add missing check for duplicated binding in the same pattern

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 `..` found".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(),
@@ -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(),

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/pat/duplicated_binding.fe

@@ -0,0 +1,11 @@
+pub enum E {
+    V(i32, u32)
+}
+
+fn foo(f: E) {
+    let E::V(x, x) = f
+
+    match f {
+        E::V(x, x) => ()
+    }
+}

Diff for: crates/uitest/fixtures/ty_check/pat/duplicated_binding.snap

@@ -0,0 +1,20 @@
+---
+source: crates/uitest/tests/ty_check.rs
+expression: diags
+input_file: fixtures/ty_check/pat/duplicated_binding.fe
+---
+error[8-0032]: duplicate binding `x` in pattern
+  ┌─ duplicated_binding.fe:6:17
+  │
+6 │     let E::V(x, x) = f
+  │              -  ^ `x` is defined again here
+  │              │   
+  │              first definition of `x` in this pattern
+
+error[8-0032]: duplicate binding `x` in pattern
+  ┌─ duplicated_binding.fe:9:17
+  │
+9 │         E::V(x, x) => ()
+  │              -  ^ `x` is defined again here
+  │              │   
+  │              first definition of `x` in this pattern

Diff for: crates/uitest/fixtures/ty_check/pat/record.snap

@@ -1,15 +1,15 @@
 ---
 source: crates/uitest/tests/ty_check.rs
 expression: diags
-input_file: crates/uitest/fixtures/ty_check/pat/record.fe
+input_file: fixtures/ty_check/pat/record.fe
 ---
-error[8-0002]: duplicated `..` found
+error[8-0002]: duplicate `..` found
    ┌─ record.fe:13:24
    │
 13 │     let Foo {x, .., y, ..}
    │                        ^^ `..` can be used only once
 
-error[8-0002]: duplicated `..` found
+error[8-0002]: duplicate `..` found
    ┌─ record.fe:19:33
    │
 19 │     let Bar::Variant {x, .., y, ..}
@@ -79,4 +79,18 @@ error[8-0011]: all fields are not given
    │                      missing `x, y`
    │                      Consider using `Bar::Variant { x, y }` instead
 
+error[8-0032]: duplicate binding `x` in pattern
+   ┌─ record.fe:12:17
+   │
+12 │     let Foo {x, x}
+   │              -  ^ `x` is defined again here
+   │              │   
+   │              first definition of `x` in this pattern
 
+error[8-0032]: duplicate binding `x` in pattern
+   ┌─ record.fe:18:26
+   │
+18 │     let Bar::Variant {x, x}
+   │                       -  ^ `x` is defined again here
+   │                       │   
+   │                       first definition of `x` in this pattern