silently fails to format match expression if arm pat contains comment

[bradjc]

For Tock, we run rustfmt with Travis CI, and some PRs (example1 and example2) are passing a rustfmt check while still using tabs for indentation.

• No config options (we use defaults only).
• Noticed this on nightly-2020-03-06 and nightly-2020-04-10.

I've also tried manually running rustfmt in the relevant crate on my computer and see the same effect. Based on a question on discord, I am opening this issue.

One example (src):

impl Driver for L3gd20Spi<'a> {
    fn command(&self, command_num: usize, data1: usize, data2: usize, _: AppId) -> ReturnCode {
        match command_num {
            0 /* check if present */ => ReturnCode::SUCCESS,
            // Check is sensor is correctly connected
            1 => {
				if self.status.get () == L3gd20Status::Idle {
					self.is_present ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}

			}
			// Power On
            2 => {
				if self.status.get () == L3gd20Status::Idle {
					self.power_on ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
			}
			// Set Scale
            3 => {
				if self.status.get () == L3gd20Status::Idle {
					let scale = data1 as u8;
					self.set_scale (scale);
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
			// Enable High Pass Filter
            4 => {
				if self.status.get () == L3gd20Status::Idle {
					let mode = data1 as u8;
					let divider = data2 as u8;
					self.set_hpf_parameters (mode, divider);
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
			}
			// Set High Pass Filter Mode and Divider
            5 => {
				if self.status.get () == L3gd20Status::Idle {
					let enabled = if data1 == 1 { true } else { false };
					self.enable_hpf (enabled);
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
			// Read XYZ
            6 => {
				if self.status.get () == L3gd20Status::Idle {
					self.read_xyz ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
			}
			// Read Temperature
            7 => {
				if self.status.get () == L3gd20Status::Idle {
					self.read_temperature ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
            // default
            _ => ReturnCode::ENOSUPPORT,
        }
    }

    fn subscribe(
        &self,
        subscribe_num: usize,
        callback: Option<Callback>,
        _app_id: AppId,
    ) -> ReturnCode {
        match subscribe_num {
            0 /* set the one shot callback */ => {
				self.callback.insert (callback);
				ReturnCode::SUCCESS
			},
            // default
            _ => ReturnCode::ENOSUPPORT,
        }
    }
}

Another example (src):

impl Driver for Lsm303dlhcI2C<'a> {
    fn command(&self, command_num: usize, data1: usize, data2: usize, _: AppId) -> ReturnCode {
        match command_num {
            0 /* check if present */ => ReturnCode::SUCCESS,
            // Check is sensor is correctly connected
            1 => {
				if self.state.get () == State::Idle {
					self.is_present ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}

			}
			// Set Accelerometer Power Mode
            2 => {
				if self.state.get () == State::Idle {
                    if let Some (data_rate) = Lsm303dlhcAccelDataRate::from_usize (data1) {
                        self.set_power_mode(data_rate, if data2 != 0 { true } else { false });
                        ReturnCode::SUCCESS
                    }
                    else
                    {
                        ReturnCode::EINVAL
                    }
				}
				else
				{
					ReturnCode::EBUSY
				}
			}
			// Set Accelerometer Scale And Resolution
            3 => {
				if self.state.get () == State::Idle {
					if let Some (scale) = Lsm303dlhcScale::from_usize(data1) {
                        self.set_scale_and_resolution(scale, if data2 != 0 { true } else { false });
                        ReturnCode::SUCCESS
                    }
                    else
                    {
                        ReturnCode::EINVAL
                    }
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
            // Set Magnetometer Temperature Enable and Data Rate 
            4 => {
				if self.state.get () == State::Idle {
                    if let Some (data_rate) = Lsm303dlhcMagnetoDataRate::from_usize (data2) {
                        self.set_temperature_and_magneto_data_rate (if data1 != 0 { true } else { false }, data_rate);
                        ReturnCode::SUCCESS
                    }
                    else
                    {
                        ReturnCode::EINVAL
                    }
				}
				else
				{
					ReturnCode::EBUSY
				}
			}
			// Set Magnetometer Range
            5 => {
				if self.state.get () == State::Idle {
					if let Some (range) = Lsm303dlhcRange::from_usize(data1) {
                        self.set_range(range);
                        ReturnCode::SUCCESS
                    }
                    else
                    {
                        ReturnCode::EINVAL
                    }
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
			// Read Acceleration XYZ
            6 => {
				if self.state.get () == State::Idle {
					self.read_acceleration_xyz ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
            // Read Temperature
            7 => {
				if self.state.get () == State::Idle {
					self.read_temperature ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
			}
			8 => {
				if self.state.get () == State::Idle {
					self.read_magnetometer_xyz ();
					ReturnCode::SUCCESS
				}
				else
				{
					ReturnCode::EBUSY
				}
            }
            // default
            _ => ReturnCode::ENOSUPPORT,
        }
    }

    fn subscribe(
        &self,
        subscribe_num: usize,
        callback: Option<Callback>,
        _app_id: AppId,
    ) -> ReturnCode {
        match subscribe_num {
            0 /* set the one shot callback */ => {
				self.callback.insert (callback);
				ReturnCode::SUCCESS
			},
            // default
            _ => ReturnCode::ENOSUPPORT,
        }
    }
}

[calebcartwright]

Thanks for the report @bradjc. Looks like the problem is with the block comment in the match first match arm that's resulting in rustfmt failing to do any formatting within the match blocks (not just failing to convert the tabs).

This is definitely a bug, and as a temporary workaround you may want to consider changing those two block comments on the 0 arms to inline comments above the arms like was done for the others

[Wilfred]

I've been bitten by this too. This affects both the match bodies and the match patterns. The following code is unmodified by rustfmt, and no warnings are produced:

fn parse_term() {
    match x { |
       TokenKind::Name => {
                let qualified_name =1;
            }
            | TokenKind::Suspend
            // foo
            | _ => {}
        }
}

[whizsid]

I am working on this issue

[smmalis37]

I've just run into this, here's a very simple example:

fn main() {
    let i = 0;
    match i {
        0|1
        // Comment!
        |2|3 => {println!("<4");}
        _=>unreachable!(),
    }
}

Removing the comment allows rustfmt to work as expected, but so long as it's there the entire match block seems to be ignored for formatting, even the entirely unrelated _ arm. Is there any update on this case?

[daprilik]

On a semi-related note: are there any logging/tracing options that can be enabled to emit a warning in this and similar cases (where where rustfmt silently "gives up")?

[ytmimi]

@daprilik you can set error_on_unformatted = true.

Trying to format the code snippet from #4119 (comment) produces the following warning:

error[internal]: not formatted because a comment would be lost
 --> <stdin>:3
  |
3 |     match i {
  |
  = note: set `error_on_unformatted = false` to suppress the warning against comments or string literals

warning: rustfmt has failed to format. See previous 1 errors.

[not-holar]

Some simple examples of this

The following silently bails on the entire match statement

match (a) {
    0 // hello world
    | 1 => (),
    _ => (),
};

and so does this

match (a) {
    0 // hello world
    => (),
    _ => (),
};