Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle non-printable characters in diff view #11687

Merged
merged 2 commits into from
Jun 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/ruff_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub mod rule_selector;
pub mod rules;
pub mod settings;
pub mod source_kind;
mod text_helpers;
pub mod upstream_categories;

#[cfg(any(test, fuzzing))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/message/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use ruff_diagnostics::{Applicability, Fix};
use ruff_source_file::{OneIndexed, SourceFile};

use crate::message::Message;
use crate::text_helpers::ShowNonprinting;

/// Renders a diff that shows the code fixes.
///
Expand Down Expand Up @@ -101,6 +102,7 @@ impl Display for Diff<'_> {
)?;

for (emphasized, value) in change.iter_strings_lossy() {
let value = value.show_nonprinting();
if emphasized {
write!(f, "{}", line_style.apply_to(&value).underline().on_black())?;
} else {
Expand Down
5 changes: 4 additions & 1 deletion crates/ruff_linter/src/message/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::message::diff::Diff;
use crate::message::{Emitter, EmitterContext, Message};
use crate::registry::AsRule;
use crate::settings::types::UnsafeFixes;
use crate::text_helpers::ShowNonprinting;

bitflags! {
#[derive(Default)]
Expand Down Expand Up @@ -251,6 +252,8 @@ impl Display for MessageCodeFrame<'_> {
range - start_offset,
);

let source_text = source.text.show_nonprinting();

let start_char = source.text[TextRange::up_to(source.annotation_range.start())]
.chars()
.count();
Expand All @@ -262,7 +265,7 @@ impl Display for MessageCodeFrame<'_> {
let snippet = Snippet {
title: None,
slices: vec![Slice {
source: &source.text,
source: &source_text,
line_start: self.notebook_index.map_or_else(
|| start_index.get(),
|notebook_index| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,28 @@ invalid_characters.py:15:6: PLE2510 [*] Invalid unescaped character backspace, u
|
13 | # (Pylint, "C3002") => Rule::UnnecessaryDirectLambdaCall,
14 | #foo = 'hi'
15 | b = ''
| PLE2510
16 | b = f''
15 | b = ''
| ^ PLE2510
16 | b = f''
|
= help: Replace with escape sequence

ℹ Safe fix
12 12 | # (Pylint, "C0414") => Rule::UselessImportAlias,
13 13 | # (Pylint, "C3002") => Rule::UnnecessaryDirectLambdaCall,
14 14 | #foo = 'hi'
15 |-b = ''
15 |-b = ''
15 |+b = '\b'
16 16 | b = f''
16 16 | b = f''
17 17 |
18 18 | b_ok = '\\b'

invalid_characters.py:16:7: PLE2510 [*] Invalid unescaped character backspace, use "\b" instead
|
14 | #foo = 'hi'
15 | b = ''
16 | b = f''
| PLE2510
15 | b = ''
16 | b = f''
| ^ PLE2510
17 |
18 | b_ok = '\\b'
|
Expand All @@ -35,8 +35,8 @@ invalid_characters.py:16:7: PLE2510 [*] Invalid unescaped character backspace, u
ℹ Safe fix
13 13 | # (Pylint, "C3002") => Rule::UnnecessaryDirectLambdaCall,
14 14 | #foo = 'hi'
15 15 | b = ''
16 |-b = f''
15 15 | b = ''
16 |-b = f''
16 |+b = f'\b'
17 17 |
18 18 | b_ok = '\\b'
Expand All @@ -46,8 +46,8 @@ invalid_characters.py:55:21: PLE2510 [*] Invalid unescaped character backspace,
|
53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 |
55 | nested_fstrings = f'{f'{f''}'}'
| PLE2510
55 | nested_fstrings = f'{f'{f''}'}'
| ^ PLE2510
56 |
57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
|
Expand All @@ -57,10 +57,8 @@ invalid_characters.py:55:21: PLE2510 [*] Invalid unescaped character backspace,
52 52 | zwsp_after_multicharacter_grapheme_cluster = "ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
53 53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 54 |
55 |-nested_fstrings = f'{f'{f''}'}'
55 |+nested_fstrings = f'\b{f'{f''}'}'
55 |-nested_fstrings = f'{f'{f''}'}'
55 |+nested_fstrings = f'\b{f'{f''}'}'
56 56 |
57 57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
58 58 | x = f"""}}ab"""


Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ invalid_characters.py:55:25: PLE2512 [*] Invalid unescaped character SUB, use "\
|
53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 |
55 | nested_fstrings = f'{f'{f''}'}'
| PLE2512
55 | nested_fstrings = f'{f'{f''}'}'
| PLE2512
56 |
57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
|
Expand All @@ -56,8 +56,8 @@ invalid_characters.py:55:25: PLE2512 [*] Invalid unescaped character SUB, use "\
52 52 | zwsp_after_multicharacter_grapheme_cluster = "ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
53 53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 54 |
55 |-nested_fstrings = f'{f'{f''}'}'
55 |+nested_fstrings = f'{f'\x1A{f''}'}'
55 |-nested_fstrings = f'{f'{f''}'}'
55 |+nested_fstrings = f'{f'\x1A{f''}'}'
56 56 |
57 57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
58 58 | x = f"""}}ab"""
Expand All @@ -68,17 +68,15 @@ invalid_characters.py:58:12: PLE2512 [*] Invalid unescaped character SUB, use "\
58 | x = f"""}}ab"""
| PLE2512
59 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998256
60 | x = f"""}}ab"""
60 | x = f"""}}ab"""
|
= help: Replace with escape sequence

ℹ Safe fix
55 55 | nested_fstrings = f'{f'{f''}'}'
55 55 | nested_fstrings = f'{f'{f''}'}'
56 56 |
57 57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
58 |-x = f"""}}ab"""
58 |+x = f"""}}a\x1Ab"""
59 59 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998256
60 60 | x = f"""}}ab"""


60 60 | x = f"""}}a␛b"""
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,27 @@ invalid_characters.py:30:16: PLE2513 [*] Invalid unescaped character ESC, use "\
|
28 | sub_ok = f'\x1a'
29 |
30 | esc = 'esc esc '
| PLE2513
31 | esc = f'esc esc '
30 | esc = 'esc esc '
| ^ PLE2513
31 | esc = f'esc esc '
|
= help: Replace with escape sequence

ℹ Safe fix
27 27 | sub_ok = '\x1a'
28 28 | sub_ok = f'\x1a'
29 29 |
30 |-esc = 'esc esc '
30 |-esc = 'esc esc '
30 |+esc = 'esc esc \x1B'
31 31 | esc = f'esc esc '
31 31 | esc = f'esc esc '
32 32 |
33 33 | esc_ok = '\x1b'

invalid_characters.py:31:17: PLE2513 [*] Invalid unescaped character ESC, use "\x1B" instead
|
30 | esc = 'esc esc '
31 | esc = f'esc esc '
| PLE2513
30 | esc = 'esc esc '
31 | esc = f'esc esc '
| ^ PLE2513
32 |
33 | esc_ok = '\x1b'
|
Expand All @@ -34,8 +34,8 @@ invalid_characters.py:31:17: PLE2513 [*] Invalid unescaped character ESC, use "\
ℹ Safe fix
28 28 | sub_ok = f'\x1a'
29 29 |
30 30 | esc = 'esc esc '
31 |-esc = f'esc esc '
30 30 | esc = 'esc esc '
31 |-esc = f'esc esc '
31 |+esc = f'esc esc \x1B'
32 32 |
33 33 | esc_ok = '\x1b'
Expand All @@ -45,8 +45,8 @@ invalid_characters.py:55:29: PLE2513 [*] Invalid unescaped character ESC, use "\
|
53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 |
55 | nested_fstrings = f'{f'{f''}'}'
| PLE2513
55 | nested_fstrings = f'{f'{f''}'}'
| ^ PLE2513
56 |
57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
|
Expand All @@ -56,8 +56,8 @@ invalid_characters.py:55:29: PLE2513 [*] Invalid unescaped character ESC, use "\
52 52 | zwsp_after_multicharacter_grapheme_cluster = "ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
53 53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 54 |
55 |-nested_fstrings = f'{f'{f''}'}'
55 |+nested_fstrings = f'{f'{f'\x1B'}'}'
55 |-nested_fstrings = f'{f'{f''}'}'
55 |+nested_fstrings = f'{f'{f'\x1B'}'}'
56 56 |
57 57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
58 58 | x = f"""}}ab"""
Expand All @@ -66,16 +66,14 @@ invalid_characters.py:60:12: PLE2513 [*] Invalid unescaped character ESC, use "\
|
58 | x = f"""}}ab"""
59 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998256
60 | x = f"""}}ab"""
| PLE2513
60 | x = f"""}}ab"""
| ^ PLE2513
|
= help: Replace with escape sequence

ℹ Safe fix
57 57 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998106
58 58 | x = f"""}}ab"""
59 59 | # https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741998256
60 |-x = f"""}}ab"""
60 |-x = f"""}}ab"""
60 |+x = f"""}}a\x1Bb"""


Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ invalid_characters.py:52:60: PLE2515 [*] Invalid unescaped character zero-width-
52 |+zwsp_after_multicharacter_grapheme_cluster = "ಫ್ರಾನ್ಸಿಸ್ಕೊ \u200b​"
53 53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 54 |
55 55 | nested_fstrings = f'{f'{f''}'}'
55 55 | nested_fstrings = f'{f'{f''}'}'

invalid_characters.py:52:61: PLE2515 [*] Invalid unescaped character zero-width-space, use "\u200B" instead
|
Expand All @@ -120,7 +120,7 @@ invalid_characters.py:52:61: PLE2515 [*] Invalid unescaped character zero-width-
52 |+zwsp_after_multicharacter_grapheme_cluster = "ಫ್ರಾನ್ಸಿಸ್ಕೊ ​\u200b"
53 53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
54 54 |
55 55 | nested_fstrings = f'{f'{f''}'}'
55 55 | nested_fstrings = f'{f'{f''}'}'

invalid_characters.py:53:61: PLE2515 [*] Invalid unescaped character zero-width-space, use "\u200B" instead
|
Expand All @@ -129,7 +129,7 @@ invalid_characters.py:53:61: PLE2515 [*] Invalid unescaped character zero-width-
53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
| PLE2515
54 |
55 | nested_fstrings = f'{f'{f''}'}'
55 | nested_fstrings = f'{f'{f''}'}'
|
= help: Replace with escape sequence

Expand All @@ -140,7 +140,7 @@ invalid_characters.py:53:61: PLE2515 [*] Invalid unescaped character zero-width-
53 |-zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
53 |+zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ \u200b​"
54 54 |
55 55 | nested_fstrings = f'{f'{f''}'}'
55 55 | nested_fstrings = f'{f'{f''}'}'
56 56 |

invalid_characters.py:53:62: PLE2515 [*] Invalid unescaped character zero-width-space, use "\u200B" instead
Expand All @@ -150,7 +150,7 @@ invalid_characters.py:53:62: PLE2515 [*] Invalid unescaped character zero-width-
53 | zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
| PLE2515
54 |
55 | nested_fstrings = f'{f'{f''}'}'
55 | nested_fstrings = f'{f'{f''}'}'
|
= help: Replace with escape sequence

Expand All @@ -161,7 +161,5 @@ invalid_characters.py:53:62: PLE2515 [*] Invalid unescaped character zero-width-
53 |-zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​​"
53 |+zwsp_after_multicharacter_grapheme_cluster = f"ಫ್ರಾನ್ಸಿಸ್ಕೊ ​\u200b"
54 54 |
55 55 | nested_fstrings = f'{f'{f''}'}'
56 56 |


55 55 | nested_fstrings = f'␈{f'{f'␛'}'}'
56 56 |
12 changes: 7 additions & 5 deletions crates/ruff_linter/src/source_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use ruff_python_ast::PySourceType;
use colored::Colorize;

use crate::fs;
use crate::text_helpers::ShowNonprinting;

#[derive(Clone, Debug, PartialEq, is_macro::Is)]
pub enum SourceKind {
Expand Down Expand Up @@ -220,8 +221,8 @@ impl<'a> CodeDiff<'a> {
impl std::fmt::Display for CodeDiff<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some((original, modified)) = self.header {
writeln!(f, "--- {}", original.red())?;
writeln!(f, "+++ {}", modified.green())?;
writeln!(f, "--- {}", original.show_nonprinting().red())?;
writeln!(f, "+++ {}", modified.show_nonprinting().green())?;
}

let mut unified = self.diff.unified_diff();
Expand All @@ -233,10 +234,11 @@ impl std::fmt::Display for CodeDiff<'_> {

// individual lines
for change in hunk.iter_changes() {
let value = change.value().show_nonprinting();
match change.tag() {
ChangeTag::Equal => write!(f, " {}", change.value())?,
ChangeTag::Delete => write!(f, "{}{}", "-".red(), change.value().red())?,
ChangeTag::Insert => write!(f, "{}{}", "+".green(), change.value().green())?,
ChangeTag::Equal => write!(f, " {value}")?,
ChangeTag::Delete => write!(f, "{}{}", "-".red(), value.red())?,
ChangeTag::Insert => write!(f, "{}{}", "+".green(), value.green())?,
}

if !self.diff.newline_terminated() {
Expand Down
23 changes: 23 additions & 0 deletions crates/ruff_linter/src/text_helpers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use std::borrow::Cow;

pub(crate) trait ShowNonprinting {
fn show_nonprinting(&self) -> Cow<'_, str>;
}

macro_rules! impl_show_nonprinting {
($(($from:expr, $to:expr)),+) => {
impl ShowNonprinting for str {
fn show_nonprinting(&self) -> Cow<'_, str> {
if self.find(&[$($from),*][..]).is_some() {
Cow::Owned(
self.$(replace($from, $to)).*
)
} else {
Cow::Borrowed(self)
}
}
}
};
}

impl_show_nonprinting!(('\x07', "␇"), ('\x08', "␈"), ('\x1b', "␛"), ('\x7f', "␡"));
Loading