Sitelet https://github.com/starkware-libs/cairo/commit/f7a6366d6cc95250dc97926ccf2cade9ebb5c57f
Skip to content

Commit f7a6366

Browse files
oriziorizi
committed
bugfix(formatter): Made formatter merge in use more consistent.
1 parent 186de1b commit f7a6366

3 files changed

Lines changed: 45 additions & 10 deletions

File tree

‎crates/cairo-lang-formatter/src/formatter_impl.rs‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,26 @@ impl LineBuilder {
408408
pub fn push_space(&mut self) {
409409
self.push_child(LineComponent::Space);
410410
}
411+
/// Whether a separating space already effectively precedes a comment that is about to be
412+
/// appended, so that no extra space should be inserted before it. This is the case when:
413+
/// * a not-yet-flushed break line point already renders as a space when unbroken (inserting a
414+
/// space would produce a double space); or
415+
/// * the previous component is an explicit space; or
416+
/// * the previous component is itself a comment - inserting a space would prevent consecutive
417+
/// comment lines from being aggregated and reflowed together.
418+
fn comment_already_separated(&mut self) -> bool {
419+
let active_builder = self.get_active_builder_mut();
420+
if matches!(
421+
active_builder.pending_break_line_points.last(),
422+
Some(LineComponent::BreakLinePoint(properties)) if properties.space_if_not_broken
423+
) {
424+
return true;
425+
}
426+
matches!(
427+
active_builder.children.last(),
428+
Some(LineComponent::Space | LineComponent::Comment { .. })
429+
)
430+
}
411431
/// Appends a user-inserted empty line to the line.
412432
pub fn push_empty_line_break_line_point(&mut self) {
413433
self.push_child(LineComponent::BreakLinePoint(BreakLinePointProperties::new_empty_line()));
@@ -945,6 +965,10 @@ pub struct FormatterImpl<'a> {
945965
is_current_line_whitespaces: bool,
946966
/// Indicates whether the last element handled was a comment.
947967
is_last_element_comment: bool,
968+
/// Indicates whether any token has been emitted yet. Used to avoid emitting a separating space
969+
/// before a comment that opens the file (it has nothing to be separated from, and the leading
970+
/// space would not be trimmed when there is no surrounding break line point).
971+
has_emitted_token: bool,
948972
}
949973
impl<'a> FormatterImpl<'a> {
950974
pub fn new(db: &'a dyn Database, config: FormatterConfig) -> Self {
@@ -955,6 +979,7 @@ impl<'a> FormatterImpl<'a> {
955979
empty_lines_allowance: 0,
956980
is_current_line_whitespaces: true,
957981
is_last_element_comment: false,
982+
has_emitted_token: false,
958983
}
959984
}
960985
/// Gets a root of a syntax tree and returns the formatted string of the code it represents.
@@ -1227,7 +1252,17 @@ impl<'a> FormatterImpl<'a> {
12271252
ast::Trivium::SingleLineComment(_)
12281253
| ast::Trivium::SingleLineDocComment(_)
12291254
| ast::Trivium::SingleLineInnerComment(_) => {
1230-
if !is_leading {
1255+
// Emit a separating space before the comment unless one is already present. The
1256+
// line-breaking pass drops spaces at the start of a line, so this is a no-op
1257+
// for a comment that stays on its own line, but produces a
1258+
// consistent ` //` spacing when a (leading) comment is
1259+
// collapsed onto the previous line. Without this,
1260+
// such a collapsed comment is emitted with no space on the first formatting
1261+
// pass and a space on the second, making `cairo-format`
1262+
// non-idempotent.
1263+
if self.has_emitted_token
1264+
&& !self.line_state.line_buffer.comment_already_separated()
1265+
{
12311266
self.line_state.line_buffer.push_space();
12321267
}
12331268
self.line_state
@@ -1264,6 +1299,7 @@ impl<'a> FormatterImpl<'a> {
12641299
self.line_state.prevent_next_space = syntax_node.force_no_space_after(self.db);
12651300
if syntax_node.kind(self.db) != SyntaxKind::TokenWhitespace {
12661301
self.is_current_line_whitespaces = false;
1302+
self.has_emitted_token = true;
12671303
}
12681304
let node_break_points = syntax_node.get_wrapping_break_line_point_properties(self.db);
12691305
self.append_break_line_point(node_break_points.leading());
@@ -1284,13 +1320,8 @@ fn compare_use_paths<'a>(a: &UsePath<'a>, b: &UsePath<'a>, db: &dyn Database) ->
12841320
match (a, b) {
12851321
// Case for multi vs multi.
12861322
(UsePath::Multi(a_multi), UsePath::Multi(b_multi)) => {
1287-
let empty_string = "".into();
12881323
let get_min_child = |multi: &ast::UsePathMulti<'a>| {
1289-
multi.use_paths(db).elements(db).min_by_key(|child| match child {
1290-
UsePath::Leaf(leaf) => leaf.extract_ident(db),
1291-
UsePath::Single(single) => single.extract_ident(db),
1292-
_ => &empty_string,
1293-
})
1324+
multi.use_paths(db).elements(db).min_by(|a, b| compare_use_paths(a, b, db))
12941325
};
12951326
match (get_min_child(a_multi), get_min_child(b_multi)) {
12961327
(Some(a_min), Some(b_min)) => compare_use_paths(&a_min, &b_min, db),

‎crates/cairo-lang-formatter/test_data/cairo_files/sort_inner_use.cairo‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,5 @@ use a::{{d}, e, ab, c};
1818

1919
use a::{a, a::{c, {a}}, a::{b}, a::{{a},b}};
2020
use a::{c, d::{a, *}, r, t::{*, c::a}};
21+
22+
use x::{{self, m}, {a, m}};

‎crates/cairo-lang-formatter/test_data/expected_results/sort_inner_use.cairo‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,19 @@ use a::{a, b, c, d};
44

55
use a::{a, b as aee, b as bee, c as cee, d};
66

7-
use a::{a, a::{b}, a::{c, {a}}, a::{b, {a}}};
7+
use a::{a, a::{b}, a::{b, {a}}, a::{c, {a}}};
88
use a::{a as ab, a as bc};
9+
10+
use a::{ab, c, e, {d}};
911
use a::{b, d};
1012
use a::{b, d};
1113

1214
use a::{c, d};
1315
use a::{c, d::{*, a}, r, t::{*, c::a}};
14-
15-
use a::{ab, c, e, {d}};
1616
use aba;
1717
use b::{a, b, c, d};
1818
use c::{*, a, b, c, d};
1919
use std::collections::HashMap;
20+
21+
use x::{{self, m}, {a, m}};
2022
use crate::utils::{a, b, c, d};

0 commit comments

Comments
 (0)