Skip to content

fix: collapsible_match suggests ref/derefs when needed #14221

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 1 commit into
base: master
Choose a base branch
from

Conversation

vishruth-thimmaiah
Copy link
Contributor

Fixes: #14155

If an expression is borrowed/dereferenced, suggest using .as_ref() or .copied() respectively in the outer conditional statement.

changelog: [collapsible_match] suggests ref/dereferencing when needed.

@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2025

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 15, 2025
@rustbot

This comment has been minimized.

@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 Mar 31, 2025
@nk9
Copy link

nk9 commented Apr 7, 2025

Checking in here. It looks to me like the state is wrong now. Aren't we actually waiting on review?

@samueltardieu
Copy link
Contributor

We are expecting both a review and a rebase. The bot settings have been changed a few days ago to let the PR in review state, so I'll reset it.

@rustbot review

@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 Apr 8, 2025
@vishruth-thimmaiah vishruth-thimmaiah force-pushed the fix_collapsible_match_ref branch 2 times, most recently from bc85de9 to 0dc1cdc Compare April 9, 2025 17:17
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.

It looks like you ignore the fact that the reference might be mutable. The following code will suggest using .as_ref():

    let mut arr = ["a", "b", "c"];
    if let Some(mut last) = arr.last_mut() {
        match &mut last {
            &mut &mut "a" | &mut &mut "b" => {
                unimplemented!()
            },
            _ => (),
        }
    }

The suggestion might also be incorrect in case of raw references, where the types may not match (untested):

    const NULL_PTR: *const &'static str = std::ptr::null();
    if let Some(last) = arr.last() {
        match &raw const *last {
            NULL_PTR => unimplemented!(),
            _ => (),
        }
    }

@vishruth-thimmaiah vishruth-thimmaiah force-pushed the fix_collapsible_match_ref branch from 0dc1cdc to 6116061 Compare April 12, 2025 18:10
@vishruth-thimmaiah
Copy link
Contributor Author

I couldn't figure out any way to handle &raw refs, so I just disabled the lint for &raw refs. If you have any suggestions on how to handle them, I'd appreciate it!

@vishruth-thimmaiah vishruth-thimmaiah force-pushed the fix_collapsible_match_ref branch from 6116061 to 3efde9f Compare April 12, 2025 18:19
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.

collapsible_match suggestion incomplete
5 participants