Fix tabs having a fixed width internally #5039

karyon · 2021-10-22T09:25:33Z

Ref #4968. Tabs were always counted to have a width of config.tab_spaces.
Since they actually can have less than that depending on the line"s
previous contents, rustfmt would sometimes complain about the line
length even if the line did actually fit.

Ref rust-lang#4968. Tabs were always counted to have a width of config.tab_spaces. Since they actually can have less than that depending on the line"s previous contents, rustfmt would sometimes complain about the line length even if the line did actually fit.

MitMaro · 2021-10-23T14:47:20Z

Looks like this still fails if there is more than one tab character in a line.

calebcartwright · 2021-10-24T20:38:33Z

@MitMaro - thanks for sharing! sounds straightforward enough, but do you have a snippet you"d be willing to share? that makes adding a corresponding test case a bit easier

MitMaro · 2021-10-25T12:09:22Z

Here"s a test case (based on the one above) that still causes a panic:

// rustfmt-error_on_unformatted: true
// rustfmt-error_on_line_overflow: true

// Part of #4968. This line has a width of 100, so it should be fine, but rustfmt panicked.
fn panic_with_tabs() {
    let a = "tab here:		Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonu est \
             est, consetetur sadipscing";
}

Here"s the stack:

thread "<unnamed>" panicked at "SourceAnnotation range `(100, 104)` is bigger than source length `100`", /home/mitmaro/.cargo/registry/src/github.com-1ecc6299db9ec823/annotate-snippets-0.8.0/src/display_list/from_snippet.rs:273:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/panicking.rs:517:5
   1: std::panicking::begin_panic_fmt
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/panicking.rs:460:5
   2: annotate_snippets::display_list::from_snippet::format_body
             at /home/mitmaro/.cargo/registry/src/github.com-1ecc6299db9ec823/annotate-snippets-0.8.0/src/display_list/from_snippet.rs:273:9
   3: annotate_snippets::display_list::from_snippet::format_slice
             at /home/mitmaro/.cargo/registry/src/github.com-1ecc6299db9ec823/annotate-snippets-0.8.0/src/display_list/from_snippet.rs:114:20
   4: annotate_snippets::display_list::from_snippet::<impl core::convert::From<annotate_snippets::snippet::Snippet> for annotate_snippets::display_list::structs::DisplayList>::from
             at /home/mitmaro/.cargo/registry/src/github.com-1ecc6299db9ec823/annotate-snippets-0.8.0/src/display_list/from_snippet.rs:487:30
   5: <rustfmt_nightly::format_report_formatter::FormatReportFormatter as core::fmt::Display>::fmt
             at ./src/format_report_formatter.rs:95:37
   6: core::fmt::write
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/core/src/fmt/mod.rs:1163:17
   7: std::io::Write::write_fmt
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/io/mod.rs:1696:15
   8: std::io::stdio::print_to::{{closure}}::{{closure}}
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/io/stdio.rs:1187:25
   9: core::option::Option<T>::map
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/core/src/option.rs:848:29
  10: std::io::stdio::print_to::{{closure}}
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/io/stdio.rs:1186:13
  11: std::thread::local::LocalKey<T>::try_with
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/thread/local.rs:399:16
  12: std::io::stdio::print_to
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/io/stdio.rs:1182:12
  13: std::io::stdio::_print
             at /rustc/1af55d19c7a9189374d89472f97dc119659bb67e/library/std/src/io/stdio.rs:1209:5
  14: rustfmt_nightly::test::check_files
             at ./src/test/mod.rs:540:17
  15: rustfmt_nightly::test::idempotence_tests::{{closure}}
             at ./src/test/mod.rs:320:40

MitMaro · 2021-10-25T12:12:56Z

Another test case, and one I reduced from one of my projects that still causes a crash in rustfmt is leading tabs with hard_tabs: true.

// rustfmt-error_on_unformatted: true
// rustfmt-error_on_line_overflow: true
// rustfmt-hard_tabs: true

fn panic_with_tabs() {
	let a = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
}

thread "<unnamed>" panicked at "SourceAnnotation range `(100, 105)` is bigger than source length `102`", /home/mitmaro/.cargo/registry/src/github.com-1ecc6299db9ec823/annotate-snippets-0.8.0/src/display_list/from_snippet.rs:273:9

karyon · 2021-10-26T14:10:40Z

Right. I didn"t fix the entire bug, only those (few) cases where the line would actually fit into the width limit. See my comment here: #4968 (comment)

camsteffen · 2022-02-02T21:46:22Z

src/formatting.rs

@@ -559,7 +559,7 @@ impl<'a> FormatLines<'a> {
    fn char(&mut self, c: char, kind: FullCodeCharKind) {
        self.newline_count = 0;
        self.line_len += if c == '\t' {
-            self.config.tab_spaces()
+            self.config.tab_spaces() - self.line_len % self.config.tab_spaces()
        } else {
            1


This 1 can also cause issues with non-ascii idents. Could be c.len_utf8().

Another possible fix for tabs is to use self.line_buffer.push_str(&" ".repeat(self.config.tab_spaces())). Not sure which is correct.

* Internally, rustfmt preserves tabs and counts them as multiple characters (based on configuration). * The annoted_snippet dependency counts tabs as 1 character. * If rustfmt produces an error on a line containing tabs, annotated_snippet may think that the error is out of range and panic. * Have rustfmt internally replace tabs with the corresponding number of spaces, so that columns can be counted unambiguously. * This change is based on PR rust-lang#5039 by karyon, but with the code review suggestions by camsteffen.

* Internally, rustfmt preserves tabs and counts them as multiple characters (based on configuration). * The annotated_snippet dependency always counts tabs as 1 character. * If rustfmt tries to display an error on a line containing tabs, the indicies are mismatched. * In the extreme case, annotated_snippet may try to access out-of-range indices, and panic. * This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted (unambiguously and) the same.

* This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted unambiguously.

drahnr · 2024-08-02T12:34:29Z

@karyon @camsteffen gentle ping

camsteffen · 2024-08-02T12:51:44Z

@drahnr neither of us can do anything to advance this PR AFAICT. The maintainers have to action on it.

* This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted unambiguously.

karyon mentioned this pull request Oct 22, 2021

panic in annotated_snippet dep #4968

Open

calebcartwright added the pr-not-reviewed label Oct 23, 2021

camsteffen reviewed Feb 2, 2022

View reviewed changes

ytmimi added p-medium p-low and removed p-medium labels Jul 27, 2022

Raekye mentioned this pull request Dec 7, 2022

Fix panic in annotated_snippet dependency (GitHub issue #4968). #5629

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tabs having a fixed width internally #5039

Fix tabs having a fixed width internally #5039

karyon commented Oct 22, 2021

MitMaro commented Oct 23, 2021

calebcartwright commented Oct 24, 2021

MitMaro commented Oct 25, 2021

MitMaro commented Oct 25, 2021

karyon commented Oct 26, 2021

camsteffen Feb 2, 2022

camsteffen Feb 2, 2022

drahnr commented Aug 2, 2024

camsteffen commented Aug 2, 2024

Fix tabs having a fixed width internally #5039

Are you sure you want to change the base?

Fix tabs having a fixed width internally #5039

Conversation

karyon commented Oct 22, 2021

MitMaro commented Oct 23, 2021

calebcartwright commented Oct 24, 2021

MitMaro commented Oct 25, 2021

MitMaro commented Oct 25, 2021

karyon commented Oct 26, 2021

camsteffen Feb 2, 2022

Choose a reason for hiding this comment

camsteffen Feb 2, 2022

Choose a reason for hiding this comment

drahnr commented Aug 2, 2024

camsteffen commented Aug 2, 2024