diff --git a/crates/ruff_python_literal/src/char.rs b/crates/ruff_python_literal/src/char.rs index d03731017e..46233c0cc8 100644 --- a/crates/ruff_python_literal/src/char.rs +++ b/crates/ruff_python_literal/src/char.rs @@ -24,17 +24,3 @@ pub(crate) fn is_printable(c: char) -> bool { | GeneralCategory::SpaceSeparator ) } - -/// Returns whether the character has a Unicode [mark category] (Mn, Mc, or Me). -/// Characters in these categories are called [combining marks]. -/// -/// [mark category]: https://www.unicode.org/reports/tr44/#General_Category_Values -/// [combining marks]: https://www.unicode.org/glossary/#combining_character -pub(crate) fn is_combining_mark(c: char) -> bool { - matches!( - GeneralCategory::for_char(c), - GeneralCategory::NonspacingMark - | GeneralCategory::SpacingMark - | GeneralCategory::EnclosingMark - ) -} diff --git a/crates/ruff_python_literal/src/escape.rs b/crates/ruff_python_literal/src/escape.rs index b0ee1f08d1..9f0cc4fbec 100644 --- a/crates/ruff_python_literal/src/escape.rs +++ b/crates/ruff_python_literal/src/escape.rs @@ -1,6 +1,8 @@ use icu_normalizer::ComposingNormalizer; -use icu_properties::{CodePointSetData, props::DefaultIgnorableCodePoint}; -use itertools::Either; +use icu_properties::{ + CodePointSetData, + props::{DefaultIgnorableCodePoint, EnumeratedProperty, GraphemeClusterBreak}, +}; use ruff_python_ast::{ BytesLiteralFlags, StringFlags, StringLiteralFlags, str::{Quote, TripleQuotes}, @@ -53,7 +55,7 @@ pub(crate) const fn choose_quote( pub struct UnicodeEscape<'a> { source: &'a str, layout: EscapeLayout, - escape_for_display: bool, + display_escapes: DisplayEscapes, } impl<'a> UnicodeEscape<'a> { @@ -63,38 +65,36 @@ impl<'a> UnicodeEscape<'a> { Self { source, layout, - escape_for_display: false, + display_escapes: DisplayEscapes::NotComputed, } } - /// Configures the representation to escape [default-ignorable characters], which may be - /// invisible. It also escapes combining marks that would otherwise attach to the opening quote - /// or an escape sequence in the output. + /// Configures the representation to distinguish [canonically equivalent] strings and escape + /// [default-ignorable characters] and other potentially invisible characters. It also prevents + /// characters from appearing as part of the quotes or escape sequences. /// - /// To distinguish [canonically equivalent] strings, it escapes non-ASCII characters that can - /// participate in normalization in parts of the string that are not in [NFC]. Characters - /// with the [`NfcInert`] property cannot interact with adjacent characters during normalization - /// and separate these parts. + /// The resulting representation is in [NFC], but still evaluates to the original string. + /// For example, `"é"` and `"e\u0301"` are shown differently, even though the strings have + /// the same NFC form. /// /// [default-ignorable characters]: https://www.unicode.org/reports/tr44/#Default_Ignorable_Code_Point /// [NFC]: https://www.unicode.org/reports/tr15/#Norm_Forms /// [canonically equivalent]: https://www.unicode.org/reports/tr15/#Canon_Compat_Equivalence - /// [`NfcInert`]: https://docs.rs/icu_properties/latest/icu_properties/props/struct.NfcInert.html #[must_use] pub fn escape_for_display(mut self) -> Self { - // ASCII is already NFC and has no combining marks or default-ignorable characters, so it - // needs no additional escaping for display. - if self.escape_for_display || self.source.is_ascii() { + match self.display_escapes { + DisplayEscapes::NotComputed => {} + DisplayEscapes::Computed(_) => return self, + } + + if self.source.is_ascii() { return self; } - self.escape_for_display = true; - let mut follows_syntax = true; - for (ch, escape_for_normalization) in - Self::display_chars(self.source, self.escape_for_display) - { - let escape = self.display_escape(ch, follows_syntax, escape_for_normalization); - follows_syntax = escape.next_follows_syntax; - if escape.should_escape { + + let display_escapes = Self::display_escapes(self.source, self.layout.quote); + + for &index in &display_escapes { + if let Some(ch) = self.source[index..].chars().next() { let extra = Self::escaped_codepoint_len(ch) - ch.len_utf8(); self.layout.len = self .layout @@ -103,6 +103,9 @@ impl<'a> UnicodeEscape<'a> { .filter(|&len| len <= isize::MAX as usize - Self::REPR_RESERVED_LEN); } } + + self.display_escapes = DisplayEscapes::Computed(display_escapes); + self } @@ -149,14 +152,6 @@ impl std::fmt::Display for StrRepr<'_, '_> { } } -/// Whether to apply additional escaping to the current character, and, when display escaping is -/// enabled, whether the next character would immediately follow an escape sequence in the output. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct DisplayEscape { - should_escape: bool, - next_follows_syntax: bool, -} - impl UnicodeEscape<'_> { const REPR_RESERVED_LEN: usize = 2; // for quotes @@ -239,67 +234,170 @@ impl UnicodeEscape<'_> { } } - /// Yields characters and whether to escape them for display to distinguish canonically - /// equivalent strings, when display escaping is enabled. + /// Finds the byte offsets of characters that need escaping beyond Python's ordinary repr. /// - /// Characters with the [`NfcInert`] property cannot interact with adjacent characters during - /// normalization, so they provide boundaries between independently normalized segments. - /// - /// [`NfcInert`]: https://docs.rs/icu_properties/latest/icu_properties/props/struct.NfcInert.html - fn display_chars(source: &str, escape_for_display: bool) -> impl Iterator { - if !escape_for_display || ComposingNormalizer::new_nfc().is_normalized(source) { - return Either::Left(source.chars().map(|ch| (ch, false))); + /// ASCII escapes separate runs of characters written without escaping. Within each run, escape + /// a character if adding it would make the run non-NFC, then start a new run. Characters that + /// would attach to the resulting escape also need escaping. This leaves unrelated characters + /// visible even when another part of the string needs an escape. + fn display_escapes(source: &str, quote: Quote) -> Vec { + let normalizer = ComposingNormalizer::new_nfc(); + let check_normalization = !normalizer.is_normalized(source); + let default_ignorables = CodePointSetData::new::(); + + // The braille blank is printable but often looks like an ordinary space. + let invisible = |ch| ch == '\u{2800}' || default_ignorables.contains(ch); + + // Characters escaped by the ordinary Python representation and invisible characters end a + // run regardless of whether it is in NFC. + let always_escaped = |ch: char| { + ch == quote.as_char() + || Self::escaped_char_len(ch) != ch.len_utf8() + || (!ch.is_ascii() && invisible(ch)) + }; + + let mut escapes = Vec::new(); + let mut position = 0; + + // The first character follows the opening quote. After an escape, the next character can + // likewise attach to the escape's final character rather than to a character in the string. + let mut follows_syntax = true; + + while let Some(ch) = source[position..].chars().next() { + let end = position + ch.len_utf8(); + + let additional = !ch.is_ascii() + && crate::char::is_printable(ch) + && (invisible(ch) || (follows_syntax && Self::attaches_to_syntax(ch))); + + if additional || always_escaped(ch) { + Self::escape_preceding_prepends(source, position, &mut escapes); + + // The writer handles ordinary Python escapes. Record only the additional escapes + // needed for display, so their lengths can be accounted for separately. + if additional { + escapes.push(position); + } + + follows_syntax = true; + position = end; + continue; + } + + let run_end = source[position..] + .char_indices() + .find(|(_, ch)| always_escaped(*ch)) + .map(|(index, _)| position + index) + .unwrap_or(source.len()); + + // A substring of an NFC string is also NFC, so an NFC source cannot contain a run + // that needs an escape for normalization. + if !check_normalization { + position = run_end; + follows_syntax = false; + continue; + } + + // In the Rust string `"e\u{301}x"`, `e` is NFC but adding the accent is not. Escaping + // the accent leaves `e` visible, so with double quotes the display is `"e\u0301x"`. + while position < run_end { + let run = &source[position..run_end]; + + let Some(index) = Self::first_non_nfc_char(run) else { + position = run_end; + follows_syntax = false; + break; + }; + + position += index; + + if let Some(ch) = source[position..].chars().next() { + Self::escape_preceding_prepends(source, position, &mut escapes); + escapes.push(position); + position += ch.len_utf8(); + follows_syntax = true; + } + + // A character immediately following the new escape could attach to the escape's + // final ASCII character. Escape it as well if it can do so. + while let Some(ch) = source[position..run_end].chars().next() { + if !Self::attaches_to_syntax(ch) { + break; + } + escapes.push(position); + position += ch.len_utf8(); + } + } } - // We've asked the upstream maintainers to reconsider the deprecation. See - // https://github.com/unicode-org/icu4x/issues/7892#issuecomment-6045495719. - #[expect(deprecated)] - let inert = CodePointSetData::new::(); - Either::Right( - source - .split_inclusive(move |ch| inert.contains(ch)) - .flat_map(move |segment| { - let escape = !ComposingNormalizer::new_nfc().is_normalized(segment); - segment.chars().map(move |ch| { - let escape = escape - && !ch.is_ascii() - && !inert.contains(ch) - && crate::char::is_printable(ch); - (ch, escape) - }) - }), + // A trailing Prepend character could attach to the closing quote, even if the final run + // is already NFC. + Self::escape_preceding_prepends(source, source.len(), &mut escapes); + escapes + } + + /// Records offsets of characters that would attach to the following escape or closing quote. + fn escape_preceding_prepends(source: &str, end: usize, escapes: &mut Vec) { + // `GraphemeClusterBreak::Prepend` characters can join the following character. Once one is + // escaped, a preceding `Prepend` would join the escape's backslash, so escape that too. + // + // See Unicode's grapheme cluster boundary rules: + // https://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundary_Rules + let start = source[..end] + .char_indices() + .rev() + .take_while(|(index, ch)| { + GraphemeClusterBreak::for_char(*ch) == GraphemeClusterBreak::Prepend + && crate::char::is_printable(*ch) + && escapes.last().is_none_or(|last| index > last) + }) + .last() + .map(|(index, _)| index) + .unwrap_or(end); + + escapes.extend( + source[start..end] + .char_indices() + .map(|(index, _)| start + index), + ); + } + + /// Returns whether a character can join a preceding ASCII quote or escape sequence. + fn attaches_to_syntax(ch: char) -> bool { + matches!( + GraphemeClusterBreak::for_char(ch), + GraphemeClusterBreak::Extend + | GraphemeClusterBreak::SpacingMark + | GraphemeClusterBreak::ZWJ ) } - /// Returns whether `ch` needs additional escaping and, when display escaping is enabled, - /// whether the next character would immediately follow an escape sequence in the output. - /// - /// When display escaping is enabled, printable default-ignorable characters are escaped. - /// `follows_syntax` is true if `ch` would immediately follow the opening quote or an escape - /// sequence; in that case, combining marks are also escaped. `escape_for_normalization` marks - /// characters in a non-NFC segment that can participate in normalization. - fn display_escape( - &self, - ch: char, - follows_syntax: bool, - escape_for_normalization: bool, - ) -> DisplayEscape { - if !self.escape_for_display { - return DisplayEscape { - should_escape: false, - next_follows_syntax: follows_syntax, - }; + /// Returns the byte offset of the first character that makes its prefix of `source` non-NFC, + /// or `None` if `source` is in NFC. + fn first_non_nfc_char(source: &str) -> Option { + let normalizer = ComposingNormalizer::new_nfc(); + + if normalizer.is_normalized(source) { + return None; } - let should_escape = escape_for_normalization - || (!ch.is_ascii() - && ((follows_syntax && crate::char::is_combining_mark(ch)) - || (CodePointSetData::new::().contains(ch) - && crate::char::is_printable(ch)))); - DisplayEscape { - should_escape, - next_follows_syntax: should_escape - || ch == self.layout.quote.as_char() - || Self::escaped_char_len(ch) != ch.len_utf8(), + + // NFC is closed under substringing, so every prefix after the first non-NFC prefix is + // also non-NFC. Search at character boundaries to identify the character that changes it. + let mut normalized = 0; + let mut not_normalized = source.len(); + + loop { + let middle = source.floor_char_boundary(normalized + (not_normalized - normalized) / 2); + + if middle == normalized { + return Some(normalized); + } + + if normalizer.is_normalized(&source[..middle]) { + normalized = middle; + } else { + not_normalized = middle; + } } } @@ -354,19 +452,23 @@ impl Escape for UnicodeEscape<'_> { #[cold] fn write_body_slow(&self, formatter: &mut impl std::fmt::Write) -> std::fmt::Result { - let mut follows_syntax = true; - for (ch, escape_for_normalization) in - Self::display_chars(self.source, self.escape_for_display) - { - let display_escape = self.display_escape(ch, follows_syntax, escape_for_normalization); - follows_syntax = display_escape.next_follows_syntax; - Self::write_char( - ch, - self.layout.quote, - display_escape.should_escape, - formatter, - )?; + let display_escapes: &[usize] = match &self.display_escapes { + DisplayEscapes::NotComputed => &[], + DisplayEscapes::Computed(offsets) => offsets, + }; + + let mut display_escapes = display_escapes.iter().peekable(); + + for (index, ch) in self.source.char_indices() { + let force_escape = display_escapes + .peek() + .is_some_and(|&&escape| escape == index); + if force_escape { + display_escapes.next(); + } + Self::write_char(ch, self.layout.quote, force_escape, formatter)?; } + Ok(()) } } @@ -537,6 +639,25 @@ impl std::fmt::Display for BytesRepr<'_, '_> { } } +/// Whether additional display escapes have been calculated for a string. +#[derive(Debug)] +enum DisplayEscapes { + /// Display escaping has not been calculated for the given string. + /// + /// ASCII strings stay in this state even after [`UnicodeEscape::escape_for_display`] is called + /// because they need no additional display escapes. + NotComputed, + + /// Display escaping has been calculated for the given string. + /// + /// The wrapped vector contains the byte offsets of characters that need extra escaping. The + /// vector may be empty. + /// + /// This state lets [`UnicodeEscape::escape_for_display`] skip recalculation, which also prevents + /// it from adding extra escape lengths to the layout twice. + Computed(Vec), +} + #[cfg(test)] mod unicode_escape_tests { use super::*; @@ -623,6 +744,23 @@ mod unicode_escape_tests { } } + #[test] + fn display_representations_are_normalized() { + // Check that every scalar has an NFC display both after `e` and before `e` followed by a + // combining acute accent, which must be escaped. + let nfc = icu_normalizer::ComposingNormalizer::new_nfc(); + for codepoint in 0..=0x0010_ffff { + let Some(ch) = char::from_u32(codepoint) else { + continue; + }; + for source in [format!("e{ch}"), format!("{ch}e\u{0301}")] { + let escaped = UnicodeEscape::new_repr(&source).escape_for_display(); + let display = format!("{}", escaped.str_repr(TripleQuotes::No)); + assert!(nfc.is_normalized(&display), "{source:?} -> {display:?}"); + } + } + } + #[test] fn default_ignorable_characters() { let source = "a\u{034f}\u{0301}\u{115f}\u{200b}"; diff --git a/crates/ty_python_core/src/ast_node_ref.rs b/crates/ty_python_core/src/ast_node_ref.rs index 59c04256f5..8cb3ed6460 100644 --- a/crates/ty_python_core/src/ast_node_ref.rs +++ b/crates/ty_python_core/src/ast_node_ref.rs @@ -124,16 +124,15 @@ where { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { cfg_select! { - debug_assertions => { - f.debug_struct("AstNodeRef") - .field("kind", &self.kind) - .field("range", &self.range) - .finish() - }, + debug_assertions => f + .debug_struct("AstNodeRef") + .field("kind", &self.kind) + .field("range", &self.range) + .finish(), _ => { // Unfortunately we have no access to the AST here. f.debug_tuple("AstNodeRef").finish_non_exhaustive() - }, + } } } } diff --git a/crates/ty_python_semantic/resources/mdtest/literal/string.md b/crates/ty_python_semantic/resources/mdtest/literal/string.md index b2fb0bc03e..ed5d64cdb5 100644 --- a/crates/ty_python_semantic/resources/mdtest/literal/string.md +++ b/crates/ty_python_semantic/resources/mdtest/literal/string.md @@ -32,9 +32,8 @@ reveal_type("\x1b\u200b\U000e0001") # revealed: Literal["\x1b\u200b\U000e0001"] ## Combining marks -When displaying a string literal, ty escapes [combining marks] at the start of the string and after -characters displayed as escape sequences. This prevents the marks from attaching to the opening -quote or to the escape sequence. +When displaying a string literal, ty escapes [combining marks] that would attach to the opening +quote or to an escape sequence. ```py reveal_type("\ufe20") # revealed: Literal["\ufe20"] @@ -51,11 +50,48 @@ reveal_type("\\\u0301") # revealed: Literal["\\\u0301"] reveal_type("\U0001d167") # revealed: Literal["\U0001d167"] ``` +Unicode's grapheme rules treat the Myanmar vowel sign `ါ` as separate from a preceding quote, so ty +can display it without an escape: + +```py +reveal_type("\u102b") # revealed: Literal["ါ"] +``` + +## Other characters that attach to quotes + +Some characters can attach to a quote even though they are not combining marks. An emoji modifier +and the Thai character `ำ` attach to the preceding character, while Malayalam dot reph `ൎ` attaches +to the following character. ty escapes them next to the corresponding quote and leaves them visible +when they are joined to a character inside the string. + +```py +reveal_type("\U0001f3fb") # revealed: Literal["\U0001f3fb"] +reveal_type("👍🏻") # revealed: Literal["👍🏻"] +reveal_type("\u0e33") # revealed: Literal["\u0e33"] +reveal_type("กำ") # revealed: Literal["กำ"] +reveal_type("\u0d4e") # revealed: Literal["\u0d4e"] +reveal_type("ൎക") # revealed: Literal["ൎക"] +``` + +An escape sequence also has to stay separate from a preceding Malayalam dot reph: + +```py +reveal_type("\u0d4e\u200b") # revealed: Literal["\u0d4e\u200b"] +``` + +The Angstrom sign `\u212b` normalizes to `Å` in NFC, so ty displays it as an escape to keep the two +values distinct. A dot reph before that escape would attach to it, so ty escapes it too. A preceding +dot reph would then attach to this new escape and is escaped as well: + +```py +reveal_type("\u0d4e\u0d4e\u212b") # revealed: Literal["\u0d4e\u0d4e\u212b"] +``` + ## Canonically equivalent strings -Distinct strings can render identically when they are [canonically equivalent]. ty escapes non-ASCII -characters that can participate in normalization in parts of the string that are not in [NFC], so -these strings can be distinguished. +Distinct strings can render identically when they are [canonically equivalent]. ty escapes +characters that would make the display non-[NFC], so these strings can be distinguished without +changing their values. NFC combines `e\u0301` into `é`, whereas `q\u0301` is already in NFC. It also puts combining marks into a standard order. @@ -70,14 +106,20 @@ reveal_type("é e\u0301") # revealed: Literal["é e\u0301"] reveal_type("Å") # revealed: Literal["Å"] reveal_type("\u212b") # revealed: Literal["\u212b"] reveal_type("가") # revealed: Literal["가"] -reveal_type("\u1100\u1161") # revealed: Literal["\u1100\u1161"] -reveal_type("q\u0315\u0300") # revealed: Literal["q\u0315\u0300"] +reveal_type("\u1100\u1161") # revealed: Literal["ᄀ\u1161"] +reveal_type("q\u0315\u0300") # revealed: Literal["q̕\u0300"] reveal_type("q\u0300\u0315") # revealed: Literal["q̀̕"] def equivalent_literals(value: Literal["é", "e\u0301"]): reveal_type(value) # revealed: Literal["é", "e\u0301"] ``` +Characters elsewhere in the string can remain visible when another character needs an escape: + +```py +reveal_type("é⏱\u212b") # revealed: Literal["é⏱\u212b"] +``` + ## Default-ignorable characters [Default-ignorable characters] can be invisible when rendered, so ty escapes them to distinguish @@ -103,6 +145,15 @@ from attaching to the escape sequence: reveal_type("a\u034f\u0301") # revealed: Literal["a\u034f\u0301"] ``` +## Braille blank + +The braille blank can look like an ordinary space, so ty displays it as an escape: + +```py +reveal_type(" ") # revealed: Literal[" "] +reveal_type("\u2800") # revealed: Literal["\u2800"] +``` + [canonically equivalent]: https://www.unicode.org/reports/tr15/#Canon_Compat_Equivalence [combining marks]: https://www.unicode.org/versions/Unicode17.0.0/core-spec/chapter-7/#G18130 [default-ignorable characters]: https://www.unicode.org/reports/tr44/#Default_Ignorable_Code_Point