Always expand tabs to four spaces in diagnostics (#19618)
## Summary I was a bit stuck on some snapshot differences I was seeing in #19415, but @BurntSushi pointed out that `annotate-snippets` already normalizes tabs on its own, which was very helpful! Instead of applying this change directly to the other branch, I wanted to try applying it in `ruff_linter` first. This should very slightly reduce the number of changes in #19415 proper. It looks like `annotate-snippets` always expands a tab to four spaces, whereas I think we were aligning to tab stops: ```diff 6 | spam(ham[1], { eggs: 2}) 7 | #: E201:1:6 - 8 | spam( ham[1], {eggs: 2}) - | ^^^ E201 + 8 | spam( ham[1], {eggs: 2}) + | ^^^^ E201 ``` ```diff 61 | #: E203:2:15 E702:2:16 62 | if x == 4: -63 | print(x, y) ; x, y = y, x - | ^ E203 +63 | print(x, y) ; x, y = y, x + | ^^^^ E203 ``` ```diff E27.py:15:6: E271 [*] Multiple spaces after keyword | -13 | True and False +13 | True and False 14 | #: E271 15 | a and b | ^^ E271 ``` I don't think this is too bad and has the major benefit of allowing us to pass the non-tab-expanded range to `annotate-snippets` in #19415, where it's also displayed in the header. Ruff doesn't have this problem currently because it uses its own concise diagnostic output as the header for full diagnostics, where the pre-expansion range is used directly. ## Test Plan Existing tests with a few snapshot updates
This commit is contained in:
Brent Westbrook
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -2891,7 +2891,6 @@ dependencies = [
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"ty_static",
|
||||
"unicode-width 0.2.1",
|
||||
"web-time",
|
||||
"zip",
|
||||
]
|
||||
|
||||
@@ -42,7 +42,6 @@ serde_json = { workspace = true, optional = true }
|
||||
thiserror = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
tracing-subscriber = { workspace = true, optional = true }
|
||||
unicode-width = { workspace = true }
|
||||
zip = { workspace = true }
|
||||
|
||||
[target.'cfg(target_arch="wasm32")'.dependencies]
|
||||
|
||||
@@ -585,8 +585,7 @@ impl<'r> RenderableSnippet<'r> {
|
||||
let EscapedSourceCode {
|
||||
text: snippet,
|
||||
annotations,
|
||||
} = replace_whitespace_and_unprintable(snippet, annotations)
|
||||
.fix_up_empty_spans_after_line_terminator();
|
||||
} = replace_unprintable(snippet, annotations).fix_up_empty_spans_after_line_terminator();
|
||||
|
||||
RenderableSnippet {
|
||||
snippet,
|
||||
@@ -828,13 +827,18 @@ fn relativize_path<'p>(cwd: &SystemPath, path: &'p str) -> &'p str {
|
||||
path
|
||||
}
|
||||
|
||||
/// Given some source code and annotation ranges, this routine replaces tabs
|
||||
/// with ASCII whitespace, and unprintable characters with printable
|
||||
/// representations of them.
|
||||
/// Given some source code and annotation ranges, this routine replaces
|
||||
/// unprintable characters with printable representations of them.
|
||||
///
|
||||
/// The source code and annotations returned are updated to reflect changes made
|
||||
/// to the source code (if any).
|
||||
fn replace_whitespace_and_unprintable<'r>(
|
||||
///
|
||||
/// We don't need to normalize whitespace, such as converting tabs to spaces,
|
||||
/// because `annotate-snippets` handles that internally. Similarly, it's safe to
|
||||
/// modify the annotation ranges by inserting 3-byte Unicode replacements
|
||||
/// because `annotate-snippets` will account for their actual width when
|
||||
/// rendering and displaying the column to the user.
|
||||
fn replace_unprintable<'r>(
|
||||
source: &'r str,
|
||||
mut annotations: Vec<RenderableAnnotation<'r>>,
|
||||
) -> EscapedSourceCode<'r> {
|
||||
@@ -866,48 +870,17 @@ fn replace_whitespace_and_unprintable<'r>(
|
||||
}
|
||||
};
|
||||
|
||||
const TAB_SIZE: usize = 4;
|
||||
let mut width = 0;
|
||||
let mut column = 0;
|
||||
let mut last_end = 0;
|
||||
let mut result = String::new();
|
||||
for (index, c) in source.char_indices() {
|
||||
let old_width = width;
|
||||
match c {
|
||||
'\n' | '\r' => {
|
||||
width = 0;
|
||||
column = 0;
|
||||
}
|
||||
'\t' => {
|
||||
let tab_offset = TAB_SIZE - (column % TAB_SIZE);
|
||||
width += tab_offset;
|
||||
column += tab_offset;
|
||||
if let Some(printable) = unprintable_replacement(c) {
|
||||
result.push_str(&source[last_end..index]);
|
||||
|
||||
let tab_width =
|
||||
u32::try_from(width - old_width).expect("small width because of tab size");
|
||||
result.push_str(&source[last_end..index]);
|
||||
let len = printable.text_len().to_u32();
|
||||
update_ranges(result.text_len().to_usize(), len);
|
||||
|
||||
update_ranges(result.text_len().to_usize(), tab_width);
|
||||
|
||||
for _ in 0..tab_width {
|
||||
result.push(' ');
|
||||
}
|
||||
last_end = index + 1;
|
||||
}
|
||||
_ => {
|
||||
width += unicode_width::UnicodeWidthChar::width(c).unwrap_or(0);
|
||||
column += 1;
|
||||
|
||||
if let Some(printable) = unprintable_replacement(c) {
|
||||
result.push_str(&source[last_end..index]);
|
||||
|
||||
let len = printable.text_len().to_u32();
|
||||
update_ranges(result.text_len().to_usize(), len);
|
||||
|
||||
result.push(printable);
|
||||
last_end = index + 1;
|
||||
}
|
||||
}
|
||||
result.push(printable);
|
||||
last_end = index + 1;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -177,4 +177,25 @@ print()
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Ensure that the header column matches the column in the user's input, even if we've replaced
|
||||
/// tabs with spaces for rendering purposes.
|
||||
#[test]
|
||||
fn tab_replacement() {
|
||||
let mut env = TestEnvironment::new();
|
||||
env.add("example.py", "def foo():\n\treturn 1");
|
||||
env.format(DiagnosticFormat::Full);
|
||||
|
||||
let diagnostic = env.err().primary("example.py", "2:1", "2:9", "").build();
|
||||
|
||||
insta::assert_snapshot!(env.render(&diagnostic), @r"
|
||||
error[test-diagnostic]: main diagnostic message
|
||||
--> example.py:2:2
|
||||
|
|
||||
1 | def foo():
|
||||
2 | return 1
|
||||
| ^^^^^^^^
|
||||
|
|
||||
");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,7 +13,6 @@ use ruff_notebook::NotebookIndex;
|
||||
use ruff_source_file::OneIndexed;
|
||||
use ruff_text_size::{TextLen, TextRange, TextSize};
|
||||
|
||||
use crate::line_width::{IndentWidth, LineWidthBuilder};
|
||||
use crate::message::diff::Diff;
|
||||
use crate::message::{Emitter, EmitterContext};
|
||||
use crate::settings::types::UnsafeFixes;
|
||||
@@ -229,7 +228,7 @@ impl Display for MessageCodeFrame<'_> {
|
||||
let start_offset = source_code.line_start(start_index);
|
||||
let end_offset = source_code.line_end(end_index);
|
||||
|
||||
let source = replace_whitespace_and_unprintable(
|
||||
let source = replace_unprintable(
|
||||
source_code.slice(TextRange::new(start_offset, end_offset)),
|
||||
self.message.expect_range() - start_offset,
|
||||
)
|
||||
@@ -272,16 +271,20 @@ impl Display for MessageCodeFrame<'_> {
|
||||
}
|
||||
|
||||
/// Given some source code and an annotation range, this routine replaces
|
||||
/// tabs with ASCII whitespace, and unprintable characters with printable
|
||||
/// representations of them.
|
||||
/// unprintable characters with printable representations of them.
|
||||
///
|
||||
/// The source code returned has an annotation that is updated to reflect
|
||||
/// changes made to the source code (if any).
|
||||
fn replace_whitespace_and_unprintable(source: &str, annotation_range: TextRange) -> SourceCode {
|
||||
///
|
||||
/// We don't need to normalize whitespace, such as converting tabs to spaces,
|
||||
/// because `annotate-snippets` handles that internally. Similarly, it's safe to
|
||||
/// modify the annotation ranges by inserting 3-byte Unicode replacements
|
||||
/// because `annotate-snippets` will account for their actual width when
|
||||
/// rendering and displaying the column to the user.
|
||||
fn replace_unprintable(source: &str, annotation_range: TextRange) -> SourceCode {
|
||||
let mut result = String::new();
|
||||
let mut last_end = 0;
|
||||
let mut range = annotation_range;
|
||||
let mut line_width = LineWidthBuilder::new(IndentWidth::default());
|
||||
|
||||
// Updates the range given by the caller whenever a single byte (at
|
||||
// `index` in `source`) is replaced with `len` bytes.
|
||||
@@ -310,19 +313,7 @@ fn replace_whitespace_and_unprintable(source: &str, annotation_range: TextRange)
|
||||
};
|
||||
|
||||
for (index, c) in source.char_indices() {
|
||||
let old_width = line_width.get();
|
||||
line_width = line_width.add_char(c);
|
||||
|
||||
if matches!(c, '\t') {
|
||||
let tab_width = u32::try_from(line_width.get() - old_width)
|
||||
.expect("small width because of tab size");
|
||||
result.push_str(&source[last_end..index]);
|
||||
for _ in 0..tab_width {
|
||||
result.push(' ');
|
||||
}
|
||||
last_end = index + 1;
|
||||
update_range(index, tab_width);
|
||||
} else if let Some(printable) = unprintable_replacement(c) {
|
||||
if let Some(printable) = unprintable_replacement(c) {
|
||||
result.push_str(&source[last_end..index]);
|
||||
result.push(printable);
|
||||
last_end = index + 1;
|
||||
|
||||
@@ -15,8 +15,8 @@ E101.py:15:1: E101 Indentation contains mixed spaces and tabs
|
||||
|
|
||||
13 | def func_mixed_start_with_space():
|
||||
14 | # E101
|
||||
15 | print("mixed starts with space")
|
||||
| ^^^^^^^^^^^^^^^ E101
|
||||
15 | print("mixed starts with space")
|
||||
| ^^^^^^^^^^^^^^^^^^^^ E101
|
||||
16 |
|
||||
17 | def xyz():
|
||||
|
|
||||
@@ -25,6 +25,6 @@ E101.py:19:1: E101 Indentation contains mixed spaces and tabs
|
||||
|
|
||||
17 | def xyz():
|
||||
18 | # E101
|
||||
19 | print("xyz");
|
||||
| ^^^^ E101
|
||||
19 | print("xyz");
|
||||
| ^^^^^^^ E101
|
||||
|
|
||||
|
||||
@@ -47,7 +47,7 @@ E20.py:6:15: E201 [*] Whitespace after '{'
|
||||
6 | spam(ham[1], { eggs: 2})
|
||||
| ^ E201
|
||||
7 | #: E201:1:6
|
||||
8 | spam( ham[1], {eggs: 2})
|
||||
8 | spam( ham[1], {eggs: 2})
|
||||
|
|
||||
= help: Remove whitespace before '{'
|
||||
|
||||
@@ -65,10 +65,10 @@ E20.py:8:6: E201 [*] Whitespace after '('
|
||||
|
|
||||
6 | spam(ham[1], { eggs: 2})
|
||||
7 | #: E201:1:6
|
||||
8 | spam( ham[1], {eggs: 2})
|
||||
| ^^^ E201
|
||||
8 | spam( ham[1], {eggs: 2})
|
||||
| ^^^^ E201
|
||||
9 | #: E201:1:10
|
||||
10 | spam(ham[ 1], {eggs: 2})
|
||||
10 | spam(ham[ 1], {eggs: 2})
|
||||
|
|
||||
= help: Remove whitespace before '('
|
||||
|
||||
@@ -84,12 +84,12 @@ E20.py:8:6: E201 [*] Whitespace after '('
|
||||
|
||||
E20.py:10:10: E201 [*] Whitespace after '['
|
||||
|
|
||||
8 | spam( ham[1], {eggs: 2})
|
||||
8 | spam( ham[1], {eggs: 2})
|
||||
9 | #: E201:1:10
|
||||
10 | spam(ham[ 1], {eggs: 2})
|
||||
| ^^^ E201
|
||||
10 | spam(ham[ 1], {eggs: 2})
|
||||
| ^^^^ E201
|
||||
11 | #: E201:1:15
|
||||
12 | spam(ham[1], { eggs: 2})
|
||||
12 | spam(ham[1], { eggs: 2})
|
||||
|
|
||||
= help: Remove whitespace before '['
|
||||
|
||||
@@ -105,10 +105,10 @@ E20.py:10:10: E201 [*] Whitespace after '['
|
||||
|
||||
E20.py:12:15: E201 [*] Whitespace after '{'
|
||||
|
|
||||
10 | spam(ham[ 1], {eggs: 2})
|
||||
10 | spam(ham[ 1], {eggs: 2})
|
||||
11 | #: E201:1:15
|
||||
12 | spam(ham[1], { eggs: 2})
|
||||
| ^^ E201
|
||||
12 | spam(ham[1], { eggs: 2})
|
||||
| ^^^^ E201
|
||||
13 | #: Okay
|
||||
14 | spam(ham[1], {eggs: 2})
|
||||
|
|
||||
|
||||
@@ -49,7 +49,7 @@ E20.py:23:11: E202 [*] Whitespace before ']'
|
||||
23 | spam(ham[1 ], {eggs: 2})
|
||||
| ^ E202
|
||||
24 | #: E202:1:23
|
||||
25 | spam(ham[1], {eggs: 2} )
|
||||
25 | spam(ham[1], {eggs: 2} )
|
||||
|
|
||||
= help: Remove whitespace before ']'
|
||||
|
||||
@@ -67,10 +67,10 @@ E20.py:25:23: E202 [*] Whitespace before ')'
|
||||
|
|
||||
23 | spam(ham[1 ], {eggs: 2})
|
||||
24 | #: E202:1:23
|
||||
25 | spam(ham[1], {eggs: 2} )
|
||||
| ^^ E202
|
||||
25 | spam(ham[1], {eggs: 2} )
|
||||
| ^^^^ E202
|
||||
26 | #: E202:1:22
|
||||
27 | spam(ham[1], {eggs: 2 })
|
||||
27 | spam(ham[1], {eggs: 2 })
|
||||
|
|
||||
= help: Remove whitespace before ')'
|
||||
|
||||
@@ -86,12 +86,12 @@ E20.py:25:23: E202 [*] Whitespace before ')'
|
||||
|
||||
E20.py:27:22: E202 [*] Whitespace before '}'
|
||||
|
|
||||
25 | spam(ham[1], {eggs: 2} )
|
||||
25 | spam(ham[1], {eggs: 2} )
|
||||
26 | #: E202:1:22
|
||||
27 | spam(ham[1], {eggs: 2 })
|
||||
| ^^^ E202
|
||||
27 | spam(ham[1], {eggs: 2 })
|
||||
| ^^^^ E202
|
||||
28 | #: E202:1:11
|
||||
29 | spam(ham[1 ], {eggs: 2})
|
||||
29 | spam(ham[1 ], {eggs: 2})
|
||||
|
|
||||
= help: Remove whitespace before '}'
|
||||
|
||||
@@ -107,10 +107,10 @@ E20.py:27:22: E202 [*] Whitespace before '}'
|
||||
|
||||
E20.py:29:11: E202 [*] Whitespace before ']'
|
||||
|
|
||||
27 | spam(ham[1], {eggs: 2 })
|
||||
27 | spam(ham[1], {eggs: 2 })
|
||||
28 | #: E202:1:11
|
||||
29 | spam(ham[1 ], {eggs: 2})
|
||||
| ^^ E202
|
||||
29 | spam(ham[1 ], {eggs: 2})
|
||||
| ^^^^ E202
|
||||
30 | #: Okay
|
||||
31 | spam(ham[1], {eggs: 2})
|
||||
|
|
||||
|
||||
@@ -25,8 +25,8 @@ E20.py:55:10: E203 [*] Whitespace before ':'
|
||||
|
|
||||
53 | x, y = y, x
|
||||
54 | #: E203:1:10
|
||||
55 | if x == 4 :
|
||||
| ^^^ E203
|
||||
55 | if x == 4 :
|
||||
| ^^^^ E203
|
||||
56 | print(x, y)
|
||||
57 | x, y = y, x
|
||||
|
|
||||
@@ -67,8 +67,8 @@ E20.py:63:16: E203 [*] Whitespace before ';'
|
||||
|
|
||||
61 | #: E203:2:15 E702:2:16
|
||||
62 | if x == 4:
|
||||
63 | print(x, y) ; x, y = y, x
|
||||
| ^ E203
|
||||
63 | print(x, y) ; x, y = y, x
|
||||
| ^^^^ E203
|
||||
64 | #: E203:3:13
|
||||
65 | if x == 4:
|
||||
|
|
||||
|
||||
@@ -1,13 +1,12 @@
|
||||
---
|
||||
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
E22.py:43:2: E223 [*] Tab before operator
|
||||
|
|
||||
41 | #: E223
|
||||
42 | foobart = 4
|
||||
43 | a = 3 # aligned with tab
|
||||
| ^^^ E223
|
||||
43 | a = 3 # aligned with tab
|
||||
| ^^^^ E223
|
||||
44 | #:
|
||||
|
|
||||
= help: Replace with single space
|
||||
|
||||
@@ -1,13 +1,12 @@
|
||||
---
|
||||
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
E24.py:6:8: E242 [*] Tab after comma
|
||||
|
|
||||
4 | b = (1, 20)
|
||||
5 | #: E242
|
||||
6 | a = (1, 2) # tab before 2
|
||||
| ^ E242
|
||||
6 | a = (1, 2) # tab before 2
|
||||
| ^^^^ E242
|
||||
7 | #: Okay
|
||||
8 | b = (1, 20) # space before 20
|
||||
|
|
||||
|
||||
@@ -66,7 +66,7 @@ E27.py:8:3: E271 [*] Multiple spaces after keyword
|
||||
|
||||
E27.py:15:6: E271 [*] Multiple spaces after keyword
|
||||
|
|
||||
13 | True and False
|
||||
13 | True and False
|
||||
14 | #: E271
|
||||
15 | a and b
|
||||
| ^^ E271
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
---
|
||||
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
E27.py:21:2: E272 [*] Multiple spaces before keyword
|
||||
|
|
||||
@@ -51,7 +50,7 @@ E27.py:25:5: E272 [*] Multiple spaces before keyword
|
||||
25 | this and False
|
||||
| ^^ E272
|
||||
26 | #: E273
|
||||
27 | a and b
|
||||
27 | a and b
|
||||
|
|
||||
= help: Replace with single space
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ E27.py:11:9: E273 [*] Tab after keyword
|
||||
11 | True and False
|
||||
| ^^^^^^^^ E273
|
||||
12 | #: E273 E274
|
||||
13 | True and False
|
||||
13 | True and False
|
||||
|
|
||||
= help: Replace with single space
|
||||
|
||||
@@ -26,7 +26,7 @@ E27.py:13:5: E273 [*] Tab after keyword
|
||||
|
|
||||
11 | True and False
|
||||
12 | #: E273 E274
|
||||
13 | True and False
|
||||
13 | True and False
|
||||
| ^^^^^^^^ E273
|
||||
14 | #: E271
|
||||
15 | a and b
|
||||
@@ -47,8 +47,8 @@ E27.py:13:10: E273 [*] Tab after keyword
|
||||
|
|
||||
11 | True and False
|
||||
12 | #: E273 E274
|
||||
13 | True and False
|
||||
| ^ E273
|
||||
13 | True and False
|
||||
| ^^^^ E273
|
||||
14 | #: E271
|
||||
15 | a and b
|
||||
|
|
||||
@@ -68,10 +68,10 @@ E27.py:27:6: E273 [*] Tab after keyword
|
||||
|
|
||||
25 | this and False
|
||||
26 | #: E273
|
||||
27 | a and b
|
||||
| ^^^ E273
|
||||
27 | a and b
|
||||
| ^^^^ E273
|
||||
28 | #: E274
|
||||
29 | a and b
|
||||
29 | a and b
|
||||
|
|
||||
= help: Replace with single space
|
||||
|
||||
@@ -87,10 +87,10 @@ E27.py:27:6: E273 [*] Tab after keyword
|
||||
|
||||
E27.py:31:10: E273 [*] Tab after keyword
|
||||
|
|
||||
29 | a and b
|
||||
29 | a and b
|
||||
30 | #: E273 E274
|
||||
31 | this and False
|
||||
| ^ E273
|
||||
31 | this and False
|
||||
| ^^^^ E273
|
||||
32 | #: Okay
|
||||
33 | from u import (a, b)
|
||||
|
|
||||
|
||||
@@ -1,15 +1,14 @@
|
||||
---
|
||||
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
E27.py:29:2: E274 [*] Tab before keyword
|
||||
|
|
||||
27 | a and b
|
||||
27 | a and b
|
||||
28 | #: E274
|
||||
29 | a and b
|
||||
| ^^^^^^^ E274
|
||||
29 | a and b
|
||||
| ^^^^^^^^ E274
|
||||
30 | #: E273 E274
|
||||
31 | this and False
|
||||
31 | this and False
|
||||
|
|
||||
= help: Replace with single space
|
||||
|
||||
@@ -25,9 +24,9 @@ E27.py:29:2: E274 [*] Tab before keyword
|
||||
|
||||
E27.py:31:5: E274 [*] Tab before keyword
|
||||
|
|
||||
29 | a and b
|
||||
29 | a and b
|
||||
30 | #: E273 E274
|
||||
31 | this and False
|
||||
31 | this and False
|
||||
| ^^^^^^^^ E274
|
||||
32 | #: Okay
|
||||
33 | from u import (a, b)
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
---
|
||||
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
W19.py:1:1: W191 Indentation contains tabs
|
||||
|
|
||||
@@ -371,7 +370,7 @@ W19.py:157:1: W191 Indentation contains tabs
|
||||
156 | f"test{
|
||||
157 | tab_indented_should_be_flagged
|
||||
| ^^^^ W191
|
||||
158 | } <- this tab is fine"
|
||||
158 | } <- this tab is fine"
|
||||
|
|
||||
|
||||
W19.py:161:1: W191 Indentation contains tabs
|
||||
@@ -379,5 +378,5 @@ W19.py:161:1: W191 Indentation contains tabs
|
||||
160 | f"""test{
|
||||
161 | tab_indented_should_be_flagged
|
||||
| ^^^^ W191
|
||||
162 | } <- this tab is fine"""
|
||||
162 | } <- this tab is fine"""
|
||||
|
|
||||
|
||||
@@ -22,7 +22,7 @@ RUF054.py:10:3: RUF054 Indented form feed
|
||||
RUF054.py:13:2: RUF054 Indented form feed
|
||||
|
|
||||
12 | def _():
|
||||
13 | pass
|
||||
13 | pass
|
||||
| ^ RUF054
|
||||
14 |
|
||||
15 | if False:
|
||||
|
||||
Reference in New Issue
Block a user