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
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