Skip to content

Fix unwrap_expect_used in test modules defined by complex configuration predicates (#16369)#16989

Open
corentinguilloteau wants to merge 4 commits into
rust-lang:masterfrom
corentinguilloteau:issues/16369
Open

Fix unwrap_expect_used in test modules defined by complex configuration predicates (#16369)#16989
corentinguilloteau wants to merge 4 commits into
rust-lang:masterfrom
corentinguilloteau:issues/16369

Conversation

@corentinguilloteau
Copy link
Copy Markdown

@corentinguilloteau corentinguilloteau commented May 10, 2026

This PR fixes detection of test contexts in complex scenarios (e.g., #[cfg(all(test, ...))]). This allows the unwrap_used and expect_used lints to correctly suppress warnings in test contexts with compound cfg conditions.

Note that this also updates the behavior of lints that indirectly depend on is_cfg_test (around 20 lints).

Fixes #16369

changelog: [unwrap_expect_used]: Fix in test modules defined by complex configuration predicates

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 10, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 10, 2026

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, llogiq, samueltardieu

@rustbot

This comment has been minimized.

Copy link
Copy Markdown
Member

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Corentin, thanks for your contribution.

It would be great if you could handle the CfgEntry::Not case as well, or at least clearly state in the function documentation why it is not handled. There is a risk that some other code in Clippy wants to use cfg_implies_test() for the reverse purpose (ensure that we cannot be in a test settings), and this function would return None for #[cfg(not(test))] while the caller could expect it to return Some(false).

r? samueltardieu
@rustbot author

View changes since this review

Comment thread clippy_utils/src/lib.rs Outdated
Comment thread clippy_utils/src/lib.rs Outdated
@rustbot rustbot assigned samueltardieu and unassigned llogiq May 11, 2026
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 11, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 11, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@corentinguilloteau
Copy link
Copy Markdown
Author

I've reworked the initial implementation with a new approach that handles more configuration predicates than the old one (most notably, it now handles the CfgEntry::Not case).
Solving the general problem would likely require a SAT solver, which feels like unnecessary complexity here. This new algorithm remains comparable in complexity to the previous one while covering a vast majority of real-world predicates.

I've also improved the API so that using cfg_implies_test_is() for the reverse use case becomes less ambiguous.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow-unwrap-in-tests does not work in unit test that are also gated by a feature flag

4 participants