From 1f15c2ca20b54a4879e0d76d321e1726307cac97 Mon Sep 17 00:00:00 2001
From: KtorZ <matthias.benkort@gmail.com>
Date: Thu, 22 Dec 2022 17:59:31 +0100
Subject: [PATCH] Sort import alphabetically when formatting.

  Fixes #211.
---
 crates/aiken-lang/src/format.rs       | 81 ++++++++++++++++-----------
 crates/aiken-lang/src/tests/format.rs | 30 +++++++++-
 2 files changed, 76 insertions(+), 35 deletions(-)

diff --git a/crates/aiken-lang/src/format.rs b/crates/aiken-lang/src/format.rs
index 2941fa0f..91b7d0f1 100644
--- a/crates/aiken-lang/src/format.rs
+++ b/crates/aiken-lang/src/format.rs
@@ -133,14 +133,14 @@ impl<'comments> Formatter<'comments> {
             let start = def.location().start;
 
             match def {
-                Definition::Use { .. } => {
+                Definition::Use(import) => {
                     has_imports = true;
 
                     let comments = self.pop_comments(start);
 
                     let def = self.definition(def);
 
-                    imports.push(commented(def, comments))
+                    imports.push((import, commented(def, comments)))
                 }
 
                 _other => {
@@ -155,7 +155,15 @@ impl<'comments> Formatter<'comments> {
             }
         }
 
-        let imports = join(imports.into_iter(), line());
+        let imports = join(
+            imports
+                .into_iter()
+                .sorted_by(|(import_a, _), (import_b, _)| {
+                    Ord::cmp(&import_a.module, &import_b.module)
+                })
+                .map(|(_, doc)| doc),
+            line(),
+        );
 
         let declarations = join(declarations.into_iter(), lines(2));
 
@@ -253,36 +261,7 @@ impl<'comments> Formatter<'comments> {
                 ..
             }) => self.data_type(*public, *opaque, name, parameters, constructors, location),
 
-            Definition::Use(Use {
-                module,
-                as_name,
-                unqualified,
-                ..
-            }) => "use "
-                .to_doc()
-                .append(Document::String(module.join("/")))
-                .append(if unqualified.is_empty() {
-                    nil()
-                } else {
-                    let unqualified = Itertools::intersperse(
-                        unqualified
-                            .iter()
-                            .sorted_by(|a, b| a.name.cmp(&b.name))
-                            .map(|e| e.to_doc()),
-                        flex_break(",", ", "),
-                    );
-                    let unqualified = break_("", "")
-                        .append(concat(unqualified))
-                        .nest(INDENT)
-                        .append(break_(",", ""))
-                        .group();
-                    ".{".to_doc().append(unqualified).append("}")
-                })
-                .append(if let Some(name) = as_name {
-                    docvec![" as ", name]
-                } else {
-                    nil()
-                }),
+            Definition::Use(import) => self.import(import),
 
             Definition::ModuleConstant(ModuleConstant {
                 public,
@@ -301,6 +280,42 @@ impl<'comments> Formatter<'comments> {
         }
     }
 
+    fn import<'a>(
+        &mut self,
+        Use {
+            module,
+            as_name,
+            unqualified,
+            ..
+        }: &'a Use<()>,
+    ) -> Document<'a> {
+        "use "
+            .to_doc()
+            .append(Document::String(module.join("/")))
+            .append(if unqualified.is_empty() {
+                nil()
+            } else {
+                let unqualified = Itertools::intersperse(
+                    unqualified
+                        .iter()
+                        .sorted_by(|a, b| a.name.cmp(&b.name))
+                        .map(|e| e.to_doc()),
+                    flex_break(",", ", "),
+                );
+                let unqualified = break_("", "")
+                    .append(concat(unqualified))
+                    .nest(INDENT)
+                    .append(break_(",", ""))
+                    .group();
+                ".{".to_doc().append(unqualified).append("}")
+            })
+            .append(if let Some(name) = as_name {
+                docvec![" as ", name]
+            } else {
+                nil()
+            })
+    }
+
     fn const_expr<'a, A, B>(&mut self, value: &'a Constant<A, B>) -> Document<'a> {
         match value {
             Constant::ByteArray { bytes, .. } => "#"
diff --git a/crates/aiken-lang/src/tests/format.rs b/crates/aiken-lang/src/tests/format.rs
index e2160ef3..822c64c0 100644
--- a/crates/aiken-lang/src/tests/format.rs
+++ b/crates/aiken-lang/src/tests/format.rs
@@ -5,14 +5,15 @@ use pretty_assertions::assert_eq;
 fn assert_fmt(src: &str, expected: &str) {
     let (module, extra) = parser::module(src, ModuleKind::Lib).unwrap();
     let mut out = String::new();
-    format::pretty(&mut out, module.clone(), extra.clone(), src);
+    format::pretty(&mut out, module, extra, src);
 
     // Output is what we expect
     assert_eq!(out, expected);
 
     // Formatting is idempotent
+    let (module2, extra2) = parser::module(&out, ModuleKind::Lib).unwrap();
     let mut out2 = String::new();
-    format::pretty(&mut out2, module, extra, &out);
+    format::pretty(&mut out2, module2, extra2, &out);
     assert_eq!(out, out2);
 }
 
@@ -171,3 +172,28 @@ fn test_format_nested_when() {
 
     assert_fmt(src, expected)
 }
+
+#[test]
+fn test_format_imports() {
+    let src = indoc! {r#"
+        use aiken/list
+        // foo
+        use aiken/bytearray
+        use aiken/transaction/certificate
+        // bar
+        use aiken/transaction
+        use aiken/transaction/value
+    "#};
+
+    let expected = indoc! {r#"
+        // foo
+        use aiken/bytearray
+        use aiken/list
+        // bar
+        use aiken/transaction
+        use aiken/transaction/certificate
+        use aiken/transaction/value
+    "#};
+
+    assert_fmt(src, expected)
+}