From 9cf8529e335995d54b178194d00172f7f7dd15d5 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 23 Nov 2015 12:00:22 +1300
Subject: [PATCH 1/5] Implement formatting for impls
Fixes #614
---
src/items.rs | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/types.rs | 16 ++++----
src/utils.rs | 7 ++++
src/visitor.rs | 14 +++----
4 files changed, 121 insertions(+), 16 deletions(-)
diff --git a/src/items.rs b/src/items.rs
index bbe51c73c40..b4bd89ebb91 100644
--- a/src/items.rs
+++ b/src/items.rs
@@ -12,7 +12,7 @@
use Indent;
use utils::{format_mutability, format_visibility, contains_skip, span_after, end_typaram,
- wrap_str, last_line_width, semicolon_for_expr};
+ wrap_str, last_line_width, semicolon_for_expr, format_unsafety, trim_newlines};
use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic,
DefinitiveListTactic, definitive_tactic, format_item_list};
use expr::{is_empty_block, is_simple_block_stmt, rewrite_assign_rhs};
@@ -435,6 +435,104 @@ impl<'a> FmtVisitor<'a> {
}
}
+pub fn format_impl(context: &RewriteContext, item: &ast::Item, offset: Indent) -> Option<String> {
+ if let ast::Item_::ItemImpl(unsafety,
+ polarity,
+ ref generics,
+ ref trait_ref,
+ ref self_ty,
+ ref items) = item.node {
+ let mut result = String::new();
+ result.push_str(format_visibility(item.vis));
+ result.push_str(format_unsafety(unsafety));
+ result.push_str("impl");
+
+ let lo = span_after(item.span, "impl", context.codemap);
+ let hi = match *trait_ref {
+ Some(ref tr) => tr.path.span.lo,
+ None => self_ty.span.lo,
+ };
+ let generics_str = try_opt!(rewrite_generics(context,
+ generics,
+ offset,
+ offset + result.len(),
+ mk_sp(lo, hi)));
+ result.push_str(&generics_str);
+
+ // FIXME might need to linebreak in the impl header, here would be a
+ // good place.
+ result.push(' ');
+ if polarity == ast::ImplPolarity::Negative {
+ result.push_str("!");
+ }
+ if let &Some(ref trait_ref) = trait_ref {
+ let budget = try_opt!(context.config.max_width.checked_sub(result.len()));
+ let indent = offset + result.len();
+ result.push_str(&*try_opt!(trait_ref.rewrite(context, budget, indent)));
+ result.push_str(" for ");
+ }
+
+ let budget = try_opt!(context.config.max_width.checked_sub(result.len()));
+ let indent = offset + result.len();
+ result.push_str(&*try_opt!(self_ty.rewrite(context, budget, indent)));
+
+ let where_clause_str = try_opt!(rewrite_where_clause(context,
+ &generics.where_clause,
+ context.config,
+ context.block_indent,
+ context.config.where_density,
+ "{",
+ None));
+ if !where_clause_str.contains('\n') &&
+ result.len() + where_clause_str.len() + offset.width() > context.config.max_width {
+ result.push('\n');
+ let width = context.block_indent.width() + context.config.tab_spaces - 1;
+ let where_indent = Indent::new(0, width);
+ result.push_str(&where_indent.to_string(context.config));
+ }
+ result.push_str(&where_clause_str);
+
+ match context.config.item_brace_style {
+ BraceStyle::AlwaysNextLine => result.push('\n'),
+ BraceStyle::PreferSameLine => result.push(' '),
+ BraceStyle::SameLineWhere => {
+ if where_clause_str.len() > 0 {
+ result.push('\n')
+ } else {
+ result.push(' ')
+ }
+ }
+ }
+ result.push('{');
+
+ if !items.is_empty() {
+ result.push('\n');
+ let indent_str = context.block_indent.to_string(context.config);
+ result.push_str(&indent_str);
+
+ let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config, None);
+ visitor.block_indent = context.block_indent.block_indent(context.config);
+
+ let snippet = context.snippet(item.span);
+ let open_pos = try_opt!(snippet.find_uncommented("{")) + 1;
+ visitor.last_pos = item.span.lo + BytePos(open_pos as u32);
+
+ for item in items {
+ visitor.visit_impl_item(&item);
+ }
+
+ result.push_str(trim_newlines(&visitor.buffer.to_string()));
+ result.push('\n');
+ result.push_str(&indent_str);
+ }
+ result.push('}');
+
+ Some(result)
+ } else {
+ unreachable!();
+ }
+}
+
pub fn format_struct(context: &RewriteContext,
item_name: &str,
ident: ast::Ident,
diff --git a/src/types.rs b/src/types.rs
index 807f0f17a43..4faa0a05f91 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -432,20 +432,22 @@ impl Rewrite for ast::PolyTraitRef {
// 6 is "for<> ".len()
let extra_offset = lifetime_str.len() + 6;
let max_path_width = try_opt!(width.checked_sub(extra_offset));
- let path_str = try_opt!(rewrite_path(context,
- false,
- None,
- &self.trait_ref.path,
- max_path_width,
- offset + extra_offset));
+ let path_str = try_opt!(self.trait_ref
+ .rewrite(context, max_path_width, offset + extra_offset));
Some(format!("for<{}> {}", lifetime_str, path_str))
} else {
- rewrite_path(context, false, None, &self.trait_ref.path, width, offset)
+ self.trait_ref.rewrite(context, width, offset)
}
}
}
+impl Rewrite for ast::TraitRef {
+ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
+ rewrite_path(context, false, None, &self.path, width, offset)
+ }
+}
+
impl Rewrite for ast::Ty {
fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
match self.node {
diff --git a/src/utils.rs b/src/utils.rs
index 6170b0c5ee0..195ca265c1f 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -142,6 +142,13 @@ pub fn semicolon_for_stmt(stmt: &ast::Stmt) -> bool {
}
}
+#[inline]
+pub fn trim_newlines(input: &str) -> &str {
+ let start = input.find(|c| c != '\n' && c != '\r').unwrap_or(0);
+ let end = input.rfind(|c| c != '\n' && c != '\r').unwrap_or(0) + 1;
+ &input[start..end]
+}
+
#[inline]
#[cfg(target_pointer_width="64")]
// Based on the trick layed out at
diff --git a/src/visitor.rs b/src/visitor.rs
index 6cbac8592dd..fcc017c98f2 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -21,7 +21,7 @@ use config::Config;
use rewrite::{Rewrite, RewriteContext};
use comment::rewrite_comment;
use macros::rewrite_macro;
-use items::rewrite_static;
+use items::{rewrite_static, format_impl};
pub struct FmtVisitor<'a> {
pub parse_session: &'a ParseSess,
@@ -200,14 +200,12 @@ impl<'a> FmtVisitor<'a> {
ast::Item_::ItemUse(ref vp) => {
self.format_import(item.vis, vp, item.span);
}
- // FIXME(#78): format impl definitions.
- ast::Item_::ItemImpl(_, _, _, _, _, ref impl_items) => {
+ ast::Item_::ItemImpl(..) => {
self.format_missing_with_indent(item.span.lo);
- self.block_indent = self.block_indent.block_indent(self.config);
- for item in impl_items {
- self.visit_impl_item(&item);
+ if let Some(impl_str) = format_impl(&self.get_context(), item, self.block_indent) {
+ self.buffer.push_str(&impl_str);
+ self.last_pos = item.span.hi;
}
- self.block_indent = self.block_indent.block_unindent(self.config);
}
// FIXME(#78): format traits.
ast::Item_::ItemTrait(_, _, _, ref trait_items) => {
@@ -334,7 +332,7 @@ impl<'a> FmtVisitor<'a> {
}
}
- fn visit_impl_item(&mut self, ii: &ast::ImplItem) {
+ pub fn visit_impl_item(&mut self, ii: &ast::ImplItem) {
if self.visit_attrs(&ii.attrs) {
return;
}
From b577f95e3c10b6b39443b7e2542ee046304b4153 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 23 Nov 2015 12:02:54 +1300
Subject: [PATCH 2/5] Reformatting due to changes
---
src/comment.rs | 10 ++++++++--
tests/target/pattern.rs | 2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/comment.rs b/src/comment.rs
index 0f118ca5d29..cb3b4b5f4d7 100644
--- a/src/comment.rs
+++ b/src/comment.rs
@@ -219,7 +219,10 @@ pub enum CodeCharKind {
Comment,
}
-impl<T> CharClasses<T> where T: Iterator, T::Item: RichChar {
+impl<T> CharClasses<T>
+ where T: Iterator,
+ T::Item: RichChar
+{
fn new(base: T) -> CharClasses<T> {
CharClasses {
base: base.peekable(),
@@ -228,7 +231,10 @@ impl<T> CharClasses<T> where T: Iterator, T::Item: RichChar {
}
}
-impl<T> Iterator for CharClasses<T> where T: Iterator, T::Item: RichChar {
+impl<T> Iterator for CharClasses<T>
+ where T: Iterator,
+ T::Item: RichChar
+{
type Item = (CodeCharKind, T::Item);
fn next(&mut self) -> Option<(CodeCharKind, T::Item)> {
diff --git a/tests/target/pattern.rs b/tests/target/pattern.rs
index aa121268daf..eb42f9fde73 100644
--- a/tests/target/pattern.rs
+++ b/tests/target/pattern.rs
@@ -15,7 +15,7 @@ fn main() {
}
}
-impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> {
+impl<'a, 'b> ResolveGeneratedContentFragmentMutator<'a, 'b> {
fn mutate_fragment(&mut self, fragment: &mut Fragment) {
match **info {
GeneratedContentInfo::ContentItem(ContentItem::Counter(ref counter_name,
From e86872c95ba21ed16b791a95e30cffb52b82e0ba Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 23 Nov 2015 13:07:53 +1300
Subject: [PATCH 3/5] tests
---
tests/source/impls.rs | 23 +++++++++++++++++++++++
tests/target/impls.rs | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
create mode 100644 tests/source/impls.rs
create mode 100644 tests/target/impls.rs
diff --git a/tests/source/impls.rs b/tests/source/impls.rs
new file mode 100644
index 00000000000..8e7b561ae4a
--- /dev/null
+++ b/tests/source/impls.rs
@@ -0,0 +1,23 @@
+impl Foo for Bar { fn foo() { "hi" } }
+
+pub impl Foo for Bar {
+ // Comment 1
+ fn foo() { "hi" }
+ // Comment 2
+ fn foo() { "hi" }
+ // Comment 3
+}
+
+pub unsafe impl<'a, 'b, X, Y: Foo<Bar>> !Foo<'a, X> for Bar<'b, Y> where X: Foo<'a, Z> {
+ fn foo() { "hi" }
+}
+
+impl<'a, 'b, X, Y: Foo<Bar>> Foo<'a, X> for Bar<'b, Y> where X: Fooooooooooooooooooooooooooooo<'a, Z>
+{
+ fn foo() { "hi" }
+}
+
+impl<'a, 'b, X, Y: Foo<Bar>> Foo<'a, X> for Bar<'b, Y> where X: Foooooooooooooooooooooooooooo<'a, Z>
+{
+ fn foo() { "hi" }
+}
diff --git a/tests/target/impls.rs b/tests/target/impls.rs
new file mode 100644
index 00000000000..7530bf16ede
--- /dev/null
+++ b/tests/target/impls.rs
@@ -0,0 +1,38 @@
+impl Foo for Bar {
+ fn foo() {
+ "hi"
+ }
+}
+
+pub impl Foo for Bar {
+ // Comment 1
+ fn foo() {
+ "hi"
+ }
+ // Comment 2
+ fn foo() {
+ "hi"
+ }
+}
+
+pub unsafe impl<'a, 'b, X, Y: Foo<Bar>> !Foo<'a, X> for Bar<'b, Y> where X: Foo<'a, Z>
+{
+ fn foo() {
+ "hi"
+ }
+}
+
+impl<'a, 'b, X, Y: Foo<Bar>> Foo<'a, X> for Bar<'b, Y>
+ where X: Fooooooooooooooooooooooooooooo<'a, Z>
+{
+ fn foo() {
+ "hi"
+ }
+}
+
+impl<'a, 'b, X, Y: Foo<Bar>> Foo<'a, X> for Bar<'b, Y> where X: Foooooooooooooooooooooooooooo<'a, Z>
+{
+ fn foo() {
+ "hi"
+ }
+}
From e3f39941de464a4110ccf11b974ed86a53ec2555 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 23 Nov 2015 13:23:41 +1300
Subject: [PATCH 4/5] Types which can be return types for function types
Fixes #643
---
src/types.rs | 23 +++++++++++------------
tests/target/fn-ty.rs | 12 ++++++++++++
2 files changed, 23 insertions(+), 12 deletions(-)
create mode 100644 tests/target/fn-ty.rs
diff --git a/src/types.rs b/src/types.rs
index 4faa0a05f91..c483032b451 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -215,8 +215,12 @@ fn rewrite_segment(expr_context: bool,
format!("{}<{}>", separator, list_str)
}
ast::PathParameters::ParenthesizedParameters(ref data) => {
+ let output = match data.output {
+ Some(ref ty) => FunctionRetTy::Return(ty.clone()),
+ None => FunctionRetTy::DefaultReturn(codemap::DUMMY_SP),
+ };
try_opt!(format_function_type(data.inputs.iter().map(|x| &**x),
- data.output.as_ref().map(|x| &**x),
+ &output,
data.span,
context,
width,
@@ -229,7 +233,7 @@ fn rewrite_segment(expr_context: bool,
}
fn format_function_type<'a, I>(inputs: I,
- output: Option<&ast::Ty>,
+ output: &FunctionRetTy,
span: Span,
context: &RewriteContext,
width: usize,
@@ -253,13 +257,14 @@ fn format_function_type<'a, I>(inputs: I,
let list_str = try_opt!(format_fn_args(items, budget, offset, context.config));
- let output = match output {
- Some(ref ty) => {
+ let output = match *output {
+ FunctionRetTy::Return(ref ty) => {
let budget = try_opt!(width.checked_sub(4));
let type_str = try_opt!(ty.rewrite(context, budget, offset + 4));
format!(" -> {}", type_str)
}
- None => String::new(),
+ FunctionRetTy::NoReturn(..) => " -> !".to_owned(),
+ FunctionRetTy::DefaultReturn(..) => String::new(),
};
let infix = if output.len() + list_str.len() > width {
@@ -540,17 +545,11 @@ fn rewrite_bare_fn(bare_fn: &ast::BareFnTy,
result.push_str("fn");
- let output = match bare_fn.decl.output {
- FunctionRetTy::Return(ref ty) => Some(&**ty),
- FunctionRetTy::NoReturn(..) => None,
- FunctionRetTy::DefaultReturn(..) => unreachable!(),
- };
-
let budget = try_opt!(width.checked_sub(result.len()));
let indent = offset + result.len();
let rewrite = try_opt!(format_function_type(bare_fn.decl.inputs.iter().map(|x| &*(x.ty)),
- output,
+ &bare_fn.decl.output,
span,
context,
budget,
diff --git a/tests/target/fn-ty.rs b/tests/target/fn-ty.rs
new file mode 100644
index 00000000000..7432fcded34
--- /dev/null
+++ b/tests/target/fn-ty.rs
@@ -0,0 +1,12 @@
+fn f(xxxxxxxxxxxxxxxxxx: fn(a, b, b) -> a,
+ xxxxxxxxxxxxxxxxxx: fn() -> a,
+ xxxxxxxxxxxxxxxxxx: fn(a, b, b),
+ xxxxxxxxxxxxxxxxxx: fn(),
+ xxxxxxxxxxxxxxxxxx: fn(a, b, b) -> !,
+ xxxxxxxxxxxxxxxxxx: fn() -> !)
+ where F1: Fn(a, b, b) -> a,
+ F2: Fn(a, b, b),
+ F3: Fn(),
+ F4: Fn() -> u32
+{
+}
From 2661592d59a30a175f71667a9ad751f52e58b3eb Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 23 Nov 2015 15:22:00 +1300
Subject: [PATCH 5/5] Handle multiply-referenced files
Fixes #645
---
src/lib.rs | 4 ++--
src/visitor.rs | 4 ++--
tests/target/mulit-file.rs | 10 ++++++++++
3 files changed, 14 insertions(+), 4 deletions(-)
create mode 100644 tests/target/mulit-file.rs
diff --git a/src/lib.rs b/src/lib.rs
index 3923f6b742d..dd065a9939c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -298,7 +298,7 @@ fn fmt_ast(krate: &ast::Crate,
println!("Formatting {}", path);
}
let mut visitor = FmtVisitor::from_codemap(parse_session, config, Some(mode));
- visitor.format_separate_mod(module, path);
+ visitor.format_separate_mod(module);
file_map.insert(path.to_owned(), visitor.buffer);
}
file_map
@@ -404,7 +404,7 @@ pub fn format_string(input: String, config: &Config, mode: WriteMode) -> FileMap
// do the actual formatting
let mut visitor = FmtVisitor::from_codemap(&parse_session, config, Some(mode));
- visitor.format_separate_mod(&krate.module, path);
+ visitor.format_separate_mod(&krate.module);
// append final newline
visitor.buffer.push_str("\n");
diff --git a/src/visitor.rs b/src/visitor.rs
index fcc017c98f2..15db08f7942 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -463,8 +463,8 @@ impl<'a> FmtVisitor<'a> {
}
}
- pub fn format_separate_mod(&mut self, m: &ast::Mod, filename: &str) {
- let filemap = self.codemap.get_filemap(filename);
+ pub fn format_separate_mod(&mut self, m: &ast::Mod) {
+ let filemap = self.codemap.lookup_char_pos(m.inner.lo).file;
self.last_pos = filemap.start_pos;
self.block_indent = Indent::empty();
self.walk_mod_items(m);
diff --git a/tests/target/mulit-file.rs b/tests/target/mulit-file.rs
new file mode 100644
index 00000000000..1f829b36f3f
--- /dev/null
+++ b/tests/target/mulit-file.rs
@@ -0,0 +1,10 @@
+// Tests that where a single file is referred to in multiple places, we don't
+// crash.
+
+#[cfg(all(foo))]
+#[path = "closure.rs"]
+pub mod imp;
+
+#[cfg(all(bar))]
+#[path = "closure.rs"]
+pub mod imp;