Merge pull request #2014 from topecongiro/fn-sig

Reduce number of arguments in rewrite_fn()

src/items.rs

@@ -10,11 +10,13 @@
 
 // Formatting top-level items - functions, structs, enums, traits, impls.
 
+use std::borrow::Cow;
 use std::cmp::min;
 
 use syntax::{abi, ast, ptr, symbol};
 use syntax::ast::ImplItem;
 use syntax::codemap::{BytePos, Span};
+use syntax::visit;
 
 use spanned::Spanned;
 use codemap::{LineRangeUtils, SpanUtils};
@@ -125,7 +127,7 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
 #[allow(dead_code)]
 struct Item<'a> {
     keyword: &'static str,
-    abi: String,
+    abi: Cow<'static, str>,
     vis: Option<&'a ast::Visibility>,
     body: Vec<BodyElement<'a>>,
     span: Span,
@@ -133,14 +135,9 @@ struct Item<'a> {
 
 impl<'a> Item<'a> {
     fn from_foreign_mod(fm: &'a ast::ForeignMod, span: Span, config: &Config) -> Item<'a> {
-        let abi = if fm.abi == abi::Abi::C && !config.force_explicit_abi() {
-            "extern".into()
-        } else {
-            format!("extern {}", fm.abi)
-        };
         Item {
             keyword: "",
-            abi: abi,
+            abi: format_abi(fm.abi, config.force_explicit_abi(), true),
             vis: None,
             body: fm.items
                 .iter()
@@ -159,10 +156,92 @@ enum BodyElement<'a> {
     ForeignItem(&'a ast::ForeignItem),
 }
 
+/// Represents a fn's signature.
+pub struct FnSig<'a> {
+    decl: &'a ast::FnDecl,
+    generics: &'a ast::Generics,
+    abi: abi::Abi,
+    constness: ast::Constness,
+    defaultness: ast::Defaultness,
+    unsafety: ast::Unsafety,
+    visibility: ast::Visibility,
+}
+
+impl<'a> FnSig<'a> {
+    pub fn new(
+        decl: &'a ast::FnDecl,
+        generics: &'a ast::Generics,
+        vis: ast::Visibility,
+    ) -> FnSig<'a> {
+        FnSig {
+            decl: decl,
+            generics: generics,
+            abi: abi::Abi::Rust,
+            constness: ast::Constness::NotConst,
+            defaultness: ast::Defaultness::Final,
+            unsafety: ast::Unsafety::Normal,
+            visibility: vis,
+        }
+    }
+
+    pub fn from_method_sig(method_sig: &'a ast::MethodSig) -> FnSig {
+        FnSig {
+            unsafety: method_sig.unsafety,
+            constness: method_sig.constness.node,
+            defaultness: ast::Defaultness::Final,
+            abi: method_sig.abi,
+            decl: &*method_sig.decl,
+            generics: &method_sig.generics,
+            visibility: ast::Visibility::Inherited,
+        }
+    }
+
+    pub fn from_fn_kind(
+        fn_kind: &'a visit::FnKind,
+        decl: &'a ast::FnDecl,
+        defualtness: ast::Defaultness,
+    ) -> FnSig<'a> {
+        match *fn_kind {
+            visit::FnKind::ItemFn(_, generics, unsafety, constness, abi, visibility, _) => FnSig {
+                decl: decl,
+                generics: generics,
+                abi: abi,
+                constness: constness.node,
+                defaultness: defualtness,
+                unsafety: unsafety,
+                visibility: visibility.clone(),
+            },
+            visit::FnKind::Method(_, ref method_sig, vis, _) => {
+                let mut fn_sig = FnSig::from_method_sig(method_sig);
+                fn_sig.defaultness = defualtness;
+                if let Some(vis) = vis {
+                    fn_sig.visibility = vis.clone();
+                }
+                fn_sig
+            }
+            _ => unreachable!(),
+        }
+    }
+
+    fn to_str(&self, context: &RewriteContext) -> String {
+        let mut result = String::with_capacity(128);
+        // Vis defaultness constness unsafety abi.
+        result.push_str(&*format_visibility(&self.visibility));
+        result.push_str(format_defaultness(self.defaultness));
+        result.push_str(format_constness(self.constness));
+        result.push_str(format_unsafety(self.unsafety));
+        result.push_str(&format_abi(
+            self.abi,
+            context.config.force_explicit_abi(),
+            false,
+        ));
+        result
+    }
+}
+
 impl<'a> FmtVisitor<'a> {
     fn format_item(&mut self, item: Item) {
         self.buffer.push_str(&item.abi);
-        self.buffer.push_str(" ");
 
         let snippet = self.snippet(item.span);
         let brace_pos = snippet.find_uncommented("{").unwrap();
@@ -216,37 +295,24 @@ pub fn rewrite_fn(
         &mut self,
         indent: Indent,
         ident: ast::Ident,
-        fd: &ast::FnDecl,
+        fn_sig: &FnSig,
-        generics: &ast::Generics,
-        unsafety: ast::Unsafety,
-        constness: ast::Constness,
-        defaultness: ast::Defaultness,
-        abi: abi::Abi,
-        vis: &ast::Visibility,
         span: Span,
         block: &ast::Block,
     ) -> Option<String> {
         let context = self.get_context();
 
-        let block_snippet = self.snippet(mk_sp(block.span.lo(), block.span.hi()));
+        let has_body =
-        let has_body = !block_snippet[1..block_snippet.len() - 1].trim().is_empty()
+            !is_empty_block(block, self.codemap) || !context.config.fn_empty_single_line();
-            || !context.config.fn_empty_single_line();
+        let mut newline_brace =
-        let mut newline_brace = newline_for_brace(self.config, &generics.where_clause, has_body);
+            newline_for_brace(self.config, &fn_sig.generics.where_clause, has_body);
 
         let (mut result, force_newline_brace) = try_opt!(rewrite_fn_base(
             &context,
             indent,
             ident,
-            fd,
+            fn_sig,
-            generics,
-            unsafety,
-            constness,
-            defaultness,
-            abi,
-            vis,
             span,
             newline_brace,
-            has_body,
             true,
         ));
 
@@ -289,17 +355,10 @@ pub fn rewrite_required_fn(
             &context,
             indent,
             ident,
-            &sig.decl,
+            &FnSig::from_method_sig(sig),
-            &sig.generics,
-            sig.unsafety,
-            sig.constness.node,
-            ast::Defaultness::Final,
-            sig.abi,
-            &ast::Visibility::Inherited,
             span,
             false,
             false,
-            false,
         ));
 
         // Re-attach semicolon
@@ -1696,38 +1755,24 @@ fn rewrite_fn_base(
     context: &RewriteContext,
     indent: Indent,
     ident: ast::Ident,
-    fd: &ast::FnDecl,
+    fn_sig: &FnSig,
-    generics: &ast::Generics,
-    unsafety: ast::Unsafety,
-    constness: ast::Constness,
-    defaultness: ast::Defaultness,
-    abi: abi::Abi,
-    vis: &ast::Visibility,
     span: Span,
     newline_brace: bool,
     has_body: bool,
-    has_braces: bool,
 ) -> Option<(String, bool)> {
     let mut force_new_line_for_brace = false;
 
-    let where_clause = &generics.where_clause;
+    let where_clause = &fn_sig.generics.where_clause;
 
     let mut result = String::with_capacity(1024);
-    // Vis defaultness constness unsafety abi.
+    result.push_str(&fn_sig.to_str(context));
-    result.push_str(&*format_visibility(vis));
-    result.push_str(format_defaultness(defaultness));
-    result.push_str(format_constness(constness));
-    result.push_str(format_unsafety(unsafety));
-    if abi != abi::Abi::Rust {
-        result.push_str(&format_abi(abi, context.config.force_explicit_abi()));
-    }
 
     // fn foo
     result.push_str("fn ");
     result.push_str(&ident.to_string());
 
     // Generics.
-    let overhead = if has_braces && !newline_brace {
+    let overhead = if has_body && !newline_brace {
         // 4 = `() {`
         4
     } else {
@@ -1741,8 +1786,9 @@ fn rewrite_fn_base(
         indent: indent,
         offset: used_width,
     };
+    let fd = fn_sig.decl;
     let g_span = mk_sp(span.lo(), fd.output.span().lo());
-    let generics_str = try_opt!(rewrite_generics(context, generics, shape, g_span));
+    let generics_str = try_opt!(rewrite_generics(context, fn_sig.generics, shape, g_span));
     result.push_str(&generics_str);
 
     let snuggle_angle_bracket = generics_str
@@ -1767,7 +1813,7 @@ fn rewrite_fn_base(
         indent,
         ret_str_len,
         newline_brace,
-        has_braces,
+        has_body,
         multi_line_ret_str,
     ));
 
@@ -1804,7 +1850,8 @@ fn rewrite_fn_base(
     }
 
     // A conservative estimation, to goal is to be over all parens in generics
-    let args_start = generics
+    let args_start = fn_sig
+        .generics
         .ty_params
         .last()
         .map_or(span.lo(), |tp| end_typaram(tp));
@@ -1987,7 +2034,7 @@ fn rewrite_fn_base(
         }
     }
 
-    let option = WhereClauseOption::new(!has_braces, put_args_in_block && ret_str.is_empty());
+    let option = WhereClauseOption::new(!has_body, put_args_in_block && ret_str.is_empty());
     let where_clause_str = try_opt!(rewrite_where_clause(
         context,
         where_clause,
@@ -2757,19 +2804,10 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
                     context,
                     shape.indent,
                     self.ident,
-                    fn_decl,
+                    &FnSig::new(fn_decl, generics, self.vis.clone()),
-                    generics,
-                    ast::Unsafety::Normal,
-                    ast::Constness::NotConst,
-                    ast::Defaultness::Final,
-                    // These are not actually rust functions,
-                    // but we format them as such.
-                    abi::Abi::Rust,
-                    &self.vis,
                     span,
                     false,
                     false,
-                    false,
                 ).map(|(s, _)| format!("{};", s))
             }
             ast::ForeignItemKind::Static(ref ty, is_mutable) => {

src/types.rs

@@ -11,7 +11,6 @@
 use std::iter::ExactSizeIterator;
 use std::ops::Deref;
 
-use syntax::abi;
 use syntax::ast::{self, FunctionRetTy, Mutability};
 use syntax::codemap::{self, BytePos, Span};
 use syntax::print::pprust;
@@ -26,7 +25,7 @@
             SeparatorPlace, SeparatorTactic};
 use rewrite::{Rewrite, RewriteContext};
 use shape::Shape;
-use utils::{colon_spaces, extra_offset, format_mutability, last_line_width, mk_sp};
+use utils::{colon_spaces, extra_offset, format_abi, format_mutability, last_line_width, mk_sp};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum PathContext {
@@ -792,12 +791,11 @@ fn rewrite_bare_fn(
 
     result.push_str(::utils::format_unsafety(bare_fn.unsafety));
 
-    if bare_fn.abi != abi::Abi::Rust {
+    result.push_str(&format_abi(
-        result.push_str(&::utils::format_abi(
+        bare_fn.abi,
-            bare_fn.abi,
+        context.config.force_explicit_abi(),
-            context.config.force_explicit_abi(),
+        false,
-        ));
+    ));
-    }
 
     result.push_str("fn");
 

src/utils.rs

@@ -88,8 +88,10 @@ pub fn format_mutability(mutability: ast::Mutability) -> &'static str {
 }
 
 #[inline]
-pub fn format_abi(abi: abi::Abi, explicit_abi: bool) -> Cow<'static, str> {
+pub fn format_abi(abi: abi::Abi, explicit_abi: bool, is_mod: bool) -> Cow<'static, str> {
-    if abi == abi::Abi::C && !explicit_abi {
+    if abi == abi::Abi::Rust && !is_mod {
+        Cow::from("")
+    } else if abi == abi::Abi::C && !explicit_abi {
         Cow::from("extern ")
     } else {
         Cow::from(format!("extern {} ", abi))

src/visitor.rs

@@ -24,7 +24,7 @@
 use config::{BraceStyle, Config};
 use items::{format_impl, format_struct, format_struct_struct, format_trait,
             rewrite_associated_impl_type, rewrite_associated_type, rewrite_static,
-            rewrite_type_alias};
+            rewrite_type_alias, FnSig};
 use lists::{itemize_list, write_list, DefinitiveListTactic, ListFormatting, SeparatorPlace,
             SeparatorTactic};
 use macros::{rewrite_macro, MacroPosition};
@@ -237,34 +237,22 @@ fn visit_fn(
         let indent = self.block_indent;
         let block;
         let rewrite = match fk {
-            visit::FnKind::ItemFn(ident, generics, unsafety, constness, abi, vis, b) => {
+            visit::FnKind::ItemFn(ident, _, _, _, _, _, b) => {
                 block = b;
                 self.rewrite_fn(
                     indent,
                     ident,
-                    fd,
+                    &FnSig::from_fn_kind(&fk, fd, defaultness),
-                    generics,
-                    unsafety,
-                    constness.node,
-                    defaultness,
-                    abi,
-                    vis,
                     mk_sp(s.lo(), b.span.lo()),
                     b,
                 )
             }
-            visit::FnKind::Method(ident, sig, vis, b) => {
+            visit::FnKind::Method(ident, _, _, b) => {
                 block = b;
                 self.rewrite_fn(
                     indent,
                     ident,
-                    fd,
+                    &FnSig::from_fn_kind(&fk, fd, defaultness),
-                    &sig.generics,
-                    sig.unsafety,
-                    sig.constness.node,
-                    defaultness,
-                    sig.abi,
-                    vis.unwrap_or(&ast::Visibility::Inherited),
                     mk_sp(s.lo(), b.span.lo()),
                     b,
                 )