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

Fix clippy #803

Merged
merged 3 commits into from
May 3, 2024
Merged

Fix clippy #803

merged 3 commits into from
May 3, 2024

Conversation

macpie
Copy link
Member

@macpie macpie commented May 3, 2024

It seems like this version of Rust does not like to have struct and impl in different files.

Copy link
Contributor

@jeffgrunewald jeffgrunewald left a comment

Choose a reason for hiding this comment

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

instead of duplicating the MockHexBoostingClient struct definition in each of the test files, couldn't you instead centralize the impl MockHexBoostingClient new method in the tests/common module? they appear to be identical.

perhaps that also solves the problem of the struct and the impl being in different places but without the amount of duplication? not sure why those new methods were implemented multiple times but that could certainly cause issues

@macpie
Copy link
Member Author

macpie commented May 3, 2024

I have tried and clippy is not happy:

error: associated function `new` is never used
  --> mobile_verifier/tests/common/mod.rs:26:12
   |
25 | impl MockHexBoostingClient {
   | -------------------------- associated function in this implementation
26 |     pub fn new(boosted_hexes: Vec<BoostedHexInfo>) -> Self {
   |            ^^^
   |
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`

But new is used...

@jeffgrunewald
Copy link
Contributor

I would go with that approach and just slap a #[allow(dead_code)] on the new method and be done with it. I think you're right that it's just unnecessarily confused by how the test environment is compiled. The above bandage is stupid but it works and allows us to maintain the struct and new impl in one place

Copy link
Contributor

@jeffgrunewald jeffgrunewald left a comment

Choose a reason for hiding this comment

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

Why do you hate proper hygienic testing, Rust?

@macpie macpie merged commit 5816910 into main May 3, 2024
1 check passed
@macpie macpie deleted the macpie/clippy_fix branch May 3, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants