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

False positive unfulfilled_lint_expectations with either of --tests or --all-targets #130021

Closed
lnicola opened this issue Sep 6, 2024 · 2 comments · Fixed by #130025
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints F-lint_reasons `#![feature(lint_reasons)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lnicola
Copy link
Member

lnicola commented Sep 6, 2024

Code

//! crate docs

#![warn(missing_docs)]
#![warn(clippy::pedantic)]

#[expect(missing_docs)]
pub fn foo() {}

#[expect(clippy::enum_glob_use)]
use std::cmp::Ordering::*;

fn main() {
    #[expect(clippy::unreadable_literal)]
    let _ = 1000000000;
    print!("{Equal:?}");
}

Current output

$ cargo check --all-targets
warning: this lint expectation is unfulfilled
 --> src/main.rs:6:10
  |
6 | #[expect(missing_docs)]
  |          ^^^^^^^^^^^^
  |
  = note: `#[warn(unfulfilled_lint_expectations)]` on by default

warning: `hello` (bin "hello" test) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.08s

Desired output

$ cargo check --all-targets
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.00s

Rationale and extra context

No response

Other cases

cargo clippy --all-targets has the same issue with clippy::enum_glob_use, however, there's no false positive on clippy::unreadable_literal.

Rust Version

rustc 1.81.0-beta.6 (b5fd9f6f1 2024-08-21)
binary: rustc
commit-hash: b5fd9f6f1061b79c045cc08fe03e00caad536800
commit-date: 2024-08-21
host: x86_64-unknown-linux-gnu
release: 1.81.0-beta.6
LLVM version: 18.1.7

Anything else?

Reported in rust-lang/rust-analyzer#17685.

@lnicola lnicola added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 6, 2024
@Urgau
Copy link
Member

Urgau commented Sep 6, 2024

Minimized:

// flags: rustc --test a.rs

#![warn(missing_docs)]

#[expect(missing_docs)]
pub fn foo() {}

cc @xFrednet

@Urgau Urgau added the F-lint_reasons `#![feature(lint_reasons)]` label Sep 6, 2024
@Urgau
Copy link
Member

Urgau commented Sep 6, 2024

This seems very relevant.

// If we're building a test harness, then warning about
// documentation is probably not really relevant right now.
if cx.sess().opts.test {
return;
}

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 8, 2024
…chenkov

Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang#130021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 8, 2024
…chenkov

Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang#130021
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Sep 9, 2024
…chenkov

Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang#130021
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 9, 2024
Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang#130021

try-job: x86_64-gnu-aux
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Sep 10, 2024
…chenkov

Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang#130021

try-job: x86_64-gnu-aux
@bors bors closed this as completed in 33855f8 Sep 10, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 11, 2024
Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang/rust#130021

try-job: x86_64-gnu-aux
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Sep 25, 2024
Also emit `missing_docs` lint with `--test` to fulfil expectations

This PR removes the "test harness" suppression of the `missing_docs` lint to be able to fulfil `#[expect]` (expectations) as it is now "relevant".

I think the goal was to maybe avoid false-positive while linting on public items under `#[cfg(test)]` but with effective visibility we should no longer have any false-positive.

Another possibility would be to query the lint level and only emit the lint if it's of expect level, but that is even more hacky.

Fixes rust-lang/rust#130021

try-job: x86_64-gnu-aux
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Nov 30, 2024
Work around rust-lang/rust#130021 which was fixed in Rust 1.83 and isn't
fixed for our MSRV at this time.
github-merge-queue bot pushed a commit to bytecodealliance/wasmtime that referenced this issue Dec 2, 2024
* Start using `#[expect]` instead of `#[allow]`

In Rust 1.81, our new MSRV, a new feature was added to Rust to use
`#[expect]` to control lint levels. This new lint annotation will
silence a lint but will itself cause a lint if it doesn't actually
silence anything. This is quite useful to ensure that annotations don't
get stale over time.

Another feature is the ability to use a `reason` directive on the
attribute with a string explaining why the attribute is there. This
string is then rendered in compiler messages if a warning or error
happens.

This commit migrates applies a few changes across the workspace:

* Some `#[allow]` are changed to `#[expect]` with a `reason`.
* Some `#[allow]` have a `reason` added if the lint conditionally fires
  (mostly related to macros).
* Some `#[allow]` are removed since the lint doesn't actually fire.
* The workspace configures `clippy::allow_attributes_without_reason = 'warn'`
  as a "ratchet" to prevent future regressions.
* Many crates are annotated to allow `allow_attributes_without_reason`
  during this transitionary period.

The end-state is that all crates should use
`#[expect(..., reason = "...")]` for any lint that unconditionally fires
but is expected. The `#[allow(..., reason = "...")]` lint should be used
for conditionally firing lints, primarily in macro-related code.
The `allow_attributes_without_reason = 'warn'` level is intended to be
permanent but the transitionary
`#[expect(clippy::allow_attributes_without_reason)]` crate annotations
to go away over time.

* Fix adapter build

prtest:full

* Fix one-core build of icache coherence

* Use `allow` for missing_docs

Work around rust-lang/rust#130021 which was fixed in Rust 1.83 and isn't
fixed for our MSRV at this time.

* More MSRV compat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints F-lint_reasons `#![feature(lint_reasons)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants