Skip to content
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

Check trait items for duplicate function names or associated type names #179

Merged
merged 2 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 23 additions & 2 deletions crates/formality-check/src/traits.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use anyhow::bail;
use fn_error_context::context;
use formality_core::Set;
use formality_prove::Env;
use formality_rust::grammar::{
AssociatedTy, AssociatedTyBoundData, Fn, Trait, TraitBoundData, TraitItem, WhereClause,
Expand Down Expand Up @@ -31,8 +33,27 @@ impl super::Check<'_> {
Ok(())
}

fn check_trait_items_have_unique_names(&self, _trait_items: &[TraitItem]) -> Fallible<()> {
// FIXME:
fn check_trait_items_have_unique_names(&self, trait_items: &[TraitItem]) -> Fallible<()> {
let mut functions = Set::new();
let mut associated_types = Set::new();
for trait_item in trait_items {
match trait_item {
TraitItem::Fn(f) => {
if !functions.insert(&f.id) {
bail!("the function name `{:?}` is defined multiple times", f.id);
}
}
TraitItem::AssociatedTy(associated_ty) => {
let AssociatedTy { id, .. } = associated_ty;
if !associated_types.insert(id) {
bail!(
"the associated type name `{:?}` is defined multiple times",
id
);
}
}
}
}
Ok(())
}

Expand Down
45 changes: 45 additions & 0 deletions src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,48 @@ fn basic_where_clauses_fail() {
expression evaluated to an empty collection: `decls.trait_invariants()`"#]]
)
}

#[test]
fn trait_items_with_duplicate_fn_names() {
crate::assert_err!(
[
crate core {
trait A {
fn a() -> ();
fn a() -> ();
}
}
]

[ /* TODO */ ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[ /* TODO */ ]
[ "function name `a` is defined multiple times" ]

try that, I think

Copy link
Contributor Author

@FullyNonlinear FullyNonlinear Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated accordingly on this commit: 1cff4c2. (An extra comma is needed for the must_have parameter.)


expect_test::expect![[r#"
check_trait(A)

Caused by:
the function name `a` is defined multiple times"#]]

);
}

#[test]
fn trait_items_with_duplicate_associated_type_names() {
crate::assert_err!(
[
crate core {
trait A {
type Assoc : [];
type Assoc : [];
}
}
]

[ /* TODO */ ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[ /* TODO */ ]
[ "associated type name `Assoc` is defined multiple times" ]

Copy link
Contributor Author

@FullyNonlinear FullyNonlinear Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have updated accordingly in this commit: 1cff4c2. (An extra comma is needed for the must_have parameter.)


expect_test::expect![[r#"
check_trait(A)

Caused by:
the associated type name `Assoc` is defined multiple times"#]]
);
}
Loading