rust-lang
Not linting irrefutable_let_patterns on let chains
Description
this PR makes the lint irrefutable_let_patterns not check for let chains,
only check for single if let, while let, and if let guard.
Motivation
Since let chains were stabilized, the following code has become common:
fn max() -> usize { 42 }
fn main() {
if let mx = max() && mx < usize::MAX { /* */ }
}
This code naturally expresses "please call that function and then do something if the return value satisfies a condition". Putting the let binding outside the if would be bad as then it remains in scope after the if, which is not the intent.
Current Output:
warning: leading irrefutable pattern in let chain
--> src/main.rs:7:8
|
7 | if let mx = max() && mx < usize::MAX {
| ^^^^^^^^^^^^^^
|
= note: this pattern will always match
= help: consider moving it outside of the construct
= note: `#[warn(irrefutable_let_patterns)]` on by default
Another common case is progressively destructuring a struct with enum fields, or an enum with struct variants:
struct NameOfOuterStruct {
middle: NameOfMiddleEnum,
other: (),
}
enum NameOfMiddleEnum {
Inner(NameOfInnerStruct),
Other(()),
}
struct NameOfInnerStruct {
id: u32,
}
fn test(outer: NameOfOuterStruct) {
if let NameOfOuterStruct { middle, .. } = outer
&& let NameOfMiddleEnum::Inner(inner) = middle
&& let NameOfInnerStruct { id } = inner
{
/* */
}
}
Current Output:
warning: leading irrefutable pattern in let chain
--> src\main.rs:17:8
|
17 | if let NameOfOuterStruct { middle, .. } = outer
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this pattern will always match
= help: consider moving it outside of the construct
= note: `#[warn(irrefutable_let_patterns)]` on by default
warning: trailing irrefutable pattern in let chain
--> src\main.rs:19:12
|
19 | && let NameOfInnerStruct { id } = inner
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this pattern will always match
= help: consider moving it into the body
To avoid the warning, the readability would be much worse:
fn test(outer: NameOfOuterStruct) {
if let NameOfOuterStruct {
middle: NameOfMiddleEnum::Inner(NameOfInnerStruct { id }),
..
} = outer
{
/* */
}
}
related issue
- rust-lang/rust#139369
possible questions
-
Moving the irrefutable pattern at the head of the chain out of it would cause a variable that was intended to be temporary to remain in scope, so we remove it. However, should we keep the check for moving the irrefutable pattern at the tail into the body?
-
Should we still lint
entire chain is made up of irrefutable let?
This is my first time contributing non-documentation code to Rust. If there are any irregularities, please feel free to point them out. : )
Failed to set assignee to Travis: invalid assignee
Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.
The job pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking miri v0.1.0 (/checkout/src/tools/miri)
error: this lint expectation is unfulfilled
--> src/tools/miri/src/shims/foreign_items.rs:903:26
|
903 | #[expect(irrefutable_let_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D unfulfilled-lint-expectations` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(unfulfilled_lint_expectations)]`
The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
--> src/tools/miri/src/shims/foreign_items.rs:903:20
|
903 | if let res = shims::math::EvalContextExt::emulate_foreign_item_inner(
| ____________________^
904 | | this, link_name, abi, args, dest,
905 | | )? && !matches!(res, EmulateItemResult::NotSupported)
| |__________________^
|
= note: this pattern will always match
= help: consider moving it outside of the construct
= note: `-D irrefutable-let-patterns` implied by `-D warnings`
The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
error in revision `disallowed`: ui test did not emit an error
note: by default, ui tests are expected not to compile.
hint: use check-pass, build-pass, or run-pass directive to change this behavior.
status: exit status: 0
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--cfg" "disallowed" "--check-cfg" "cfg(test,FALSE,allowed,disallowed)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.disallowed" "-A" "unused" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "--edition=2024"
stdout: none
stderr: none
---- [ui] tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs#disallowed stdout end ----
Some changes occurred in match checking
cc @Nadrieril
The Miri subtree was changed
cc @rust-lang/miri
It seems that only modifying tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs is needed to cover this change. Do I still need to add an additional new test?
It seems that only modifying
tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rsis needed to cover this change. Do I still need to add an additional new test?
All the lints seem to be removed in that test, but you'll want to ensure there are tests that cover the edges of what is still linted here.
@rustbot author
The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Compiling rustc_mir_transform v0.0.0 (/checkout/compiler/rustc_mir_transform)
error: this lint expectation is unfulfilled
--> compiler/rustc_mir_transform/src/check_alignment.rs:85:14
|
85 | #[expect(irrefutable_let_patterns)]
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D unfulfilled-lint-expectations` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(unfulfilled_lint_expectations)]`
I tried adding a new test file, but I don't know why ./x test tests/ui/binding/irrefutable-in-let-chains.rs --bless is not generating the .stderr file. I'm not sure what I did incorrectly.
: (
edit: adding //@ check-pass can fix it : )
The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
error: ui test did not emit an error
note: by default, ui tests are expected not to compile.
hint: use check-pass, build-pass, or run-pass directive to change this behavior.
status: exit status: 0
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/binding/irrefutable-in-let-chains.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/binding/irrefutable-in-let-chains" "-A" "unused" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "--edition=2024"
stdout: none
--- stderr -------------------------------
warning: irrefutable `if let` pattern
##[warning] --> /checkout/tests/ui/binding/irrefutable-in-let-chains.rs:10:8
|
---
warning: irrefutable `if let` pattern
##[warning] --> /checkout/tests/ui/binding/irrefutable-in-let-chains.rs:44:8
|
LL | if let Range { start: local_start, end: _ } = (None..Some(1)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this pattern will always match, so the `if let` is useless
= help: consider replacing the `if let` with a `let`
---
warning: irrefutable `if let` pattern
##[warning] --> /checkout/tests/ui/binding/irrefutable-in-let-chains.rs:111:15
|
LL | } else if let x = opt.clone().map(|_| 1) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this pattern will always match, so the `if let` is useless
= help: consider replacing the `if let` with a `let`
This will need an FCP by the lang team. For that, this needs a description of what we'll be stabilizing exactly and the reasons for it. Have a look at other PRs that are labeled T-lang and finished-final-comment-period to see examples of this.
@rustbot author
this needs a description of what we'll be stabilizing exactly and the reasons for it. Have a look at other PRs that are labeled
T-langandfinished-final-comment-periodto see examples of this.
Is there anything I can do to move this forward?
I have updated the top comment of this PR. Does it now have the proper format for a FCP? If so, what are the next steps to get it started?
If so, what are the next steps to get it started?
No one answered, but the answer is "add the I-lang-nominated label and wait for the lang team to look at it", which @traviscross did. So there's nothing more to do for you until then :)
Hi, I really hope this PR can be merged before the 1.91 release. Is there any progress now : )
I really hope this PR can be merged before the 1.91 release.
That's not possible, master is 1.92 already and 1.91 is beta meaning the latter only accepts patches for stable-to-beta and stable-to-stable regressions (also, 1.91 will be promoted to stable next week).
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:
- [x] @joshtriplett
- [ ] @nikomatsakis
- [ ] @scottmcm
- [x] @tmandry
- [x] @traviscross
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.
:warning: Warning :warning:
-
This PR is based on an upstream commit that is older than 28 days.
It's recommended to update your branch according to the rustc-dev-guide.
The job aarch64-gnu-llvm-20-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
fmt check
fmt: checked 6521 files
tidy check
tidy [rustdoc_json (src)]: `rustdoc-json-types` modified, checking format version
tidy [fluent_used (compiler)]: /checkout/compiler/rustc_mir_build/messages.ftl: message `mir_build_trailing_irrefutable_let_patterns` is not used
tidy [fluent_used (compiler)]: /checkout/compiler/rustc_mir_build/messages.ftl: message `mir_build_leading_irrefutable_let_patterns` is not used
tidy [fluent_used (compiler)]: FAIL
tidy: Skipping binary file check, read-only filesystem
tidy: The following check failed: fluent_used (compiler)
Command `/checkout/obj/build/aarch64-unknown-linux-gnu/stage1-tools-bin/rust-tidy /checkout /checkout/obj/build/aarch64-unknown-linux-gnu/stage0/bin/cargo /checkout/obj/build 4 npm` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:1549:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:1280:29
Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `--stage 2 test --skip tests --skip coverage-map --skip coverage-run --skip library --skip tidyselftest`
Build completed unsuccessfully in 0:00:48
local time: Sun Nov 2 01:56:22 UTC 2025
network time: Sun, 02 Nov 2025 01:56:22 GMT
##[error]Process completed with exit code 1.
The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
fmt check
fmt: checked 6521 files
tidy check
tidy [rustdoc_json (src)]: `rustdoc-json-types` modified, checking format version
tidy [fluent_used (compiler)]: /checkout/compiler/rustc_mir_build/messages.ftl: message `mir_build_leading_irrefutable_let_patterns` is not used
tidy [fluent_used (compiler)]: /checkout/compiler/rustc_mir_build/messages.ftl: message `mir_build_trailing_irrefutable_let_patterns` is not used
tidy [fluent_used (compiler)]: FAIL
tidy: Skipping binary file check, read-only filesystem
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' and 'venv'
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' and 'virtualenv'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (25.2)
---
linting javascript files and applying suggestions
Running eslint on rustdoc JS files
info: ES-Check: there were no ES version matching errors! 🎉
typechecking javascript files
tidy: The following check failed: fluent_used (compiler)
Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools-bin/rust-tidy /checkout /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo /checkout/obj/build 4 /node/bin/npm --extra-checks=py,cpp,js,spellcheck` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:1549:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:1280:29
Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `test src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`
Build completed unsuccessfully in 0:03:13
local time: Sun Nov 2 02:10:12 UTC 2025
network time: Sun, 02 Nov 2025 02:10:12 GMT
##[error]Process completed with exit code 1.
The job aarch64-gnu-llvm-20-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
fmt check
fmt: checked 6521 files
tidy check
tidy [rustdoc_json (src)]: `rustdoc-json-types` modified, checking format version
tidy [fluent_used (compiler)]: /checkout/compiler/rustc_mir_build/messages.ftl: message `mir_build_leading_irrefutable_let_patterns` is not used
tidy [fluent_used (compiler)]: /checkout/compiler/rustc_mir_build/messages.ftl: message `mir_build_trailing_irrefutable_let_patterns` is not used
tidy [fluent_used (compiler)]: FAIL
tidy: Skipping binary file check, read-only filesystem
tidy: The following check failed: fluent_used (compiler)
Command `/checkout/obj/build/aarch64-unknown-linux-gnu/stage1-tools-bin/rust-tidy /checkout /checkout/obj/build/aarch64-unknown-linux-gnu/stage0/bin/cargo /checkout/obj/build 4 npm` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:1549:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:1280:29
Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `--stage 2 test --skip tests --skip coverage-map --skip coverage-run --skip library --skip tidyselftest`
Build completed unsuccessfully in 0:00:48
local time: Sun Nov 2 03:20:53 UTC 2025
network time: Sun, 02 Nov 2025 03:20:53 GMT
##[error]Process completed with exit code 1.
Note that the listed motivation
if let mx = max() && mx < usize::MAX { /* */ }
could of course be written as
if let mx @ ..usize::MAX = max() { /* */ }
which would make it refutable again.
To avoid the warning, the readability would be much worse:
Well, it could also be
fn test(outer: NameOfOuterStruct) {
let NameOfOuterStruct { middle, .. } = outer
if let NameOfMiddleEnum::Inner(inner) = middle
&& let NameOfInnerStruct { id } = inner
{
/* */
}
}
where the readability is fine?
