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

Use the rustc unknown lints attribute #1609

Merged
merged 4 commits into from
Jan 19, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jan 18, 2021

Motivation

The clippy unknown lints attribute was deprecated in nightly in rust-lang/rust#80524. The old lint name now produces a warning.

Solution

Eliminate as many lints as possible, then configure retained lints to avoid warnings on nightly and stable. That's a bit tricky, at least for the transition period.

Retained Lints

  1. Since we're using allow(unknown_lints) to suppress warnings, we need to add the canonical name, so we can continue to build without warnings on nightly.

  2. But we also need to keep the old name, so we can continue to build without warnings on stable.

  3. And therefore, we also need to disable the "removed lints" warning, otherwise we'll get warnings about the old name on nightly.

We'll need to keep this transitional clippy config from nightly 2021-01-17 until rustc 1.51 is stable.

Review

This change is a low priority, unless your workflow depends on building without warnings on nightly.

Related Issues

rust-lang/rust#80524 - the deprecation PR

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup labels Jan 18, 2021
@teor2345 teor2345 added this to the 2021 Sprint 1 milestone Jan 18, 2021
@teor2345 teor2345 requested a review from a team January 18, 2021 04:37
@teor2345 teor2345 self-assigned this Jan 18, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Jan 18, 2021

This change doesn't work, because the stable version of rustc doesn't include clippy in unknown_lints.

We might have to use conditional compilation with rustc_version. But we should also check if any of these lints are known in the latest stable, so we can delete unknown_lints for them.

@teor2345 teor2345 marked this pull request as draft January 18, 2021 05:26
@teor2345 teor2345 marked this pull request as ready for review January 18, 2021 20:22
@teor2345 teor2345 force-pushed the unknown-lints branch 2 times, most recently from 3a9e5e4 to 4ab633f Compare January 19, 2021 00:36
@teor2345
Copy link
Contributor Author

I set this PR to auto-merge (rebase) when it's approved.

I picked rebase so we keep a history of the multiple different steps we took to resolve the warnings.

It's important for people who use nightly, and it's safe to merge immediately.

The clippy unknown lints attribute was deprecated in
nightly in rust-lang/rust#80524. The old lint name now produces a
warning.

Since we're using `allow(unknown_lints)` to suppress warnings, we need to
add the canonical name, so we can continue to build without warnings on
nightly.

But we also need to keep the old name, so we can continue to build
without warnings on stable.

And therefore, we also need to disable the "removed lints" warning,
otherwise we'll get warnings about the old name on nightly.

We'll need to keep this transitional clippy config until rustc 1.51 is
stable.
@teor2345
Copy link
Contributor Author

Oops, my local clippy wasn't checking tests, even though I said --tests

@dconnolly dconnolly merged commit 49e6150 into ZcashFoundation:main Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants