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

add inner use sort to formatter. #6467

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

133 changes: 99 additions & 34 deletions crates/cairo-lang-formatter/src/formatter_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::fmt;
use cairo_lang_filesystem::span::TextWidth;
use cairo_lang_syntax as syntax;
use cairo_lang_syntax::attribute::consts::FMT_SKIP_ATTR;
use cairo_lang_syntax::node::ast::PathSegment;
use cairo_lang_syntax::node::db::SyntaxGroup;
use cairo_lang_syntax::node::{SyntaxNode, Terminal, TypedSyntaxNode, ast};
use itertools::Itertools;
Expand Down Expand Up @@ -826,41 +827,11 @@ impl<'a> FormatterImpl<'a> {
// TODO(ilya): consider not copying here.
let mut children = self.db.get_children(syntax_node.clone()).to_vec();
let n_children = children.len();

if self.config.sort_module_level_items {
let mut start_idx = 0;
while start_idx < children.len() {
let kind = children[start_idx].as_sort_kind(self.db);
let mut end_idx = start_idx + 1;
// Find the end of the current section.
while end_idx < children.len() {
if kind != children[end_idx].as_sort_kind(self.db) {
break;
}
end_idx += 1;
}
// Sort within this section if it's `Module` or `UseItem`.
match kind {
SortKind::Module => {
children[start_idx..end_idx].sort_by_key(|node| {
ast::ItemModule::from_syntax_node(self.db, node.clone())
.name(self.db)
.text(self.db)
});
}
SortKind::UseItem => {
children[start_idx..end_idx].sort_by_key(|node| {
ast::ItemUse::from_syntax_node(self.db, node.clone())
.use_path(self.db)
.as_syntax_node()
.get_text_without_trivia(self.db)
});
}
SortKind::Immovable => {}
}

// Move past the sorted section.
start_idx = end_idx;
if let SyntaxKind::UsePathList = syntax_node.kind(self.db) {
self.sort_inner_use_path(&mut children);
} else {
self.sort_items_sections(&mut children);
}
}
for (i, child) in children.iter().enumerate() {
Expand All @@ -878,6 +849,100 @@ impl<'a> FormatterImpl<'a> {
self.empty_lines_allowance = allowed_empty_between;
}
}

/// Sorting function for `UsePathMulti` children.
fn sort_inner_use_path(&self, children: &mut Vec<SyntaxNode>) {
// Filter and collect only UsePathLeaf and UsePathSingle, while excluding TokenComma.
let mut sorted_leaf_and_single: Vec<_> = children
.iter()
.filter(|node| {
matches!(node.kind(self.db), SyntaxKind::UsePathLeaf | SyntaxKind::UsePathSingle)
})
.cloned()
.collect();

// Sort the filtered nodes by their ident (text).
sorted_leaf_and_single.sort_by_key(|node| {
match node.kind(self.db) {
SyntaxKind::UsePathLeaf | SyntaxKind::UsePathSingle => {
let path_segment =
ast::UsePathLeaf::from_syntax_node(self.db, node.clone()).ident(self.db);
match path_segment {
PathSegment::Simple(simple_segment) => {
simple_segment.as_syntax_node().get_text_without_trivia(self.db)
}
PathSegment::WithGenericArgs(_) => {
// Handle the case with generic arguments if needed (e.g., extract base
// name).
"".to_string()
}
}
}
_ => "".to_string(), // Default case (shouldn't happen).
}
});

// Filter and collect the other node types (specifically commas).
let other_types: Vec<_> = children
.iter()
.filter(|node| {
!matches!(node.kind(self.db), SyntaxKind::UsePathLeaf | SyntaxKind::UsePathSingle)
})
.cloned()
.collect();

// Combine the sorted UsePathLeaf/UsePathSingle nodes with the other node types.
*children = sorted_leaf_and_single
.clone()
.into_iter()
.zip(other_types)
.flat_map(|(a, b)| vec![a, b])
.collect();

// Push the last remaining element from the sorted_leaf_and_single (we always have one less
// comma than the number of UsePathLeaf/UsePathSingle).
if let Some(last) = sorted_leaf_and_single.last() {
children.push(last.clone());
}
}

/// Sorting function for module-level items.
fn sort_items_sections(&self, children: &mut [SyntaxNode]) {
let mut start_idx = 0;
while start_idx < children.len() {
let kind = children[start_idx].as_sort_kind(self.db);
let mut end_idx = start_idx + 1;
// Find the end of the current section.
while end_idx < children.len() {
if kind != children[end_idx].as_sort_kind(self.db) {
break;
}
end_idx += 1;
}
// Sort within this section if it's `Module` or `UseItem`.
match kind {
SortKind::Module => {
children[start_idx..end_idx].sort_by_key(|node| {
ast::ItemModule::from_syntax_node(self.db, node.clone())
.name(self.db)
.text(self.db)
});
}
SortKind::UseItem => {
children[start_idx..end_idx].sort_by_key(|node| {
ast::ItemUse::from_syntax_node(self.db, node.clone())
.use_path(self.db)
.as_syntax_node()
.get_text_without_trivia(self.db)
});
}
SortKind::Immovable => {}
}

// Move past the sorted section.
start_idx = end_idx;
}
}
/// Formats a terminal node and appends the formatted string to the result.
fn format_terminal(&mut self, syntax_node: &SyntaxNode) {
// TODO(spapini): Introduce a Terminal and a Token enum in ast.rs to make this cleaner.
Expand Down
5 changes: 5 additions & 0 deletions crates/cairo-lang-formatter/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ impl Upcast<dyn FilesGroup> for DatabaseImpl {
"test_data/expected_results/sorted_mod_use.cairo",
true
)]
#[test_case(
"test_data/cairo_files/sort_inner_use.cairo",
"test_data/expected_results/sort_inner_use.cairo",
true
)]
fn format_and_compare_file(unformatted_filename: &str, expected_filename: &str, use_sorting: bool) {
let db_val = SimpleParserDatabase::default();
let db = &db_val;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
use std::collections::HashMap;
use crate::utils::{a, b, d, c};
use b::{d, c, b, a};
use a::{d, b, a, c};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
use a::{a, b, c, d};
use b::{a, b, c, d};
use crate::utils::{a, b, c, d};
use std::collections::HashMap;
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod SRC5 {
//! Header comment, should not be moved by the formatter.
/// Doc comment, should be moved by the formatter.
use openzeppelin::introspection::interface;
use openzeppelin::introspection::{interface, AB};
use openzeppelin::introspection::{AB, interface};

#[storage]
struct Storage {
Expand Down