Comment indentation/alignment changed in Rust 1.81
printfn opened this issue · 9 comments
When I updated to Rust 1.81 I noticed that the comment indentation changed. Not sure if this is an intentional breakage or a bug.
I have the following code (formatted with Rust 1.80.1, rustfmt 1.7.0):
enum Enum12 {
Fn,
NotEquals,
Backslash,
}
fn parse_symbol2(ch: char) -> Enum12 {
match ch {
'=' => Enum12::Fn,
'\u{2260}' => Enum12::NotEquals, // unicode not equal to symbol
'\\' | '\u{3bb}' => Enum12::Backslash, // lambda symbol
_ => todo!(),
}
}
.rustfmt.toml:
hard_tabs = true
When I updated to Rust 1.81 (rustfmt 1.7.1) I got this diff:
diff --git a/src/lib.rs b/src/lib.rs
index 634b281..edc0fca 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -7,7 +7,7 @@ enum Enum12 {
fn parse_symbol2(ch: char) -> Enum12 {
match ch {
'=' => Enum12::Fn,
- '\u{2260}' => Enum12::NotEquals, // unicode not equal to symbol
+ '\u{2260}' => Enum12::NotEquals, // unicode not equal to symbol
'\\' | '\u{3bb}' => Enum12::Backslash, // lambda symbol
_ => todo!(),
}
Strangely the bug seems to be very dependent on the length of some of my identifiers. If I change the enum name to Enum or E the formatting difference goes away. Are comments like this meant to be aligned or not?
Thanks for the report. I took a look at the 1.7.1 CHANGELOG, but it's unclear to me which of those changes, if any, caused this. Also, I'm unable to reproduce this when building the v1.7.1 tag and running rustfmt on the input.
git switch v1.7.1 --detach
cargo run --bin rustfmt -- --config=version=One,hard_tabs=true --check
That said, I am able to reproduce this when I run with the 1.81 toolchain:
rustfmt +1.81 --config=hard_tabs=true --check
There's a chance that this change in behavior is related to changes in rustc (it has happened before). I think the best way to figure out what's going on here is to bisect between the 1.80.1 and 1.81 release to see what commit caused this change in behavior.
cargo-bisect-rustc identified this commit: rust-lang/rust@a70b2ae.
searched nightlies: from nightly-2024-06-07 to nightly-2024-07-20
regressed nightly: nightly-2024-06-10
searched commit range: rust-lang/rust@f21554f...a70b2ae
regressed commit: rust-lang/rust@a70b2ae
bisected with cargo-bisect-rustc v0.6.9
Host triple: x86_64-unknown-linux-gnu
Reproduce with:
cargo bisect-rustc --start=1.80.1 --end=1.81.0 -c rustfmt -- fmt --check
I've tracked down the regression to the unicode_width crate, which was updated from 0.1.12 to 0.1.13 in this PR: rust-lang/rust#126172. The PR unicode-rs/unicode-width#45 updated widths of control characters from 0 to 1, including the width of tab characters.
This patch reverses the behaviour and fixes the regression:
diff --git a/src/utils.rs b/src/utils.rs
index d1cfc6ac..31957e97 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -690,7 +690,7 @@ impl NodeIdExt for NodeId {
}
pub(crate) fn unicode_str_width(s: &str) -> usize {
- s.width()
+ s.replace('\t', "").width()
}
#[cfg(test)]
@@ -713,4 +713,9 @@ mod test {
Some("aaa\n bbb\n ccc".to_string())
);
}
+
+ #[test]
+ fn tab_width() {
+ assert_eq!(unicode_str_width("\t"), 0);
+ }
}
Thanks for digging into this! That helped me remember that #6203 mentioned there would be an impact to non-ascii unicode chars, but I don't think we expected this to impact \t characters.
@calebcartwright do you have any thoughts on how we should handle this breakage?
If we don't run this with hard_tabs=true then both rustfmt +1.81 and rustfmt +1.80 format this as follows:
enum Enum12 {
Fn,
NotEquals,
Backslash,
}
fn parse_symbol2(ch: char) -> Enum12 {
match ch {
'=' => Enum12::Fn,
'\u{2260}' => Enum12::NotEquals, // unicode not equal to symbol
'\\' | '\u{3bb}' => Enum12::Backslash, // lambda symbol
_ => todo!(),
}
}
Maybe that suggests that \t being assigned a width of 0 before was actually a mistake?
I don't think either 0 or 1 width is correct, rustfmt needs to properly calculate the tab width to make it consistent.
Here's an example that's broken with Rust 1.81's version of rustfmt (i.e. unicode-width v0.1.13):
cargo fmt -- --config hard_tabs=false
#[derive(Copy, Clone)]
pub enum EitherOrBoth {
Left(VersionChunk),
Right(VersionChunk),
Both(VersionChunk, VersionChunk),
}
#[derive(Copy, Clone)]
pub enum VersionChunk {
Str(&'static str),
Number { source: &'static str },
}
pub fn version_sort(either_or_both: EitherOrBoth) -> std::cmp::Ordering {
loop {
match either_or_both {
EitherOrBoth::Left(_) => return std::cmp::Ordering::Greater,
EitherOrBoth::Right(_) => return std::cmp::Ordering::Less,
EitherOrBoth::Both(a, b) => match (a, b) {
(VersionChunk::Str(ca), VersionChunk::Str(cb))
| (VersionChunk::Str(ca), VersionChunk::Number { source: cb, .. })
| (VersionChunk::Number { source: ca, .. }, VersionChunk::Str(cb)) => {
match ca.cmp(&cb) {
std::cmp::Ordering::Equal => {
continue;
}
order @ _ => return order,
}
}
_ => todo!(),
},
}
}
}
cargo fmt -- --config hard_tabs=true
#[derive(Copy, Clone)]
pub enum EitherOrBoth {
Left(VersionChunk),
Right(VersionChunk),
Both(VersionChunk, VersionChunk),
}
#[derive(Copy, Clone)]
pub enum VersionChunk {
Str(&'static str),
Number { source: &'static str },
}
pub fn version_sort(either_or_both: EitherOrBoth) -> std::cmp::Ordering {
loop {
match either_or_both {
EitherOrBoth::Left(_) => return std::cmp::Ordering::Greater,
EitherOrBoth::Right(_) => return std::cmp::Ordering::Less,
EitherOrBoth::Both(a, b) => match (a, b) {
(VersionChunk::Str(ca), VersionChunk::Str(cb))
| (VersionChunk::Str(ca), VersionChunk::Number { source: cb, .. })
| (VersionChunk::Number { source: ca, .. }, VersionChunk::Str(cb)) => match ca.cmp(&cb) {
std::cmp::Ordering::Equal => {
continue;
}
order @ _ => return order,
},
_ => todo!(),
},
}
}
}
You can see how the match ca.cmp(&cb) expression is formatted quite differently depending on whether we're using spaces or tabs.