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

chore: Apply Clippy lints redundant_clone and clear_with_drain #1640

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jbencin
Copy link
Member

@jbencin jbencin commented Jan 14, 2025

Description

I've been doing some code cleanup on stacks-core, so I thought I'd try it on this repo also. This repo is already configured to use Clippy, which is great, but by default Clippy does not apply it's "nursery" (experimental) lints. Some of these can be useful and are worth applying. Specifically, I checked the following:

clear_with_drain
collection_is_never_read
needless_collect
iter_with_drain
mutex_integer
set_contains_or_insert
trivial_regex
useless_let_if_seq
unused_rounding
unnecessary_struct_initialization
tuple_array_conversions

Only redundant_clone and clear_with_drain resulted in any changes

Breaking change?

No

Example

This can make the code more clear and improve performance by removing unnecessary function calls. For example, one change I made was:

fn level_to_string(level: &Level) -> String {
    match level {
        Level::Note => blue!("note:").to_string(),
        Level::Warning => yellow!("warning:").to_string(),
        Level::Error => red!("error:").to_string(),
    }
}

to

fn level_to_string(level: &Level) -> String {
    match level {
        Level::Note => blue!("note:"),
        Level::Warning => yellow!("warning:"),
        Level::Error => red!("error:"),
    }
}

Checklist

  • Tests added in this PR (if applicable)

@hugocaillard
Copy link
Collaborator

@jbencin thaznks for trying it on Clarinet as well 🙌 💯

What are the benefits/risks of running the experimental lints?
I see that you didn't change any config, so I assume you ran a custom command to get the lints? Should also change a config that would run clippy experimental features so that we don't keep introducing code that break any of these rules?

@jbencin
Copy link
Member Author

jbencin commented Jan 14, 2025

What are the benefits/risks of running the experimental lints?

They are just less reliable, so sometimes they miss things, or find false positives. They are still useful, it's just more of a manual process instead of cargo clippy --fix

Should also change a config that would run clippy experimental features so that we don't keep introducing code that break any of these rules?

Personally I wouldn't recommend it because of the false positives. There are still a couple incorrect redundant_clone violations that Clippy finds (I was using Rust/Clippy 1.83)

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.

2 participants