Skip to content

Format all patterns (well, except macros) #820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 14, 2016
Merged

Format all patterns (well, except macros) #820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d86cfb3
Format all patterns (well, except macros)
nrc Feb 12, 2016
160eb73
reviewer changes
nrc Feb 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Rewrite for ast::ViewPath {
}

fn rewrite_single_use_list(path_str: String, vpi: &ast::PathListItem) -> String {
let path_item_str = if let ast::PathListItem_::PathListIdent{ name, .. } = vpi.node {
let path_item_str = if let ast::PathListItem_::PathListIdent { name, .. } = vpi.node {
// A name.
if path_str.is_empty() {
name.to_string()
Expand All @@ -74,17 +74,17 @@ fn rewrite_single_use_list(path_str: String, vpi: &ast::PathListItem) -> String

fn rewrite_path_item(vpi: &&ast::PathListItem) -> Option<String> {
let path_item_str = match vpi.node {
ast::PathListItem_::PathListIdent{ name, .. } => name.to_string(),
ast::PathListItem_::PathListMod{ .. } => "self".to_owned(),
ast::PathListItem_::PathListIdent { name, .. } => name.to_string(),
ast::PathListItem_::PathListMod { .. } => "self".to_owned(),
};

Some(append_alias(path_item_str, vpi))
}

fn append_alias(path_item_str: String, vpi: &ast::PathListItem) -> String {
match vpi.node {
ast::PathListItem_::PathListIdent{ rename: Some(rename), .. } |
ast::PathListItem_::PathListMod{ rename: Some(rename), .. } => {
ast::PathListItem_::PathListIdent { rename: Some(rename), .. } |
ast::PathListItem_::PathListMod { rename: Some(rename), .. } => {
format!("{} as {}", path_item_str, rename)
}
_ => path_item_str,
Expand Down
159 changes: 126 additions & 33 deletions src/patterns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,32 @@ use lists::{format_item_list, itemize_list};
use expr::{rewrite_unary_prefix, rewrite_pair, rewrite_tuple};
use types::rewrite_path;

use syntax::ast::{BindingMode, Pat, Pat_};
use syntax::ast::{BindingMode, Pat, Pat_, FieldPat};

// FIXME(#18): implement pattern formatting.
impl Rewrite for Pat {
fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
match self.node {
Pat_::PatBox(ref pat) => rewrite_unary_prefix(context, "box ", &**pat, width, offset),
Pat_::PatIdent(binding_mode, ident, None) => {
Pat_::PatIdent(binding_mode, ident, ref sub_pat) => {
let (prefix, mutability) = match binding_mode {
BindingMode::BindByRef(mutability) => ("ref ", mutability),
BindingMode::BindByValue(mutability) => ("", mutability),
};
let mut_infix = format_mutability(mutability);
let result = format!("{}{}{}", prefix, mut_infix, ident.node);
let id_str = ident.node.to_string();

let sub_pat = match *sub_pat {
Some(ref p) => {
// 3 - ` @ `.
let width = try_opt!(width.checked_sub(prefix.len() + mut_infix.len() +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No 3 for " @ "?

id_str.len() +
3));
format!(" @ {}", try_opt!(p.rewrite(context, width, offset)))
}
None => "".to_owned(),
};

let result = format!("{}{}{}{}", prefix, mut_infix, id_str, sub_pat);
wrap_str(result, context.config.max_width, width, offset)
}
Pat_::PatWild => {
Expand All @@ -55,39 +67,106 @@ impl Rewrite for Pat {
width,
offset)
}
Pat_::PatEnum(ref path, Some(ref pat_vec)) => {
let path_str = try_opt!(::types::rewrite_path(context,
true,
None,
path,
width,
offset));
Pat_::PatEnum(ref path, ref pat_vec) => {
let path_str = try_opt!(rewrite_path(context, true, None, path, width, offset));

if pat_vec.is_empty() {
Some(path_str)
} else {
// 1 = (
let width = try_opt!(width.checked_sub(path_str.len() + 1));
let offset = offset + path_str.len() + 1;
let items = itemize_list(context.codemap,
pat_vec.iter(),
")",
|item| item.span.lo,
|item| item.span.hi,
|item| item.rewrite(context, width, offset),
span_after(self.span, "(", context.codemap),
self.span.hi);
Some(format!("{}({})",
path_str,
try_opt!(format_item_list(items, width, offset, context.config))))
match *pat_vec {
Some(ref pat_vec) => {
if pat_vec.is_empty() {
Some(path_str)
} else {
// 1 = (
let width = try_opt!(width.checked_sub(path_str.len() + 1));
let offset = offset + path_str.len() + 1;
let items = itemize_list(context.codemap,
pat_vec.iter(),
")",
|item| item.span.lo,
|item| item.span.hi,
|item| item.rewrite(context, width, offset),
span_after(self.span, "(", context.codemap),
self.span.hi);
Some(format!("{}({})",
path_str,
try_opt!(format_item_list(items,
width,
offset,
context.config))))
}
}
None => Some(format!("{}(..)", path_str)),
}
}
Pat_::PatLit(ref expr) => expr.rewrite(context, width, offset),
// FIXME(#8): format remaining pattern variants.
Pat_::PatIdent(_, _, Some(..)) |
Pat_::PatEnum(_, None) |
Pat_::PatStruct(..) |
Pat_::PatVec(..) |
Pat_::PatVec(ref prefix, ref slice_pat, ref suffix) => {
// Rewrite all the sub-patterns.
let prefix = prefix.iter().map(|p| p.rewrite(context, width, offset));
let slice_pat = slice_pat.as_ref().map(|p| {
Some(format!("{}..", try_opt!(p.rewrite(context, width, offset))))
});
let suffix = suffix.iter().map(|p| p.rewrite(context, width, offset));

// Munge them together.
let pats: Option<Vec<String>> = prefix.chain(slice_pat.into_iter())
.chain(suffix)
.collect();

// Check that all the rewrites succeeded, and if not return None.
let pats = try_opt!(pats);

// Unwrap all the sub-strings and join them with commas.
let result = format!("[{}]", pats.join(", "));
wrap_str(result, context.config.max_width, width, offset)
}
Pat_::PatStruct(ref path, ref fields, elipses) => {
let path = try_opt!(rewrite_path(context, true, None, path, width, offset));

let (elipses_str, terminator) = if elipses {
(", ..", "..")
} else {
("", "}")
};

// 5 = `{` plus space before and after plus `}` plus space before.
let budget = try_opt!(width.checked_sub(path.len() + 5 + elipses_str.len()));
// FIXME Using visual indenting, should use block or visual to match
// struct lit preference (however, in practice I think it is rare
// for struct patterns to be multi-line).
// 3 = `{` plus space before and after.
let offset = offset + path.len() + 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment explaining the origin of the 3 and 5?


let items = itemize_list(context.codemap,
fields.iter(),
terminator,
|f| f.span.lo,
|f| f.span.hi,
|f| f.node.rewrite(context, budget, offset),
span_after(self.span, "{", context.codemap),
self.span.hi);
let mut field_string = try_opt!(format_item_list(items,
budget,
offset,
context.config));
if elipses {
if field_string.contains('\n') {
field_string.push_str(",\n");
field_string.push_str(&offset.to_string(context.config));
field_string.push_str("..");
} else {
if field_string.len() > 0 {
field_string.push_str(", ");
}
field_string.push_str("..");
}
}

if field_string.is_empty() {
Some(format!("{} {{}}", path))
} else {
Some(format!("{} {{ {} }}", path, field_string))
}
}
// FIXME(#819) format pattern macros.
Pat_::PatMac(..) => {
wrap_str(context.snippet(self.span),
context.config.max_width,
Expand All @@ -97,3 +176,17 @@ impl Rewrite for Pat {
}
}
}

impl Rewrite for FieldPat {
fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
let pat = self.pat.rewrite(context, width, offset);
if self.is_shorthand {
pat
} else {
wrap_str(format!("{}: {}", self.ident.to_string(), try_opt!(pat)),
context.config.max_width,
width,
offset)
}
}
}
14 changes: 14 additions & 0 deletions tests/source/pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@ fn main() {
let Some ( ref xyz /* comment! */) = opt;

if let None = opt2 { panic!("oh noes"); }

let foo@bar (f) = 42;
let a::foo ( ..) = 42;
let [ ] = 42;
let [a.., b,c ] = 42;
let [ a,b,c.. ] = 42;
let [a, b, c, d..,e,f, g] = 42;
let foo { } = 42;
let foo {..} = 42;
let foo { x, y: ref foo, .. } = 42;
let foo { x, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo, .. } = 42;
let foo { x, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo, } = 42;
let foo { x, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo, .. };
let foo { x, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo, };
}

impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> {
Expand Down
2 changes: 2 additions & 0 deletions tests/system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,8 @@ fn handle_result(result: HashMap<String, String>,

if fmt_text != text {
let diff = make_diff(&text, &fmt_text, DIFF_CONTEXT_SIZE);
assert!(!diff.is_empty(),
"Empty diff? Maybe due to a missing a newline at the end of a file?");
failures.insert(file_name, diff);
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/target/match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ fn issue280() {

fn issue383() {
match resolution.last_private {
LastImport{..} => false,
LastImport { .. } => false,
_ => true,
};
}
Expand Down
17 changes: 17 additions & 0 deletions tests/target/pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@ fn main() {
if let None = opt2 {
panic!("oh noes");
}

let foo @ bar(f) = 42;
let a::foo(..) = 42;
let [] = 42;
let [a.., b, c] = 42;
let [a, b, c..] = 42;
let [a, b, c, d.., e, f, g] = 42;
let foo {} = 42;
let foo { .. } = 42;
let foo { x, y: ref foo, .. } = 42;
let foo { x, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo, .. } = 42;
let foo { x, yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo } = 42;
let foo { x,
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo,
.. };
let foo { x,
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: ref foo };
}

impl<'a, 'b> ResolveGeneratedContentFragmentMutator<'a, 'b> {
Expand Down