Add generic `allow-in-tests` configuration by dcsommer · Pull Request #17520 · rust-lang/rust-clippy

dcsommer · 2026-08-07T19:49:39Z

NOTE: some of the code for this PR originated from an LLM (Claude Opus 5), so I am disclosing this per the
disclosure guidelines. I (the human) reviewed all code and wrote the human-facing parts (PR description, documentation, error messages) myself, to comply with the LLM policy which also applies to rust-clippy repo.

Problem

Clippy has nine per-lint options controlling whether a lint applies to test code:

option	lint(s) affected
`allow-dbg-in-tests`	`dbg_macro`
`allow-expect-in-tests`	`expect_used`
`allow-indexing-slicing-in-tests`	`indexing_slicing`
`allow-large-stack-frames-in-tests`	`large_stack_frames`
`allow-panic-in-tests`	`panic`
`allow-print-in-tests`	`print_stderr`, `print_stdout`
`allow-unwrap-in-tests`	`unwrap_used`
`allow-useless-vec-in-tests`	`useless_vec`
`check-incompatible-msrv-in-tests`	`incompatible_msrv`

They all express the same idea: should this lint be applied to test code?. Each is implemented from scratch inside the lint it controls.

What each individual `allow-*-in-tests` costs to add

A recent proposed addition (#17516) of a single option (allow-panic-in-result-fn-in-tests) touches 8 files and adds 66 lines, of which only a handful were the actual behavior. None of this code is shared with the other allow-*-in-tests options.

Each allow-* option addition pollutes user-facing documentation in the form of extra search completions, CHANGELOG entries, and option entries.

It is also a burden on maintainers to continue reviewing PRs for each consequent allow-*.

Given the number of these allow-* options, I think we can safely assume this trend will continue, and the costs will continue to be accrued.

Behavioral drift

Because each lint implements the check itself, each has its own method of determining "are we inside test code currently?" Already there is variance: some use expr.hir_id, others local_def_id_to_hir_id(..). Nothing enforces consistency, despite the allow-*-in-tests naming scheme implying some sort of common implementation.

For users

One option name to learn and grep for to solve the problem of "how do I turn off lint X in a test?" instead of a bespoke name per lint would be a much better UX.

Solution

One new option, allow-in-tests = [...], which suppresses the lints specified in tests.

# Example: do not fire dbg_macro and unwrap_used lints inside test code
allow-in-tests = ["dbg_macro", "unwrap_used"]

Rather than threading a flag into each lint, suppression happens once at diagnostic emission. This avoids having to refactor every lint to be aware of allow-in-tests.

In order to support non-late-pass lints, there is a new AST pass to collect spans of test config code. When lints are emitted within those test-config spans, they are eligible for suppression via allow-in-tests.

As one would expect, if you specify a non-existent lint in the list for this option, you get an error.

Deprecation

The existing allow-*-in-tests options keep working but are deprecated in documentation only. Setting a legacy allow option does not warn for now. Each option's docs name the exact replacement line to write. #[replaced_by_allow_in_tests] records the mapping in machine-readable form so the warning can be switched on in a follow-up once allow-in-tests has shipped everywhere.

On naming `allow-in-tests`

allow-in-tests generalizes the allow-*-in-tests options it supersedes. allow-* is by far the most common prefix in clippy.toml (17 options), and it maps exactly onto #[allow]: the same operation, applied by configuration instead of by attribute. Sticking with historical precedent minimizes mental churn for users.

However, thinking ahead to an "inverse" option, one that enables a lint to apply to test code where it didn't before, its worth considering how this PR's new option and that follow-up option (coming in a later PR) would pair together. Here are the pros/cons for the considered naming schemes.

naming	pro	con
`allow-` / `check-` (chosen scheme)	Generalizes existing naming families one-to-one, so follows precedent. `allow` matches `#[allow]` semantics; `check` means "don't skip".	The two are not obvious opposites at a glance, so knowing one doesn't let you guess the other.
`allow-` / `disallow-`	Symmetric and guessable.	`disallow` risks "double negative" confusion as it is read.
`disable-` / `enable-`	Symmetric and guessable. States the effect in plain terms.	Contra existing precedent: it inverts the direction relative to all nine existing options. `allow-dbg-in-tests = true` means don't lint, so `disable-in-tests` would be its synonym and `enable-in-tests` its opposite. Every migrating user has to flip the "sign" of the configuration language (from `allow` to `disable`).
`skip-` / `check-`	Clean opposites that avoid lint-level vocabulary entirely.	Contra existing precedent for `allow-*`.

Prior art

This has been attempted once before, in #15600 and its continuation rust-lang/rust#156396. That approach suppressed lints whenever --test was passed to rustc, which also silenced them in non-test code. @y21 and @flip1995 independently identified the per-node check — what clippy_utils::is_in_test does, and what the existing allow-*-in-tests options do — as the behavior users actually expect, and the PR was closed on that basis. This PR uses that per-node check.

Checklist

Added passing UI tests (including committed .stderr files)
cargo test passes locally
Added documentation
Ran cargo dev fmt and cargo bless --test config-metadata

changelog: Added [allow-in-tests] configuration to suppress any lint in test code
changelog: Deprecated the per-lint allow-*-in-tests options in favor of allow-in-tests

Prior requests for this feature

This is not a new idea. It has been asked for repeatedly since 2016.

Community review:
I am not sure that this is a good idea.

I think it is not uncommon to want to allow .expect, but not dbg! in tests.

Therefore this seems like a lot of churn for some users and not an 100% pure win at that.

https://github.com/search?q=allow-expect-in-tests+language%3ATOML&type=code

https://github.com/search?q=allow-dbg-in-tests+language%3ATOML&type=code

View changes since the review

@CommanderStorm what in particular is not a good idea? Yes, the premise of this change is that things should be spelled differently, for the benefit of consistency and searchability going forward, as well as immediate extensibility to many more lints. Are you voting that such benefits are not worth the eventual respelling?

I think there are people for which this is an regression.
As said:

I think it is not uncommon to want to allow .expect, but not dbg! in tests.

Not everyone wants to opt into an "allow everything, including all the future things we will allow" switch, there are also others who I see wanting exactly this.
Thus, I think it is not a clear win

allow-in-tests doesn't allow everything though. It takes a list of specific lints to allow in tests. For the scenario you mentioned, you can get it with this:

allow-in-tests = ["expect_used"] # `.expect` allowed in tests, `dbg!` still linted

I've reworded these lines in the latest version to be clearer.

blyxyas · 2026-08-10T15:40:01Z

Was AI used in this PR? If so, we require disclosure.

dcsommer · 2026-08-10T16:31:55Z

Was AI used in this PR? If so, we require disclosure.

Thanks for the flag. Yes, I used AI (Opus 5) and will figure out where to disclose that.

blyxyas · 2026-09-07T22:34:59Z

Ping @dcsommer 🏓! Are you there?

dcsommer · 2026-09-08T14:09:13Z

this big of a change probably will need an FCP, but some community review

Who determines this? How do I know if I should make an FCP?

DanielEScherzer · 2026-09-08T15:43:10Z

this big of a change probably will need an FCP, but some community review

Who determines this? How do I know if I should make an FCP?

I think only team members can actually start an FCP (e.g. I can't), so this was mostly just a heads up to you that it might take some discussion before this gets merged

CommanderStorm · 2026-09-08T15:55:17Z

this big of a change probably will need an FCP, but some community review

Who determines this

Big user facing changes like new lints or configuration options just do.
See #t-clippy/fcp

We should write this process down btw 😉

How do I know if I should make an FCP

You cannot do an FCP, only people with merge rights can. Once a PR is ready, it gets the lint-nominated label applied.

@rustbot label +needs-fcp +llm-assisted

CommanderStorm · 2026-09-09T09:22:39Z

@dcsommer have you had a chance to review our policy? In particular regarding the docs, diagnostics and the PR description?

https://forge.rust-lang.org/policies/llm-usage.html#-banned

dcsommer · 2026-09-09T14:00:47Z

I'm reading it now, but it seems to be scoped to rust-lang/rust. Just checking to make sure it also is in scope here, and if so what brings it in scope.

…

On Wed, Sep 9, 2026, 2:23 AM Frank Elsinga ***@***.***> wrote: *CommanderStorm* left a comment (rust-lang/rust-clippy#17520) <#17520 (comment)> @dcsommer <https://github.com/dcsommer> have you had a chance to review our policy? In particular regarding the docs, diagnostics and the PR description? https://forge.rust-lang.org/policies/llm-usage.html#-banned — Reply to this email directly, view it on GitHub <#17520?email_source=notifications&email_token=AAIIKIDH4GJJWSDYGS37XKD5OEORTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJZHE2TGMJXGU2KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5599531754>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIIKIC5TTCPRFDUU4FHCYD5OEORTAVCNFSNUABEKJSXA33TNF2G64TZHMZDMOBVGA2DIMZ3JFZXG5LFHM2TAOJTGQ4DEMJZHCQXMAQ> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AAIIKIHEN3ICVK57HJBDRKT5OEORTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJZHE2TGMJXGU2KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AAIIKIARNS3XHSPBL3CZHFD5OEORTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJZHE2TGMJXGU2KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>

CommanderStorm · 2026-09-09T14:07:42Z

Yes, here is the link of clippy ratifying this

https://doc.rust-lang.org/nightly/clippy/development/llm_usage.html

Gri-ffin

Its quite...daunting to review this PR, its trying to do too much, it should probably be split into separate PRs.

View changes since this review

dcsommer · 2026-09-09T16:22:52Z

I can split out the "inverse" option if you want, so one option per PR.

…

On Wed, Sep 9, 2026, 7:22 AM Gri-ffin ***@***.***> wrote: ***@***.**** commented on this pull request. Its quite...daunting to review this PR, its trying to do too much, it should probably be split into separate PRs. *View changes since this review <https://triagebot.infra.rust-lang.org/gh-changes-since/rust-lang/rust-clippy/17520/c9d938fb4d39fe1c56362436ac944a8e4550f969..26c9a14848b5fc50e2b6ca5c03e5a74a3622c368>* — Reply to this email directly, view it on GitHub <#17520?email_source=notifications&email_token=AAIIKIHD5HYLI5T4OFGQ5T35OFRQ5A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJVGU3DEMRUGQ22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-5155622445>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIIKIBPK3R75VDHA3HDPAT5OFRQ5AVCNFSNUABEKJSXA33TNF2G64TZHMZDMOBVGA2DIMZ3JFZXG5LFHM2TAOJTGQ4DEMJZHCQXMAQ> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AAIIKIF22YYUUJRQK4SEQBT5OFRQ5A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJVGU3DEMRUGQ22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AAIIKIBZP434WVM5R42ZQ7T5OFRQ5A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJVGU3DEMRUGQ22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>

Clippy has accumulated seven per-lint options for suppressing a lint in test code, each implemented from scratch inside the lint it controls: - `allow-dbg-in-tests` (`dbg_macro`) - `allow-expect-in-tests` (`expect_used`) - `allow-indexing-slicing-in-tests` (`indexing_slicing`) - `allow-panic-in-tests` (`panic`) - `allow-print-in-tests` (`print_stderr`, `print_stdout`) - `allow-unwrap-in-tests` (`unwrap_used`) - `allow-useless-vec-in-tests` (`useless_vec`) `allow-in-tests` replaces them with one option naming lints directly. Suppression happens once at diagnostic emission in `clippy_utils`, so it covers every lint rather than needing per-lint plumbing, and it resolves test-ness against the same node rustc resolves `#[allow]` against. Every one of those seven lints is emitted from a late pass, because the existing technique needs a `TyCtxt` and a HIR node; early-pass lints could never have had such an option. They are covered here as well: they have no parent chain to walk, so a collector records the spans of `#[cfg(test)]` items and `#[test]` functions in `check_crate` and emission matches a lint's span against them. The walk is skipped entirely unless the option names a lint. The superseded options keep working and are deprecated in documentation only, with no warning on use; `#[replaced_by_allow_in_tests]` records the replacement so the warning can be turned on once this has shipped. Lint names are resolved against the declared lints at startup, and names which don't resolve are reported with a span into `clippy.toml`. changelog: Added [`allow-in-tests`] configuration to suppress any lint in test code changelog: Deprecated the per-lint `allow-*-in-tests` options in favor of [`allow-in-tests`]

dcsommer force-pushed the test-config branch from 79d8dc0 to 9a80385 Compare August 7, 2026 20:08

dcsommer marked this pull request as ready for review August 7, 2026 20:19

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. labels Aug 7, 2026

dcsommer mentioned this pull request Aug 7, 2026

Add allow-panic-in-result-fn-in-tests configuration #17516

Open

DanielEScherzer suggested changes Aug 8, 2026

View reviewed changes

CommanderStorm reviewed Aug 10, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added llm-assisted For PRs that were partially or completely done with assistance of AI/LLM tools needs-fcp PRs that add, remove, or rename lints and need an FCP labels Sep 8, 2026

dcsommer marked this pull request as draft September 9, 2026 02:35

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 9, 2026

dcsommer changed the title ~~Add generic allow-in-tests configuration~~ Add generic allow-in-tests (and inverse check-in-tests) configuration Sep 9, 2026

dcsommer force-pushed the test-config branch 4 times, most recently from 39817f9 to 26c9a14 Compare September 9, 2026 04:06

dcsommer marked this pull request as ready for review September 9, 2026 04:15

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 9, 2026

Gri-ffin reviewed Sep 9, 2026

View reviewed changes

dcsommer force-pushed the test-config branch from 26c9a14 to b657b66 Compare September 10, 2026 20:09

dcsommer marked this pull request as draft September 10, 2026 20:21

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 10, 2026

dcsommer changed the title ~~Add generic allow-in-tests (and inverse check-in-tests) configuration~~ Add generic allow-in-tests configuration Sep 10, 2026

+                  ///
+                  /// This supersedes the per-lint `allow-<lint>-in-tests` options, which are deprecated but
+                  /// still honored: a lint is suppressed in test code if it is listed here or its own
+                  /// `allow-<lint>-in-tests` option is set.

+              /// Resolves the lint names given in the `allow-in-tests` configuration and hands them to
+              /// `clippy_utils`, which drops their diagnostics when they are emitted from test code.
+              ///
+              /// Names which don't refer to a Clippy lint are reported and ignored.

               ## `allow-expect-in-tests`
               Whether `expect` should be allowed in test functions or `#[cfg(test)]`
+              Deprecated in favor of [`allow-in-tests`](#allow-in-tests), which works for any
+              lint. This option still works, but new configurations should use `allow-in-tests`.

+                                              concat!("`", $name_str, "` is deprecated"),
+                                          )
+                                          .with_help(format!(
+                                              "use `allow-in-tests = [{}]` instead, which works for any lint",

Uh oh!

Uh oh!

Conversation

dcsommer commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

What each individual allow-*-in-tests costs to add

Behavioral drift

For users

Solution

Deprecation

On naming allow-in-tests

Prior art

Checklist

Prior requests for this feature

Related

Uh oh!

DanielEScherzer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CommanderStorm Aug 10, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blyxyas commented Aug 10, 2026

Uh oh!

dcsommer commented Aug 10, 2026

Uh oh!

This comment has been minimized.

blyxyas commented Sep 7, 2026

Uh oh!

dcsommer commented Sep 8, 2026

Uh oh!

DanielEScherzer commented Sep 8, 2026

Uh oh!

CommanderStorm commented Sep 8, 2026

Uh oh!

CommanderStorm commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcsommer commented Sep 9, 2026 via email

Uh oh!

CommanderStorm commented Sep 9, 2026

Uh oh!

Gri-ffin left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcsommer commented Sep 9, 2026 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dcsommer commented Aug 7, 2026 •

edited

Loading

What each individual `allow-*-in-tests` costs to add

On naming `allow-in-tests`

DanielEScherzer left a comment •

edited by rustbot

Loading

CommanderStorm Aug 10, 2026 •

edited by rustbot

Loading

CommanderStorm commented Sep 9, 2026 •

edited

Loading

Gri-ffin left a comment •

edited by rustbot

Loading