Code review feedback

crates/ruff_formatter/src/format_element.rs

@@ -399,19 +399,39 @@ pub trait FormatElements {
     fn end_tag(&self, kind: TagKind) -> Option<&Tag>;
 }
 
+/// New-type wrapper for a single-line text unicode width.
+/// Mainly to prevent access to the inner value.
+///
+/// ## Representation
+///
+/// Represents the width by adding 1 to the actual width so that the width can be represented by a [`NonZeroU32`],
+/// allowing [`TextWidth`] or [`Option<Width>`] fit in 4 bytes rather than 8.
+///
+/// This means that 2^32 can not be precisely represented and instead has the same value as 2^32-1.
+/// This imprecision shouldn't matter in practice because either text are longer than any configured line width
+/// and thus, the text should break.
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub struct Width(NonZeroU32);
+
+impl Width {
+    pub(crate) const fn new(width: u32) -> Self {
+        Width(NonZeroU32::MIN.saturating_add(width))
+    }
+
+    pub const fn value(self) -> u32 {
+        self.0.get() - 1
+    }
+}
+
+/// The pre-computed unicode width of a text if it is a single-line text or a marker
+/// that it is a multiline text if it contains a line feed.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum TextWidth {
-    Width(NonZeroU32),
+    Width(Width),
     Multiline,
 }
 
 impl TextWidth {
-    pub(crate) const fn new_width(width: u32) -> Self {
-        // SAFETY: 1 + x is guaranteed to be non zero
-        #[allow(unsafe_code)]
-        TextWidth::Width(unsafe { NonZeroU32::new_unchecked(width.saturating_add(1)) })
-    }
-
     pub fn from_text(text: &str, tab_width: TabWidth) -> TextWidth {
         let mut width = 0u32;
 
@@ -425,12 +445,12 @@ impl TextWidth {
             width += char_width;
         }
 
-        Self::new_width(width)
+        Self::Width(Width::new(width))
     }
 
-    pub const fn width(self) -> Option<u32> {
+    pub const fn width(self) -> Option<Width> {
         match self {
-            TextWidth::Width(width) => Some(width.get() - 1),
+            TextWidth::Width(width) => Some(width),
             TextWidth::Multiline => None,
         }
     }
@@ -467,19 +487,21 @@ mod sizes {
     // be recomputed at a later point in time?
     // You reduced the size of a format element? Excellent work!
 
+    use super::{BestFittingVariants, Interned, TextWidth};
     use static_assertions::assert_eq_size;
 
     assert_eq_size!(ruff_text_size::TextRange, [u8; 8]);
-    assert_eq_size!(crate::prelude::tag::VerbatimKind, [u8; 8]);
-    assert_eq_size!(crate::prelude::Interned, [u8; 16]);
-    assert_eq_size!(crate::format_element::BestFittingVariants, [u8; 16]);
+    assert_eq_size!(TextWidth, [u8; 4]);
+    assert_eq_size!(super::tag::VerbatimKind, [u8; 8]);
+    assert_eq_size!(Interned, [u8; 16]);
+    assert_eq_size!(BestFittingVariants, [u8; 16]);
 
     #[cfg(not(debug_assertions))]
     assert_eq_size!(crate::SourceCodeSlice, [u8; 8]);
 
     #[cfg(not(debug_assertions))]
-    assert_eq_size!(crate::format_element::Tag, [u8; 16]);
+    assert_eq_size!(super::Tag, [u8; 16]);
 
     #[cfg(not(debug_assertions))]
-    assert_eq_size!(crate::FormatElement, [u8; 24]);
+    assert_eq_size!(super::FormatElement, [u8; 24]);
 }

crates/ruff_formatter/src/format_element/document.rs

@@ -263,7 +263,7 @@ impl Format<IrFormatContext<'_>> for &[FormatElement] {
                         let (text, text_width) = match element {
                             #[allow(clippy::cast_possible_truncation)]
                             FormatElement::Token { text } => {
-                                (*text, TextWidth::new_width(text.len() as u32))
+                                (*text, TextWidth::Width(Width::new(text.len() as u32)))
                             }
                             FormatElement::Text { text, text_width } => {
                                 (text.as_ref(), *text_width)

crates/ruff_formatter/src/printer/mod.rs

@@ -99,7 +99,7 @@ impl<'a> Printer<'a> {
             FormatElement::Text { text, text_width } => self.print_text(
                 Text::Text {
                     text,
-                    width: *text_width,
+                    text_width: *text_width,
                 },
                 None,
             ),
@@ -108,7 +108,7 @@ impl<'a> Printer<'a> {
                 self.print_text(
                     Text::Text {
                         text,
-                        width: *text_width,
+                        text_width: *text_width,
                     },
                     Some(slice.range()),
                 );
@@ -407,10 +407,13 @@ impl<'a> Printer<'a> {
                 self.state.buffer.push_str(token);
                 self.state.line_width += token.len() as u32;
             }
-            Text::Text { text, width } => {
+            Text::Text {
+                text,
+                text_width: width,
+            } => {
                 if let Some(width) = width.width() {
                     self.state.buffer.push_str(text);
-                    self.state.line_width += width;
+                    self.state.line_width += width.value();
                 } else {
                     for char in text.chars() {
                         self.print_char(char);
@@ -1107,7 +1110,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
                 return Ok(self.fits_text(
                     Text::Text {
                         text,
-                        width: *text_width,
+                        text_width: *text_width,
                     },
                     args,
                 ))
@@ -1117,7 +1120,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
                 return Ok(self.fits_text(
                     Text::Text {
                         text,
-                        width: *text_width,
+                        text_width: *text_width,
                     },
                     args,
                 ));
@@ -1338,9 +1341,9 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
             Text::Token(token) => {
                 self.state.line_width += token.len() as u32;
             }
-            Text::Text { text, width } => {
-                if let Some(width) = width.width() {
-                    self.state.line_width += width;
+            Text::Text { text, text_width } => {
+                if let Some(width) = text_width.width() {
+                    self.state.line_width += width.value();
                 } else {
                     for c in text.chars() {
                         let char_width = match c {
@@ -1486,7 +1489,10 @@ enum Text<'a> {
     /// ASCII only text that contains no line breaks or tab characters.
     Token(&'a str),
     /// Arbitrary text. May contain `\n` line breaks, tab characters, or unicode characters.
-    Text { text: &'a str, width: TextWidth },
+    Text {
+        text: &'a str,
+        text_width: TextWidth,
+    },
 }
 
 #[cfg(test)]

crates/ruff_python_formatter/src/comments/format.rs

@@ -375,10 +375,12 @@ impl Format<PyFormatContext<'_>> for FormatTrailingEndOfLineComment<'_> {
             0
         } else {
             // Start with 2 because of the two leading spaces.
-            let width = TextWidth::from_text(&normalized_comment, f.options().tab_width())
-                .width()
-                .expect("Expected comment not to contain any newlines")
-                + 2;
+            let width = 2u32.saturating_add(
+                TextWidth::from_text(&normalized_comment, f.options().tab_width())
+                    .width()
+                    .expect("Expected comment not to contain any newlines")
+                    .value(),
+            );
 
             width
         };