Merge pull request #159 from marcusklaas/explicit-self

Correctly format typed self in function arguments

src/items.rs

@@ -11,7 +11,8 @@
 // Formatting top-level items - functions, structs, enums, traits, impls.
 
 use {ReturnIndent, BraceStyle};
-use utils::{format_visibility, make_indent, contains_skip, span_after, end_typaram};
+use utils::{format_mutability, format_visibility, make_indent, contains_skip, span_after,
+            end_typaram};
 use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic};
 use comment::FindUncommented;
 use visitor::FmtVisitor;
@@ -230,46 +231,14 @@ impl<'a> FmtVisitor<'a> {
                     -> String {
         let mut arg_item_strs: Vec<_> = args.iter().map(|a| self.rewrite_fn_input(a)).collect();
         // Account for sugary self.
-        let mut min_args = 1;
+        // FIXME: the comment for the self argument is dropped. This is blocked
-        if let Some(explicit_self) = explicit_self {
+        // on rust issue #27522.
-            match explicit_self.node {
+        let min_args = explicit_self.and_then(|explicit_self| {
-                ast::ExplicitSelf_::SelfRegion(ref lt, ref m, _) => {
+                           rewrite_explicit_self(explicit_self, args)
-                    let lt_str = match lt {
+                       }).map(|self_str| {
-                        &Some(ref l) => format!("{} ", pprust::lifetime_to_string(l)),
+                           arg_item_strs[0] = self_str;
-                        &None => String::new(),
+                           2
-                    };
+                       }).unwrap_or(1);
-                    let mut_str = match m {
-                        &ast::Mutability::MutMutable => "mut ".to_owned(),
-                        &ast::Mutability::MutImmutable => String::new(),
-                    };
-                    arg_item_strs[0] = format!("&{}{}self", lt_str, mut_str);
-                    min_args = 2;
-                }
-                ast::ExplicitSelf_::SelfExplicit(ref ty, _) => {
-                    arg_item_strs[0] = format!("self: {}", pprust::ty_to_string(ty));
-                }
-                ast::ExplicitSelf_::SelfValue(_) => {
-                    assert!(args.len() >= 1, "&[ast::Arg] shouldn't be empty.");
-
-                    // this hacky solution caused by absence of `Mutability` in `SelfValue`.
-                    let mut_str = {
-                        if let ast::Pat_::PatIdent(ast::BindingMode::BindByValue(mutability), _, _)
-                                = args[0].pat.node {
-                            match mutability {
-                                ast::Mutability::MutMutable => "mut ",
-                                ast::Mutability::MutImmutable => "",
-                            }
-                        } else {
-                            panic!("there is a bug or change in structure of AST, aborting.");
-                        }
-                    };
-
-                    arg_item_strs[0] = format!("{}self", mut_str);
-                    min_args = 2;
-                }
-                _ => {}
-            }
-        }
 
         // Comments between args
         let mut arg_items = Vec::new();
@@ -802,6 +771,37 @@ impl<'a> FmtVisitor<'a> {
     }
 }
 
+fn rewrite_explicit_self(explicit_self: &ast::ExplicitSelf, args: &[ast::Arg]) -> Option<String> {
+    match explicit_self.node {
+        ast::ExplicitSelf_::SelfRegion(lt, m, _) => {
+            let mut_str = format_mutability(m);
+            match lt {
+                Some(ref l) => Some(format!("&{} {}self", pprust::lifetime_to_string(l), mut_str)),
+                None => Some(format!("&{}self", mut_str)),
+            }
+        }
+        ast::ExplicitSelf_::SelfExplicit(ref ty, _) => {
+            Some(format!("self: {}", pprust::ty_to_string(ty)))
+        }
+        ast::ExplicitSelf_::SelfValue(_) => {
+            assert!(args.len() >= 1, "&[ast::Arg] shouldn't be empty.");
+
+            // this hacky solution caused by absence of `Mutability` in `SelfValue`.
+            let mut_str = {
+                if let ast::Pat_::PatIdent(ast::BindingMode::BindByValue(mutability), _, _)
+                        = args[0].pat.node {
+                    format_mutability(mutability)
+                } else {
+                    panic!("there is a bug or change in structure of AST, aborting.");
+                }
+            };
+
+            Some(format!("{}self", mut_str))
+        }
+        _ => None
+    }
+}
+
 fn span_lo_for_arg(arg: &ast::Arg) -> BytePos {
     if is_named_arg(arg) {
         arg.pat.span.lo

src/utils.rs

@@ -64,6 +64,14 @@ pub fn format_visibility(vis: Visibility) -> &'static str {
     }
 }
 
+#[inline]
+pub fn format_mutability(mutability: ast::Mutability) -> &'static str {
+    match mutability {
+        ast::Mutability::MutMutable => "mut ",
+        ast::Mutability::MutImmutable => ""
+    }
+}
+
 fn is_skip(meta_item: &MetaItem) -> bool {
     match meta_item.node {
         MetaItem_::MetaWord(ref s) => *s == SKIP_ANNOTATION,

tests/target/trait.rs

@@ -22,3 +22,7 @@ trait Foo {
 pub trait WriteMessage {
     fn write_message(&mut self, &FrontendMessage) -> io::Result<()>;
 }
+
+trait Runnable {
+    fn handler(self: &Runnable);
+}