Skip to content

needless_match false positive #9084

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

Closed
senekor opened this issue Jul 1, 2022 · 1 comment · Fixed by #9092
Closed

needless_match false positive #9084

senekor opened this issue Jul 1, 2022 · 1 comment · Fixed by #9092
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@senekor
Copy link

senekor commented Jul 1, 2022

Summary

In the following snipped, clippy marks the match statement as needless and provides an incorrect quick fix.

let next_brightness = match self.1 {
    _ if algorithm.0[0] == Brightness::Dark => self.1,
    Brightness::Light => Brightness::Dark,
    Brightness::Dark => Brightness::Light,
};

Lint Name

needless_match

Reproducer

I tried this code: (advent of code, year 21 day 20)

use std::collections::HashMap;
use std::str::FromStr;

struct Algorithm(Vec<Brightness>);

#[derive(PartialEq, Eq, Clone, Copy)]
enum Brightness {
    Light,
    Dark,
}

#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, PartialOrd, Ord)]
struct Position {
    x: i32,
    y: i32,
}

struct TrenchMap(HashMap<Position, Brightness>, Brightness);

struct Input(Algorithm, TrenchMap);

impl FromStr for Brightness {
    type Err = String;
    fn from_str(s: &str) -> Result<Self, String> {
        match s {
            "#" => Ok(Brightness::Light),
            "." => Ok(Brightness::Dark),
            _ => panic!(),
        }
    }
}

impl FromStr for Input {
    type Err = String;
    fn from_str(s: &str) -> Result<Self, String> {
        let mut iter = s.split("\n\n");
        let algorithm = iter.next().unwrap().parse().unwrap();
        let trench_map = iter.next().unwrap().parse().unwrap();
        Ok(Input(algorithm, trench_map))
    }
}

impl FromStr for Algorithm {
    type Err = String;
    fn from_str(input: &str) -> Result<Self, String> {
        Ok(Algorithm(
            input
                .chars()
                .map(|c| c.to_string().parse().unwrap())
                .collect(),
        ))
    }
}

impl FromStr for TrenchMap {
    type Err = String;
    fn from_str(input: &str) -> Result<Self, String> {
        let mut trench_map = TrenchMap(HashMap::<Position, Brightness>::new(), Brightness::Dark);
        for (y, line) in input.lines().enumerate() {
            for (x, brightness) in line.chars().enumerate() {
                let new_pos = Position {
                    x: x as i32,
                    y: y as i32,
                };
                trench_map.insert(new_pos, brightness.to_string().parse().unwrap());
            }
        }
        Ok(trench_map)
    }
}

impl TrenchMap {
    fn insert(&mut self, position: Position, brightness: Brightness) {
        self.0.insert(position, brightness);
    }

    fn extend_indexes_per_position(
        &self,
        pos: Position,
        x_axis: i32,
        y_axis: i32,
        index_dual: &mut Vec<usize>,
    ) {
        let cal_pos = Position {
            x: pos.x - 1 + x_axis,
            y: pos.y - 1 + y_axis,
        };
        match self.0.get(&cal_pos).unwrap_or(&self.1) {
            Brightness::Light => index_dual.push(1),
            Brightness::Dark => index_dual.push(0),
        }
    }

    fn convert(&self, current_pos: Position, algorithm: &Algorithm) -> Brightness {
        let mut index_dual = Vec::new();
        for y_axis in 0..3 {
            for x_axis in 0..3 {
                self.extend_indexes_per_position(current_pos, x_axis, y_axis, &mut index_dual);
            }
        }
        let index_decimal = self.get_index_decimal(index_dual);
        algorithm.0[index_decimal]
    }

    fn above_and_below(&mut self, min: &Position, max: &Position) {
        for i in min.x - 1..=max.x + 1 {
            let position_above = Position { x: i, y: min.y - 1 };
            self.insert(position_above, self.1);
            let position_below = Position { x: i, y: max.y + 1 };
            self.insert(position_below, self.1);
        }
    }

    fn left_and_right(&mut self, min: &Position, max: &Position) {
        for i in min.y - 1..=max.y + 1 {
            let position_left = Position { x: min.x - 1, y: i };
            self.insert(position_left, self.1);
            let position_right = Position { x: max.x + 1, y: i };
            self.insert(position_right, self.1);
        }
    }

    fn increase_border(&mut self) {
        let &min = self.0.keys().min().unwrap();
        let &max = self.0.keys().max().unwrap();
        self.above_and_below(&min, &max);
        self.left_and_right(&min, &max);
    }

    fn enhance(&self, algorithm: &Algorithm) -> TrenchMap {
        let next_brightness = match self.1 {
            _ if algorithm.0[0] == Brightness::Dark => self.1,
            Brightness::Light => Brightness::Dark,
            Brightness::Dark => Brightness::Light,
        };
        let mut enhance = TrenchMap(HashMap::<Position, Brightness>::new(), next_brightness);
        for (i, _) in self.0.iter() {
            let brightness = self.convert(*i, algorithm);
            enhance.insert(*i, brightness);
        }
        enhance.increase_border();
        enhance
    }

    fn get_index_decimal(&self, index_dual: Vec<usize>) -> usize {
        let mut index_dec = 0;
        for (i, value) in index_dual.iter().rev().enumerate() {
            index_dec += (1 << i) * value;
        }
        index_dec
    }

    fn count(&self) -> i32 {
        let mut count = 0;
        for (_, brightness) in self.0.iter() {
            if *brightness == Brightness::Light {
                count += 1
            };
        }
        count
    }
}

pub fn part1_and_2(input: &str, enhancements: i32) -> i32 {
    let Input(algorithm, mut trench_map) = input.parse().unwrap();
    trench_map.increase_border();
    for _ in 0..enhancements {
        trench_map = trench_map.enhance(&algorithm);
    }
    trench_map.count()
}

I saw this happen:

this match expression is unnecessary
`#[warn(clippy::[needless_match](https://rust-lang.github.io/rust-clippy/master/index.html#needless_match))]` on by default
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_match
lib.rs(131, 31): replace it with: `self.1`

I expected to see this happen:

no warning

Version

rustc 1.61.0
binary: rustc
commit-hash: unknown
commit-date: unknown
host: aarch64-apple-darwin
release: 1.61.0
LLVM version: 14.0.0

this is weird though, I'm pretty sure this bug was introduced with 1.62
rustup show:

// snip
active toolchain
----------------

stable-x86_64-apple-darwin (default)
rustc 1.62.0 (a8314ef7d 2022-06-27)


### Additional Labels

@rustbot label +I-suggestion-causes-error

<!-- TRIAGEBOT_START -->

<!-- TRIAGEBOT_ASSIGN_START -->

<!-- TRIAGEBOT_ASSIGN_DATA_START$${"user":"tamaroning"}$$TRIAGEBOT_ASSIGN_DATA_END -->

<!-- TRIAGEBOT_ASSIGN_END -->
<!-- TRIAGEBOT_END -->
@senekor senekor added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jul 1, 2022
@tamaroning
Copy link
Contributor

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants