Skip to content

Add missing check for duplicated binding in same pattern #1047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions crates/hir-analysis/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(),

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
)]
}
Expand Down
66 changes: 61 additions & 5 deletions crates/hir-analysis/src/ty/ty_check/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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>,
Expand Down Expand Up @@ -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, '_>,
Expand Down
16 changes: 12 additions & 4 deletions crates/hir-analysis/src/ty/ty_check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down
6 changes: 2 additions & 4 deletions crates/uitest/fixtures/ty_check/aug_assign.snap
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -20,5 +20,3 @@ error[8-0018]: left-hand side of assignment is immutable
6 │ f -= f
7 │ f.x *= 1
│ ^^^ immutable assignment


12 changes: 5 additions & 7 deletions crates/uitest/fixtures/ty_check/binary.snap
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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`


7 changes: 6 additions & 1 deletion crates/uitest/fixtures/ty_check/call.fe
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
20 changes: 12 additions & 8 deletions crates/uitest/fixtures/ty_check/call.snap
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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


6 changes: 2 additions & 4 deletions crates/uitest/fixtures/ty_check/index.snap
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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`


Loading
Loading