Skip to content

chore: fix a bunch of annoying clippy lints #4558

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 8 commits into from
Mar 8, 2022
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Mar 8, 2022

Motivation

Recent Clippy releases have added some new lints that trigger on some
code in Tokio. These aren't a big deal, but seeing them in my editor is
mildly annoying.

Solution

This branch fixes the following issues flagged by Clippy:

  • manual Option::map implementation
  • use of .map(...).flatten(...) that could be replaced with
    .and_then(...)
  • manual implementation of saturating arithmetic on Durations
  • simplify some boolean expressions in assertions (!res.is_ok() can be
    res.is_err())
  • fix redundant field names in initializers
  • allow the clippy::unnecessary_to_owned lint in ToSocketAddrs impl
    for &[SocketAddr] --- the "unnecessary" to_vec() call is actually
    necessary to ensure the iterator is 'static
  • replace an unnecessary cast to usize with an explicitly typed
    integer literal

hawkw added 7 commits March 8, 2022 09:18
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
fixes `clippy::nonminimal_bool` lints

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@github-actions github-actions bot added the R-loom Run loom tests on this PR label Mar 8, 2022
@Noah-Kennedy
Copy link
Contributor

Looks like clippy is failing.

@hawkw
Copy link
Member Author

hawkw commented Mar 8, 2022

Looks like clippy is failing.

Hmm, it looks like it's failing because the clippy::unnecessary_to_owned lint doesn't exist. This seems to be because the Clippy CI workflow is running on Rust 1.52.0 for some reason and that lint wasn't added to Clippy until Rust 1.58.

I'm not sure why the Clippy CI job isn't running on the latest stable Rust, but we seem to have explicitly pinned it to 1.52.0 on purpose...

I guess the simplest solution is to just remove the allow attribute for that lint, so that the clippy CI build passes; users with newer Rust toolchains can just accept that they'll get warnings there, I guess.

@hawkw
Copy link
Member Author

hawkw commented Mar 8, 2022

Oh, it looks like the reason Clippy is pinned to 1.52 is to avoid breaking LTS releases when new lints are added: 2cee1db

I'll just go ahead and remove the allow attributes.

That lint doesn't exist on the Rust 1.52.0 version of clippy, so it breaks CI.
@Darksonn Darksonn added the A-tokio Area: The main tokio crate label Mar 8, 2022
@hawkw hawkw merged commit dee26c9 into master Mar 8, 2022
@hawkw hawkw deleted the eliza/clippy-stuff branch March 8, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate R-loom Run loom tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants