Skip to content

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

Open
dcsommer wants to merge 1 commit into
rust-lang:masterfrom
dcsommer:master
Open

Add allow-panic-in-result-fn-in-tests configuration#17516
dcsommer wants to merge 1 commit into
rust-lang:masterfrom
dcsommer:master

Conversation

@dcsommer

@dcsommer dcsommer commented Aug 6, 2026

Copy link
Copy Markdown

NOTE: this PR was drafted with Opus 5. I (the human) reviewed all content, including by-hand fixes, prior to submission.

changelog: [panic_in_result_fn]: add allow-panic-in-result-fn-in-tests to allow panics and assertions in tests

allow-panic-in-tests only configures the panic lint and does not
suppress panic_in_result_fn.

Add a dedicated allow-panic-in-result-fn-in-tests configuration option. It uses the same expression-level test detection as the other allow-*-in-tests options and covers panic!, assert!, assert_eq!, and assert_ne!.

This intentionally retains the existing behavior for integration tests and benchmarks, which are compiled as ordinary targets rather than #[cfg(test)] code.

@dcsommer
dcsommer force-pushed the master branch 3 times, most recently from 7e677ee to c8932dc Compare August 7, 2026 15:25
Add a dedicated test exemption for `panic_in_result_fn` rather than
broadening `allow-panic-in-tests`, since this lint also covers assertions.
Apply it to test functions and code under `#[cfg(test)]`.

changelog: [`panic_in_result_fn`]: add `allow-panic-in-result-fn-in-tests`
to allow panics and assertions in tests
@dcsommer
dcsommer marked this pull request as ready for review August 7, 2026 16:43
@rustbot 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

dcsommer commented Aug 7, 2026

Copy link
Copy Markdown
Author

Note, I'd prefer to ship #17520 -- this PR is only a backup in case that is not accepted.

@rustbot

rustbot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #17607) made this pull request unmergeable. Please resolve the merge conflicts.

@CommanderStorm CommanderStorm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Community review:
LGTM assuming you clarify if the brackets are the way they should and if not extract the variables to make the code more readable

View changes since this review

Comment on lines +89 to +90
if !(is_inside_always_const_context(cx.tcx, e.hir_id)
|| allow_panic_in_result_fn_in_tests && is_in_test(cx.tcx, e.hir_id))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this bracketed the way you want? Did you not intend this?

likely with some extracting of variables, because the brackets are getting overbearing.

Suggested change
if !(is_inside_always_const_context(cx.tcx, e.hir_id)
|| allow_panic_in_result_fn_in_tests && is_in_test(cx.tcx, e.hir_id))
if (!(is_inside_always_const_context(cx.tcx, e.hir_id)
|| (allow_panic_in_result_fn_in_tests && is_in_test(cx.tcx, e.hir_id))))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. 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.

3 participants