fix(fmt): account for CRLF when handling cursor (#11874)

* fix(fmt): account for CRLF when handling cursor

* style: clippy

* perf: initial scan + avoid snippet alloc

* fix: handle oob

* style: helper fn for line break logic

Author: 0xrusowsky

crates/fmt/src/state/common.rs

@@ -147,7 +147,7 @@ impl<'ast> State<'_, 'ast> {
         let quote = match self.config.quote_style {
             config::QuoteStyle::Double => '\"',
             config::QuoteStyle::Single => '\'',
-            config::QuoteStyle::Preserve => self.char_at(quote_pos),
+            config::QuoteStyle::Preserve => self.char_at(quote_pos).unwrap_or_default(),
         };
         debug_assert!(matches!(quote, '\"' | '\''), "{quote:?}");
         let s = solar::parse::interface::data_structures::fmt::from_fn(move |f| {
@@ -546,7 +546,7 @@ impl<'ast> State<'_, 'ast> {
                                 self.s.offset(offset);
                             }
                         } else if style.is_isolated() {
-                            Separator::Space.print(&mut self.s, &mut self.cursor);
+                            self.print_sep_unhandled(Separator::Space);
                             self.s.offset(offset);
                         }
                     }
@@ -555,7 +555,7 @@ impl<'ast> State<'_, 'ast> {
                             self.zerobreak();
                             self.s.offset(offset);
                         } else if self.cursor.enabled {
-                            Separator::Space.print(&mut self.s, &mut self.cursor);
+                            self.print_sep_unhandled(Separator::Space);
                             self.s.offset(offset);
                             self.cursor.advance_to(block_lo, true);
                         }

crates/fmt/src/state/mod.rs

@@ -129,6 +129,7 @@ pub(super) struct State<'sess, 'ast> {
     inline_config: InlineConfig<()>,
     cursor: SourcePos,
 
+    has_crlf: bool,
     contract: Option<&'ast ast::ItemContract<'ast>>,
     single_line_stmt: Option<bool>,
     named_call_expr: bool,
@@ -170,6 +171,10 @@ impl SourcePos {
         self.enabled = enabled;
     }
 
+    pub(super) fn next_line(&mut self, is_at_crlf: bool) {
+        self.pos += if is_at_crlf { 2 } else { 1 };
+    }
+
     pub(super) fn span(&self, to: BytePos) -> Span {
         Span::new(self.pos, to)
     }
@@ -183,14 +188,15 @@ pub(super) enum Separator {
 }
 
 impl Separator {
-    fn print(&self, p: &mut pp::Printer, cursor: &mut SourcePos) {
+    fn print(&self, p: &mut pp::Printer, cursor: &mut SourcePos, is_at_crlf: bool) {
         match self {
             Self::Nbsp => p.nbsp(),
             Self::Space => p.space(),
             Self::Hardbreak => p.hardbreak(),
             Self::SpaceOrNbsp(breaks) => p.space_or_nbsp(*breaks),
         }
-        cursor.advance(1);
+
+        cursor.next_line(is_at_crlf);
     }
 }
 
@@ -217,6 +223,7 @@ impl<'sess> State<'sess, '_> {
             config,
             inline_config,
             cursor: SourcePos { pos: BytePos::from_u32(0), enabled: true },
+            has_crlf: false,
             contract: None,
             single_line_stmt: None,
             named_call_expr: false,
@@ -228,6 +235,25 @@ impl<'sess> State<'sess, '_> {
         }
     }
 
+    /// Checks a span of the source for a carriage return (`\r`) to determine if the file
+    /// uses CRLF line endings.
+    ///
+    /// If a `\r` is found, `self.has_crlf` is set to `true`. This is intended to be
+    /// called once at the beginning of the formatting process for efficiency.
+    fn check_crlf(&mut self, span: Span) {
+        if let Ok(snip) = self.sm.span_to_snippet(span)
+            && snip.contains('\r')
+        {
+            self.has_crlf = true;
+        }
+    }
+
+    /// Checks if the cursor is currently positioned at the start of a CRLF sequence (`\r\n`).
+    /// The check is only meaningful if `self.has_crlf` is true.
+    fn is_at_crlf(&self) -> bool {
+        self.has_crlf && self.char_at(self.cursor.pos) == Some('\r')
+    }
+
     fn space_left(&self) -> usize {
         std::cmp::min(
             self.s.space_left(),
@@ -272,9 +298,9 @@ impl<'sess> State<'sess, '_> {
 
 /// Span to source.
 impl State<'_, '_> {
-    fn char_at(&self, pos: BytePos) -> char {
+    fn char_at(&self, pos: BytePos) -> Option<char> {
         let res = self.sm.lookup_byte_offset(pos);
-        res.sf.src[res.pos.to_usize()..].chars().next().unwrap()
+        res.sf.src.get(res.pos.to_usize()..)?.chars().next()
     }
 
     fn print_span(&mut self, span: Span) {
@@ -340,11 +366,19 @@ impl State<'_, '_> {
     }
 
     fn print_sep(&mut self, sep: Separator) {
-        if self.handle_span(self.cursor.span(self.cursor.pos + BytePos(1)), true) {
+        if self.handle_span(
+            self.cursor.span(self.cursor.pos + if self.is_at_crlf() { 2 } else { 1 }),
+            true,
+        ) {
             return;
         }
 
-        sep.print(&mut self.s, &mut self.cursor);
+        self.print_sep_unhandled(sep);
+    }
+
+    fn print_sep_unhandled(&mut self, sep: Separator) {
+        let is_at_crlf = self.is_at_crlf();
+        sep.print(&mut self.s, &mut self.cursor, is_at_crlf);
     }
 
     fn print_ident(&mut self, ident: &ast::Ident) {
@@ -698,7 +732,7 @@ impl<'sess> State<'sess, '_> {
                         let hb = |this: &mut Self| {
                             this.hardbreak();
                             if pos.is_last {
-                                this.cursor.advance(1);
+                                this.cursor.next_line(this.is_at_crlf());
                             }
                         };
                         if line.is_empty() {
@@ -732,7 +766,7 @@ impl<'sess> State<'sess, '_> {
                         let hb = |this: &mut Self| {
                             this.hardbreak();
                             if pos.is_last {
-                                this.cursor.advance(1);
+                                this.cursor.next_line(this.is_at_crlf());
                             }
                         };
                         if line.is_empty() {
@@ -808,7 +842,7 @@ impl<'sess> State<'sess, '_> {
                 // Pre-requisite: ensure that blank links are printed at the beginning of new line.
                 if !self.last_token_is_break() && !self.is_bol_or_only_ind() {
                     config.hardbreak(&mut self.s);
-                    self.cursor.advance(1);
+                    self.cursor.next_line(self.is_at_crlf());
                 }
 
                 // We need to do at least one, possibly two hardbreaks.
@@ -820,10 +854,10 @@ impl<'sess> State<'sess, '_> {
                 };
                 if twice {
                     config.hardbreak(&mut self.s);
-                    self.cursor.advance(1);
+                    self.cursor.next_line(self.is_at_crlf());
                 }
                 config.hardbreak(&mut self.s);
-                self.cursor.advance(1);
+                self.cursor.next_line(self.is_at_crlf());
             }
         }
     }

crates/fmt/src/state/sol.rs

@@ -25,6 +25,11 @@ macro_rules! get_span {
 /// Language-specific pretty printing: Solidity.
 impl<'ast> State<'_, 'ast> {
     pub(crate) fn print_source_unit(&mut self, source_unit: &'ast ast::SourceUnit<'ast>) {
+        // Figure out if the cursor needs to check for CR (`\r`).
+        if let Some(item) = source_unit.items.first() {
+            self.check_crlf(item.span.to(source_unit.items.last().unwrap().span));
+        }
+
         let mut items = source_unit.items.iter().peekable();
         let mut is_first = true;
         while let Some(item) = items.next() {
@@ -150,7 +155,7 @@ impl<'ast> State<'_, 'ast> {
         self.print_comments(span.hi(), CommentConfig::default());
         self.print_trailing_comment(span.hi(), None);
         self.hardbreak_if_not_bol();
-        self.cursor.advance(1);
+        self.cursor.next_line(self.is_at_crlf());
     }
 
     fn print_pragma(&mut self, pragma: &'ast ast::PragmaDirective<'ast>) {
@@ -821,7 +826,7 @@ impl<'ast> State<'_, 'ast> {
                 self.end();
             } else if is_binary_expr(&init.kind) {
                 if !self.is_bol_or_only_ind() {
-                    Separator::Space.print(&mut self.s, &mut self.cursor);
+                    self.print_sep_unhandled(Separator::Space);
                 }
                 if matches!(ty.kind, ast::TypeKind::Elementary(..) | ast::TypeKind::Mapping(..)) {
                     self.s.offset(self.ind);
@@ -864,7 +869,7 @@ impl<'ast> State<'_, 'ast> {
                     }
                 } else {
                     if !self.is_bol_or_only_ind() {
-                        Separator::Space.print(&mut self.s, &mut self.cursor);
+                        self.print_sep_unhandled(Separator::Space);
                     }
                     if matches!(ty.kind, ast::TypeKind::Elementary(..) | ast::TypeKind::Mapping(..))
                     {
@@ -2149,7 +2154,7 @@ impl<'ast> State<'_, 'ast> {
 
     fn print_if_cond(&mut self, kw: &'static str, cond: &'ast ast::Expr<'ast>, pos_hi: BytePos) {
         self.print_word(kw);
-        Separator::Nbsp.print(&mut self.s, &mut self.cursor);
+        self.print_sep_unhandled(Separator::Nbsp);
         self.print_tuple(
             std::slice::from_ref(cond),
             cond.span.lo(),

crates/forge/tests/cli/fmt_integration.rs

@@ -25,5 +25,4 @@ fmt_test!(
     "e41f2b9b7ed677ca03ff7bd7221a4e2fdd55504f"
 );
 
-#[cfg(not(windows))]
 fmt_test!(fmt_0x_settler, "0xProject", "0x-settler", "a388c8251ab6c4bedce1641b31027d7b1136daef");