Add new formatting rules (#30)

* Sort top-level items and items in impl blocks * Add a test to make sure comments are removed * Replace identifiers of generic parameters with placeholders --------- Co-authored-by: Remo Senekowitsch <[email protected]>
exercism · Nov 28, 2023 · 34a58c4 · 34a58c4
1 parent b4a7504
commit 34a58c4
Show file tree

Hide file tree

Showing 27 changed files with 288 additions and 39 deletions.
diff --git a/src/main.rs b/src/main.rs
@@ -37,7 +37,7 @@ fn main() {
         matches.get_one::<String>("input_path").unwrap()
     );
 
-    if let Err(error) = run(&path, &matches.get_one::<String>("output_path").unwrap()) {
+    if let Err(error) = run(&path, matches.get_one::<String>("output_path").unwrap()) {
         eprintln!("[error] {}", error);
         process::exit(1);
     }

diff --git a/src/replace_identifier.rs b/src/replace_identifier.rs
@@ -1,7 +1,8 @@
 use proc_macro2::{Ident, Span};
 use syn::{
-    ExprMethodCall, Field, ItemConst, ItemEnum, ItemStatic, ItemStruct, ItemTrait, ItemType,
-    ItemUnion, Member, PatIdent, PathSegment, Signature, Variant,
+    ConstParam, ExprMethodCall, Field, ItemConst, ItemEnum, ItemStatic, ItemStruct, ItemTrait,
+    ItemType, ItemUnion, LifetimeParam, Member, PatIdent, PathSegment, Signature, TypeParam,
+    Variant,
 };
 
 // encapsulates fetching and updating a node's identifier
@@ -17,6 +18,36 @@ pub trait ReplacePossibleIdentifier {
     fn set_ident(&mut self, ident: String);
 }
 
+impl ReplaceIdentifier for TypeParam {
+    fn ident_string(&self) -> String {
+        self.ident.to_string()
+    }
+
+    fn set_ident(&mut self, ident: String) {
+        self.ident = Ident::new(&ident, Span::call_site());
+    }
+}
+
+impl ReplaceIdentifier for ConstParam {
+    fn ident_string(&self) -> String {
+        self.ident.to_string()
+    }
+
+    fn set_ident(&mut self, ident: String) {
+        self.ident = Ident::new(&ident, Span::call_site());
+    }
+}
+
+impl ReplaceIdentifier for LifetimeParam {
+    fn ident_string(&self) -> String {
+        self.lifetime.ident.to_string()
+    }
+
+    fn set_ident(&mut self, ident: String) {
+        self.lifetime.ident = Ident::new(&ident, Span::call_site());
+    }
+}
+
 impl ReplaceIdentifier for PatIdent {
     fn ident_string(&self) -> String {
         self.ident.to_string()

diff --git a/src/visit_mut.rs b/src/visit_mut.rs
@@ -1,25 +1,87 @@
+use proc_macro2::Ident;
 use quote::quote;
 use syn::punctuated::Punctuated;
 use syn::visit_mut::VisitMut;
 use syn::{
-    Arm, Expr, ExprAssign, ExprBinary, ExprCall, ExprClosure, ExprField, ExprForLoop, ExprIf,
-    ExprLet, ExprLoop, ExprMatch, ExprMethodCall, ExprPath, ExprUnary, ExprWhile, Field, Fields,
-    FnArg, ItemConst, ItemEnum, ItemFn, ItemImpl, ItemStatic, ItemStruct, ItemTrait, ItemType,
-    ItemUnion, Macro, Member, Pat, PatIdent, PatTuple, PatType, Path, PathSegment, ReturnType,
-    Signature, Token, Type, Variant,
+    Arm, ConstParam, Expr, ExprAssign, ExprBinary, ExprCall, ExprClosure, ExprField, ExprForLoop,
+    ExprIf, ExprLet, ExprLoop, ExprMatch, ExprMethodCall, ExprPath, ExprUnary, ExprWhile, Field,
+    Fields, FnArg, ImplItem, Item, ItemConst, ItemEnum, ItemFn, ItemImpl, ItemStatic, ItemStruct,
+    ItemTrait, ItemType, ItemUnion, Macro, Member, Pat, PatIdent, PatTuple, PatType, Path,
+    PathSegment, ReturnType, Signature, Token, Type, TypeParam, Variant,
 };
 
 use crate::ident_visitor::IdentVisitor;
 
+fn get_ident(item: &Item) -> Option<syn::Ident> {
+    match item {
+        Item::Const(item_const) => Some(item_const.ident.clone()),
+        Item::Enum(item_enum) => Some(item_enum.ident.clone()),
+        Item::Fn(item_fn) => Some(item_fn.sig.ident.clone()),
+        Item::ExternCrate(item_extern_crate) => Some(item_extern_crate.ident.clone()),
+        Item::Impl(_) => None,
+        Item::ForeignMod(item_mod) => item_mod
+            .abi
+            .name
+            .clone()
+            .map(|name| Ident::new(&name.value(), name.span())),
+        Item::Macro(item_macro) => item_macro.ident.clone(),
+        Item::Static(item_static) => Some(item_static.ident.clone()),
+        Item::Struct(item_struct) => Some(item_struct.ident.clone()),
+        Item::Trait(item_trait) => Some(item_trait.ident.clone()),
+        Item::TraitAlias(item_trait_alias) => Some(item_trait_alias.ident.clone()),
+        Item::Type(item_type) => Some(item_type.ident.clone()),
+        Item::Union(item_union) => Some(item_union.ident.clone()),
+        Item::Use(_) => None,
+        Item::Verbatim(_) => None,
+        _ => None,
+    }
+}
+
+fn get_impl_item_ident(item: &ImplItem) -> Option<syn::Ident> {
+    match item {
+        ImplItem::Const(item_const) => Some(item_const.ident.clone()),
+        ImplItem::Fn(item_method) => Some(item_method.sig.ident.clone()),
+        ImplItem::Macro(_) => None,
+        ImplItem::Type(item_type) => Some(item_type.ident.clone()),
+        ImplItem::Verbatim(_) => None,
+        _ => None,
+    }
+}
+
 impl VisitMut for IdentVisitor {
+    fn visit_file_mut(&mut self, i: &mut syn::File) {
+        i.items.sort_by_cached_key(get_ident);
+        for it in &mut i.attrs {
+            self.visit_attribute_mut(it);
+        }
+        for it in &mut i.items {
+            self.visit_item_mut(it);
+        }
+    }
+
     fn visit_pat_ident_mut(&mut self, node: &mut PatIdent) {
         self.replace_identifier(node);
     }
 
+    fn visit_type_param_mut(&mut self, node: &mut TypeParam) {
+        self.replace_identifier(node);
+    }
+
+    fn visit_const_param_mut(&mut self, node: &mut ConstParam) {
+        self.replace_identifier(node);
+    }
+
+    fn visit_lifetime_param_mut(&mut self, i: &mut syn::LifetimeParam) {
+        self.replace_identifier(i);
+    }
+
     fn visit_item_struct_mut(&mut self, node: &mut ItemStruct) {
         // visit struct's identifier
         self.replace_identifier(node);
 
+        // visit generics
+        self.visit_generics_mut(&mut node.generics);
+
         // visit struct's fields
         for field in node.fields.iter_mut() {
             self.visit_field_mut(field);
@@ -57,10 +119,13 @@ impl VisitMut for IdentVisitor {
                 self.replace_identifier_if_mapped(segment);
             }
         }
+        self.visit_generics_mut(&mut node.generics);
 
         // visit the impl's type
         self.visit_type_mut(&mut node.self_ty);
 
+        node.items.sort_by_cached_key(get_impl_item_ident);
+
         // visit the impl's items
         for impl_item in node.items.iter_mut() {
             self.visit_impl_item_mut(impl_item);
@@ -176,6 +241,7 @@ impl VisitMut for IdentVisitor {
 
             for segment in segments.iter_mut() {
                 self.replace_identifier_if_mapped(segment);
+                self.visit_path_arguments_mut(&mut segment.arguments);
             }
 
             type_path.path.segments = segments;

diff --git a/tests/const_names/expected_mapping.json b/tests/const_names/expected_mapping.json
@@ -1,4 +1,4 @@
 {
-  "PLACEHOLDER_1": "MAX",
-  "PLACEHOLDER_2": "KEYWORDS"
+  "PLACEHOLDER_1": "KEYWORDS",
+  "PLACEHOLDER_2": "MAX"
 }
diff --git a/tests/const_names/expected_representation.txt b/tests/const_names/expected_representation.txt
@@ -1,2 +1,2 @@
-const PLACEHOLDER_1: u16 = 65535;
-const PLACEHOLDER_2: &'static [&'static str] = &["main"];
+const PLACEHOLDER_1: &'static [&'static str] = &["main"];
+const PLACEHOLDER_2: u16 = 65535;
diff --git a/tests/fn_names/expected_mapping.json b/tests/fn_names/expected_mapping.json
@@ -1,4 +1,4 @@
 {
-  "PLACEHOLDER_1": "process",
-  "PLACEHOLDER_2": "initialize"
+  "PLACEHOLDER_1": "initialize",
+  "PLACEHOLDER_2": "process"
 }
diff --git a/tests/fn_names/expected_representation.txt b/tests/fn_names/expected_representation.txt
@@ -1,2 +1,2 @@
-fn PLACEHOLDER_1(&mut self) {}
-pub unsafe fn PLACEHOLDER_2(&self) -> bool {}
+pub unsafe fn PLACEHOLDER_1(&self) -> bool {}
+fn PLACEHOLDER_2(&mut self) {}
diff --git a/tests/generics/expected_mapping.json b/tests/generics/expected_mapping.json
@@ -0,0 +1,10 @@
+{
+  "PLACEHOLDER_1": "A",
+  "PLACEHOLDER_2": "B",
+  "PLACEHOLDER_3": "superb",
+  "PLACEHOLDER_4": "var_c",
+  "PLACEHOLDER_5": "One",
+  "PLACEHOLDER_6": "Test",
+  "PLACEHOLDER_7": "a",
+  "PLACEHOLDER_8": "b"
+}
diff --git a/tests/generics/expected_representation.json b/tests/generics/expected_representation.json
@@ -0,0 +1,3 @@
+{
+  "version": 1
+}
diff --git a/tests/generics/expected_representation.txt b/tests/generics/expected_representation.txt
@@ -0,0 +1,10 @@
+impl<PLACEHOLDER_1, PLACEHOLDER_2> Test<PLACEHOLDER_1, PLACEHOLDER_2> {
+    fn PLACEHOLDER_3() {
+        let PLACEHOLDER_4 = 3;
+    }
+}
+struct PLACEHOLDER_5<PLACEHOLDER_1>(PLACEHOLDER_1);
+struct PLACEHOLDER_6<PLACEHOLDER_1, PLACEHOLDER_2> {
+    PLACEHOLDER_7: PLACEHOLDER_1,
+    PLACEHOLDER_8: PLACEHOLDER_2,
+}
diff --git a/tests/generics/src/lib.rs b/tests/generics/src/lib.rs
@@ -0,0 +1,10 @@
+struct One<A>(A);
+struct Test<A, B> {
+    a: A,
+    b: B
+}
+impl<A, B> Test<A, B> {
+    fn superb() {
+        let var_c = 3;
+    }
+}
diff --git a/tests/ignore_comments/expected_mapping.json b/tests/ignore_comments/expected_mapping.json
@@ -0,0 +1,6 @@
+{
+  "PLACEHOLDER_1": "a",
+  "PLACEHOLDER_2": "A",
+  "PLACEHOLDER_3": "B",
+  "PLACEHOLDER_4": "C"
+}
diff --git a/tests/ignore_comments/expected_representation.json b/tests/ignore_comments/expected_representation.json
@@ -0,0 +1,3 @@
+{
+  "version": 1
+}
diff --git a/tests/ignore_comments/expected_representation.txt b/tests/ignore_comments/expected_representation.txt
@@ -0,0 +1,17 @@
+impl C for A {
+    fn PLACEHOLDER_1(&self) {
+        let PLACEHOLDER_1 = 10;
+    }
+}
+struct PLACEHOLDER_2 {
+    PLACEHOLDER_1: i32,
+}
+enum PLACEHOLDER_3 {
+    PLACEHOLDER_2,
+}
+trait PLACEHOLDER_4 {
+    fn PLACEHOLDER_1(&self);
+}
+fn main() {
+    let PLACEHOLDER_1 = 10;
+}
diff --git a/tests/ignore_comments/src/lib.rs b/tests/ignore_comments/src/lib.rs
@@ -0,0 +1,32 @@
+// Superb function
+fn main() {
+    // Superb variable
+    let a = 10;
+}
+
+// Superb struct
+struct A {
+    // Superb field
+    a: i32,
+}
+
+// Superb enum
+enum B {
+    // Superb variant
+    A,
+}
+
+// Superb trait
+trait C {
+    // Superb method
+    fn a(&self);
+}
+
+// Superb impl
+impl C for A {
+    // Superb method
+    fn a(&self) {
+        // Superb expression
+        let a = 10;
+    }
+}
diff --git a/tests/impl_blocks/expected_mapping.json b/tests/impl_blocks/expected_mapping.json
@@ -1,7 +1,7 @@
 {
-  "PLACEHOLDER_1": "Example",
+  "PLACEHOLDER_1": "answer",
   "PLACEHOLDER_2": "number",
   "PLACEHOLDER_3": "boo",
-  "PLACEHOLDER_4": "answer",
-  "PLACEHOLDER_5": "get_number"
+  "PLACEHOLDER_4": "get_number",
+  "PLACEHOLDER_5": "Example"
 }
diff --git a/tests/impl_blocks/expected_representation.txt b/tests/impl_blocks/expected_representation.txt
@@ -1,14 +1,14 @@
-struct PLACEHOLDER_1 {
-    PLACEHOLDER_2: i32,
-}
-impl PLACEHOLDER_1 {
+impl Example {
+    fn PLACEHOLDER_1(&mut self) {
+        self.PLACEHOLDER_2 += 42;
+    }
     fn PLACEHOLDER_3() {
         println!("boo! Example::boo() was called!");
     }
-    fn PLACEHOLDER_4(&mut self) {
-        self.PLACEHOLDER_2 += 42;
-    }
-    fn PLACEHOLDER_5(&self) -> i32 {
+    fn PLACEHOLDER_4(&self) -> i32 {
         self.PLACEHOLDER_2
     }
 }
+struct PLACEHOLDER_5 {
+    PLACEHOLDER_2: i32,
+}
diff --git a/tests/mod.rs b/tests/mod.rs
@@ -13,10 +13,11 @@ macro_rules! test_cases {
                 let expected = include_str!(concat!(stringify!($name), "/expected_representation.txt"));
 
                 let mut input: syn::File = syn::parse_str(input)?;
-                let _ = replace(&mut input);
+                let mapping = replace(&mut input);
 
                 if OVERWRITE_EXPECTED_OUTPUT {
-                    std::fs::write(concat!("tests", stringify!($name), "expected_representation.txt"), prettyplease::unparse(&input))?;
+                    std::fs::write(concat!("tests/", stringify!($name), "/expected_representation.txt"), prettyplease::unparse(&input))?;
+                    std::fs::write(concat!("tests/", stringify!($name), "/expected_mapping.json"), serde_json::to_string_pretty(&mapping).unwrap())?;
                 } else {
                     assert_eq!(prettyplease::unparse(&input), expected);
                 }
@@ -37,8 +38,10 @@ test_cases!(
     fn_calls
     fn_names
     for_loops
+    generics
     if_expressions
     if_let_expressions
+    ignore_comments
     ignore_doc_comments
     impl_blocks
     leap_year
@@ -49,6 +52,7 @@ test_cases!(
     match_expressions
     method_calls
     replace_same_identifier
+    sort_by_ascii_file_items
     static_names
     struct_fields
     struct_names

diff --git a/tests/sort_by_ascii_file_items/expected_mapping.json b/tests/sort_by_ascii_file_items/expected_mapping.json
@@ -0,0 +1,13 @@
+{
+  "PLACEHOLDER_1": "amazing",
+  "PLACEHOLDER_10": "b",
+  "PLACEHOLDER_11": "var_b",
+  "PLACEHOLDER_2": "var_d",
+  "PLACEHOLDER_3": "beautiful",
+  "PLACEHOLDER_4": "var_e",
+  "PLACEHOLDER_5": "superb",
+  "PLACEHOLDER_6": "var_c",
+  "PLACEHOLDER_7": "Test",
+  "PLACEHOLDER_8": "a",
+  "PLACEHOLDER_9": "var_a"
+}
diff --git a/tests/sort_by_ascii_file_items/expected_representation.json b/tests/sort_by_ascii_file_items/expected_representation.json
@@ -0,0 +1,3 @@
+{
+  "version": 1
+}