Skip to content

fix(lints): avoid method-name false positives in suspicious_operation… - #17580

Open
NicDevTV wants to merge 4 commits into
rust-lang:masterfrom
NicDevTV:fix/6747-suspicious-operation-groupings
Open

fix(lints): avoid method-name false positives in suspicious_operation…#17580
NicDevTV wants to merge 4 commits into
rust-lang:masterfrom
NicDevTV:fix/6747-suspicious-operation-groupings

Conversation

@NicDevTV

@NicDevTV NicDevTV commented Aug 17, 2026

Copy link
Copy Markdown

Fix a false positive in suspicious_operation_groupings

Summary

suspicious_operation_groupings treated different method names as identifier changes and could suggest invalid replacements, such as replacing has_authority() with is_file().

This change compares method names before calculating identifier differences. Expressions using different methods are treated as incompatible, while receiver mismatches with the same method name are still detected.

Added regression tests for both cases.

Testing

  • cargo fmt --all -- --check
  • git diff --check
  • UI test could not be run locally because this repository requires Nightly Rust and rustc_private; only Stable Rust is installed.

changelog: [suspicious_operation_groupings]: avoid false-positive suggestions for expressions that call different methods

fixes #6747

@rustbot rustbot added 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 labels Aug 17, 2026
@rustbot

rustbot commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome!

You should hear from one of our reviewers after this PR gets at least 2 reviews from the community.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@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, though I suspect this creates a false negative somewhat..
Better though given that not confusing...
🤷🏻‍♂️

View changes since this review

Comment on lines +588 to +594
(MethodCall(left, ..), MethodCall(right, ..)) => {
// A method name is part of the expression's structure, not a
// candidate for the identifier swap this lint is looking for.
if !eq_id(left.seg.ident, right.seg.ident) {
return (IdentDifference::NonIdent, base);
}
},

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.

lets simplify..

Also I don't think the comment is very insightfull and mostly just stating the obvious...

Suggested change
(MethodCall(left, ..), MethodCall(right, ..)) => {
// A method name is part of the expression's structure, not a
// candidate for the identifier swap this lint is looking for.
if !eq_id(left.seg.ident, right.seg.ident) {
return (IdentDifference::NonIdent, base);
}
},
(MethodCall(left, ..), MethodCall(right, ..)) if eq_id(left.seg.ident, right.seg.ident) => {
// keep going
},

@CommanderStorm

Copy link
Copy Markdown
Contributor

make sure to squash your commits so that this does not create an unnessary review cycle..

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.

Strange suggestion from suspicious_operation_groupings

3 participants