rust-lang/rust-clippy

Strange suggestion from suspicious_operation_groupings

Open

#6,747 opened on Feb 16, 2021

View on GitHub
 (5 comments) (2 reactions) (1 assignee)Rust (10,406 stars) (1,391 forks)batch import
C-bugI-false-positivegood first issue

Description

Lint name: suspicious_operation_groupings

I tried this code:

    pub fn set_scheme(&mut self, scheme: &str) -> Result<(), ()> {
        let mut parser = Parser::for_setter(String::new());
        let remaining = parser.parse_scheme(parser::Input::new(scheme))?;
        let new_scheme_type = SchemeType::from(&parser.serialization);
        let old_scheme_type = SchemeType::from(self.scheme());
        // If url’s scheme is a special scheme and buffer is not a special scheme, then return.
        if (new_scheme_type.is_special() && !old_scheme_type.is_special()) ||
            // If url’s scheme is not a special scheme and buffer is a special scheme, then return.
            (!new_scheme_type.is_special() && old_scheme_type.is_special()) ||
            // If url includes credentials or has a non-null port, and buffer is "file", then return.
            // If url’s scheme is "file" and its host is an empty host or null, then return.
            (new_scheme_type.is_file() && self.has_authority())
        {
            return Err(());
        }

I expected to see this happen: accepted without warnings.

Instead, this happened:

error: This sequence of operators looks suspiciously like a bug.
Error:     --> url/src/lib.rs:2098:14
     |
2098 |             (new_scheme_type.is_file() && self.has_authority())
     |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: I think you meant: `new_scheme_type.is_file() && self.is_file()`
     |
     = note: `-D clippy::suspicious-operation-groupings` implied by `-D warnings`
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_operation_groupings

Meta

  • cargo clippy -V: clippy 0.0.212 (cb75ad5d 2021-02-10)
  • rustc -Vv:

rustc 1.50.0 (cb75ad5db 2021-02-10) binary: rustc commit-hash: cb75ad5db02783e8b0222fee363c5f63f7e2cf5b commit-date: 2021-02-10 host: x86_64-apple-darwin release: 1.50.0



<!-- TRIAGEBOT_START -->

<!-- TRIAGEBOT_ASSIGN_START -->

<!-- TRIAGEBOT_ASSIGN_END -->
<!-- TRIAGEBOT_END -->

Contributor guide

Tech stack
rust
Domain
toolingdeveloper experience
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Rust programmingUnderstanding of clippy lint structure
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
40
Research direction
Investigate the false positive in clippy's `suspicious operation groupings` lint. The issue is that the lint incorrectly suggests replacing `self.has authority()` with `self.is file()`. Look at the lint implementation in the clippy source code, specifically the function that checks for suspicious operation groupings. The provided code snippet shows the condition that triggers the false positive. The fix may involve adjusting the lint's logic to not flag cases where the operands are of different types or are method calls on different objects.
Strange suggestion from suspicious_operation_groupings · rust-lang/rust-clippy#6747 | Good First Issue