Skip to content

Fix manual unwrap or lint to respect msrv for unwrap or default() #14879

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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

poglesbyg
Copy link

changelog: [unwrap_or_default]: Fix MSRV attribute handling to properly respect per-item MSRV settings

This PR fixes an issue where the unwrap_or_default lint was not respecting per-item MSRV attributes. Previously, it would suggest using unwrap_or_default() even when the MSRV was set below 1.16 (when this method was stabilized).

The fix adds extract_msrv_attr!(); to the check_expr method in the Methods lint pass implementation, ensuring that all method lints properly extract and respect per-item MSRV attributes.

Added a test case in tests/ui/msrv_unwrap_or_default.rs that verifies a function annotated with #[clippy::msrv = "1.15"] using unwrap_or(vec![]) does not trigger the unwrap_or_default lint.

This change affects all method lints that check for MSRV-dependent features, making them more accurate and useful for codebases that need to maintain compatibility with older Rust versions.

poglesbyg added 2 commits May 23, 2025 12:36
The `manual_unwrap_or` lint was incorrectly suggesting `unwrap_or_default()` when the MSRV was set to 1.15 or lower, even though `unwrap_or_default()` was only stabilized in Rust 1.16.

Changes made:
1. Fixed the MSRV check in `manual_unwrap_or.rs` to use the correct constant `msrvs::UNWRAP_OR_DEFAULT` instead of `msrvs::STR_REPEAT`
2. Added the `UNWRAP_OR_DEFAULT` constant to the MSRV aliases in `msrvs.rs`, marking it as stabilized in Rust 1.16

This ensures that the lint will only suggest using `unwrap_or_default()` when the MSRV is at least 1.16.

changelog: Fix [`manual_unwrap_or`] false positive when MSRV is below 1.16

On branch Fix-`manual_unwrap_or`-lint-to-respect-MSRV-for-`unwrap_or_default()`
Changes to be committed:
	modified:   clippy_lints/src/matches/manual_unwrap_or.rs
	modified:   clippy_lints/src/matches/mod.rs
	modified:   clippy_utils/src/msrvs.rs
…fault`) were not respecting per-item MSRV attributes. The fix ensures that when a function or module is annotated with `#[clippy::msrv = "1.15"]`, method lints will properly respect that MSRV version.

Previously, method lints like `unwrap_or_default` would suggest using `unwrap_or_default()` even when the MSRV was set below 1.16 (when this method was stabilized). This happened because the `extract_msrv_attr!` macro was not being used in the `Methods` lint pass, so per-item MSRV attributes were not being respected.

Added `extract_msrv_attr!();` to the `check_expr` method in the `Methods` lint pass implementation. This ensures that all method lints properly extract and respect per-item MSRV attributes.

Added a new test case in `tests/ui/msrv_unwrap_or_default.rs` that verifies:
- A function annotated with `#[clippy::msrv = "1.15"]` using `unwrap_or(vec![])` does not trigger the `unwrap_or_default` lint
- The test passes with only the expected unused variable warning

This change affects all method lints that check for MSRV-dependent features. The fix ensures that these lints properly respect per-item MSRV attributes, making them more accurate and useful for codebases that need to maintain compatibility with older Rust versions.

Fixes #[issue number] (if there is an open issue for this)

- [x] Added test case
- [x] Test passes
- [x] No regression in other lints
- [x] Documentation is up to date
@rustbot
Copy link
Collaborator

rustbot commented May 23, 2025

r? @samueltardieu

rustbot has assigned @samueltardieu.
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

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

rustbot commented May 23, 2025

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please remove them as they will spam the issue with references to the commit.

samueltardieu

This comment was marked as duplicate.

Copy link
Contributor

@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.

  • You forgot to remove the rules files for your LLM.
  • Option::unwrap_or_default() is not an issue, the issue is only Result::unwrap_or_default()
  • extract_msrv_attr!() is for early lint passes, the LLM got it wrong

@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 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants