Skip to content

Deny unfulfilled_lint_expectations to make sure we honor #[expect] #15723

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Deny unfulfilled_lint_expectations to make sure we honor #[expect] #15723

wants to merge 2 commits into from

Conversation

samueltardieu
Copy link
Member

Using it in Cargo.toml makes sure we won't by default not honor a #[expect] in a test if, for example, the node on which the lint is posted changes, and the modification to the standard error is not carefully reviewed.

changelog: none

@samueltardieu
Post lint on the type HIR node
392396f 
Out of the 5 lints in the `derive` directory, only two of them
(`unsafe_derive_deserialize` and `derive_partial_eq_without_eq`) posted
the lint on the ADT node. This fixes the situation for the three other
lints:

- `derive_hash_with_manual_eq`
- `derive_ord_xor_partial_ord`
- `expl_impl_clone_on_copy`

This allows `#[expect]` to be used on the ADT to silence the lint.

Also, this makes `expl_impl_clone_on_copy` properly use an "help" message
instead of a "note" one when suggesting a fix.
@samueltardieu
Deny unfulfilled_lint_expectations to make sure we honor #[expect]
215b8bc 
Using it in `Cargo.toml` makes sure we won't by default not honor a
`#[expect]` in a test if, for example, the node on which the lint is
posted changes, and the modification to the standard error is not
carefully reviewed.
@samueltardieu
Copy link
Member Author

The PR includes #15709 as the first commit to illustrate what it would replace.

@samueltardieu samueltardieu mentioned this pull request Sep 19, 2025
Merged
@github-actions GitHub Actions
Copy link

Lintcheck changes for 215b8bc

Lint Added Removed Changed
clippy::expl_impl_clone_on_copy 0 0 326

This comment will be updated if you push new changes

@Alexendoo
Copy link
Member

Using it in Cargo.toml makes sure we won't by default not honor a #[expect] in a test

The root Cargo.toml wouldn't apply to tests since they either run clippy-driver manually or have their own Cargo.tomls

Isn't this already the case though? Tests are run with -Dwarnings and you'd have to add a //~^ marker for it

@samueltardieu
Copy link
Member Author

Using it in Cargo.toml makes sure we won't by default not honor a #[expect] in a test

The root Cargo.toml wouldn't apply to tests since they either run clippy-driver manually or have their own Cargo.tomls

Isn't this already the case though? Tests are run with -Dwarnings and you'd have to add a //~^ marker for it

Indeed!

@blyxyas
Copy link
Member

blyxyas commented Sep 21, 2025

Oh I was just checking out this PR to test this, good to know!

@samueltardieu
Copy link
Member Author

It's added in tests/compile-test.rs:

        config.program.args.extend(
            [
                "--emit=metadata",
                "-Aunused",
                "-Ainternal_features",
                "-Zui-testing",
                "-Zdeduplicate-diagnostics=no",
                "-Dwarnings",
            ]

@samueltardieu
Copy link
Member Author

(and here I learned that map() exists on arrays, I'm always converting them to an iterator first while it's not needed if you want an array as a result, or just even a IntoIterator)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samueltardieu @Alexendoo @blyxyas