Skip to content

Commit

Permalink
Don't bail out of formatting code blocks if lines are >100 chars
Browse files Browse the repository at this point in the history
Today rustfmt bails when trying to handle a code block that, after
formatting, has lines >100 chars. The reasoning was that if there were
lines longer than 100 chars it wasn't clear if we failed to format or
that was the intended result, so to be cautious the handler would bail.

However, if formatting the snippet fails the code block formatter will
already have jumped out by the time this line-width check is done:

https://github.com/rust-lang/rustfmt/blob/ff57cc6293ce450380dbb6da6a1a3656dbae538d/src/formatting/util.rs#L81

So I am not sure that this check is needed, and removing it broke no
tests except one designed explicitly for it. NB w.r.t. that unit test,
formatting a code like

```
fn main() {
    let expected = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);";
}
```

would yield no change, so I believe it is also the expected result.

Closes #4325
  • Loading branch information
ayazhafiz committed Jul 21, 2020
1 parent 27b54e9 commit 3ba8719
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 13 deletions.
19 changes: 6 additions & 13 deletions src/formatting/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,6 @@ pub(crate) fn format_code_block(code_snippet: &str, config: &Config) -> Option<F
}
let trimmed_line = if !is_indented {
line
} else if line.len() > config.max_width() {
// If there are lines that are larger than max width, we cannot tell
// whether we have succeeded but have some comments or strings that
// are too long, or we have failed to format code block. We will be
// conservative and just return `None` in this case.
return None;
} else if line.len() > indent_str.len() {
// Make sure that the line has leading whitespaces.
if line.starts_with(indent_str.as_ref()) {
Expand Down Expand Up @@ -885,13 +879,6 @@ mod test {
assert!(test_format_inner(format_snippet, snippet, expected));
}

#[test]
fn test_format_code_block_fail() {
#[rustfmt::skip]
let code_block = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);";
assert!(format_code_block(code_block, &Config::default()).is_none());
}

#[test]
fn test_format_code_block() {
// simple code block
Expand Down Expand Up @@ -942,5 +929,11 @@ false,
)
};";
assert!(test_format_inner(format_code_block, code_block, expected));

#[rustfmt::skip]
let code_block = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);";
#[rustfmt::skip]
let expected = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);";
assert!(test_format_inner(format_code_block, code_block, expected));
}
}
10 changes: 10 additions & 0 deletions tests/target/issue-4325.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
macro_rules! bad {
() => {
macro_rules! inner {
() => {
// This needs to have a width of over 100 characters to trigger the issue 12345678901
("a", "B")
};
}
};
}

0 comments on commit 3ba8719

Please sign in to comment.