refactor: cleanup duplicative Impl handling code

src/items.rs

@@ -579,48 +579,16 @@ impl<'a> FmtVisitor<'a> {
 
     fn visit_impl_items(&mut self, items: &[ptr::P<ast::AssocItem>]) {
         if self.get_context().config.reorder_impl_items() {
-            // Create visitor for each items, then reorder them.
-            let mut buffer = vec![];
-            for item in items {
-                self.visit_impl_item(item);
-                buffer.push((self.buffer.clone(), item.clone()));
-                self.buffer.clear();
-            }
-
-            fn is_type(ty: &Option<rustc_ast::ptr::P<ast::Ty>>) -> bool {
-                if let Some(lty) = ty {
-                    if let ast::TyKind::ImplTrait(..) = lty.kind {
-                        return false;
-                    }
-                }
-                true
-            }
-
-            fn is_opaque(ty: &Option<rustc_ast::ptr::P<ast::Ty>>) -> bool {
-                !is_type(ty)
-            }
-
-            fn both_type(
-                a: &Option<rustc_ast::ptr::P<ast::Ty>>,
-                b: &Option<rustc_ast::ptr::P<ast::Ty>>,
-            ) -> bool {
-                is_type(a) && is_type(b)
-            }
-
-            fn both_opaque(
-                a: &Option<rustc_ast::ptr::P<ast::Ty>>,
-                b: &Option<rustc_ast::ptr::P<ast::Ty>>,
-            ) -> bool {
-                is_opaque(a) && is_opaque(b)
-            }
-
-            // In rustc-ap-v638 the `OpaqueTy` AssocItemKind variant was removed but
-            // we still need to differentiate to maintain sorting order.
-
-            // type -> opaque -> const -> macro -> method
+            type TyOpt = Option<ptr::P<ast::Ty>>;
             use crate::ast::AssocItemKind::*;
-            fn need_empty_line(a: &ast::AssocItemKind, b: &ast::AssocItemKind) -> bool {
-                match (a, b) {
+            let is_type = |ty: &TyOpt| {
+                ty.as_ref()
+                    .map_or(true, |t| !matches!(t.kind, ast::TyKind::ImplTrait(..)))
+            };
+            let is_opaque = |ty: &TyOpt| !is_type(ty);
+            let both_type = |left: &TyOpt, right: &TyOpt| is_type(left) && is_type(right);
+            let both_opaque = |left: &TyOpt, right: &TyOpt| is_opaque(left) && is_opaque(right);
+            let need_empty_line = |a: &ast::AssocItemKind, b: &ast::AssocItemKind| match (a, b) {
                 (TyAlias(lty), TyAlias(rty))
                     if both_type(&lty.ty, &rty.ty) || both_opaque(&lty.ty, &rty.ty) =>
                 {
@@ -628,7 +596,14 @@ impl<'a> FmtVisitor<'a> {
                 }
                 (Const(..), Const(..)) => false,
                 _ => true,
-                }
+            };
+
+            // Create visitor for each items, then reorder them.
+            let mut buffer = vec![];
+            for item in items {
+                self.visit_impl_item(item);
+                buffer.push((self.buffer.clone(), item.clone()));
+                self.buffer.clear();
             }
 
             buffer.sort_by(|(_, a), (_, b)| match (&a.kind, &b.kind) {
@@ -676,17 +651,17 @@ impl<'a> FmtVisitor<'a> {
 pub(crate) fn format_impl(
     context: &RewriteContext<'_>,
     item: &ast::Item,
+    iimpl: &ast::Impl,
     offset: Indent,
 ) -> Option<String> {
-    if let ast::ItemKind::Impl(impl_kind) = &item.kind {
     let ast::Impl {
-            ref generics,
-            ref self_ty,
-            ref items,
+        generics,
+        self_ty,
+        items,
         ..
-        } = **impl_kind;
+    } = iimpl;
     let mut result = String::with_capacity(128);
-        let ref_and_type = format_impl_ref_and_type(context, item, offset)?;
+    let ref_and_type = format_impl_ref_and_type(context, item, iimpl, offset)?;
     let sep = offset.to_string_with_newline(context.config);
     result.push_str(&ref_and_type);
 
@@ -803,9 +778,6 @@ pub(crate) fn format_impl(
     result.push('}');
 
     Some(result)
-    } else {
-        unreachable!();
-    }
 }
 
 fn is_impl_single_line(
@@ -830,9 +802,9 @@ fn is_impl_single_line(
 fn format_impl_ref_and_type(
     context: &RewriteContext<'_>,
     item: &ast::Item,
+    iimpl: &ast::Impl,
     offset: Indent,
 ) -> Option<String> {
-    if let ast::ItemKind::Impl(impl_kind) = &item.kind {
     let ast::Impl {
         unsafety,
         polarity,
@@ -842,7 +814,7 @@ fn format_impl_ref_and_type(
         of_trait: ref trait_ref,
         ref self_ty,
         ..
-        } = **impl_kind;
+    } = *iimpl;
     let mut result = String::with_capacity(128);
 
     result.push_str(&format_visibility(context, &item.vis));
@@ -896,10 +868,8 @@ fn format_impl_ref_and_type(
     } else {
         0
     };
-        let used_space = last_line_width(&result)
-            + polarity_overhead
-            + trait_ref_overhead
-            + curly_brace_overhead;
+    let used_space =
+        last_line_width(&result) + polarity_overhead + trait_ref_overhead + curly_brace_overhead;
     // 1 = space before the type.
     let budget = context.budget(used_space + 1);
     if let Some(self_ty_str) = self_ty.rewrite(context, Shape::legacy(budget, offset)) {
@@ -932,9 +902,6 @@ fn format_impl_ref_and_type(
     };
     result.push_str(&*self_ty.rewrite(context, Shape::legacy(budget, type_offset))?);
     Some(result)
-    } else {
-        unreachable!();
-    }
 }
 
 fn rewrite_trait_ref(

src/visitor.rs

@@ -485,9 +485,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         if should_visit_node_again {
             match item.kind {
                 ast::ItemKind::Use(ref tree) => self.format_import(item, tree),
-                ast::ItemKind::Impl { .. } => {
+                ast::ItemKind::Impl(ref iimpl) => {
                     let block_indent = self.block_indent;
-                    let rw = self.with_context(|ctx| format_impl(ctx, item, block_indent));
+                    let rw = self.with_context(|ctx| format_impl(ctx, item, iimpl, block_indent));
                     self.push_rewrite(item.span, rw);
                 }
                 ast::ItemKind::Trait(..) => {