Skip to content

Try shorthand #893

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.

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 3 commits into from
May 12, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
49 changes: 44 additions & 5 deletions src/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// except according to those terms.

/// Formatting of chained expressions, i.e. expressions which are chained by
/// dots: struct and enum field access and method calls.
/// dots: struct and enum field access, method calls, and try shorthand (?).
///
/// Instead of walking these subexpressions one-by-one, as is our usual strategy
/// for expression formatting, we collect maximal sequences of these expressions
Expand Down Expand Up @@ -81,7 +81,6 @@
/// true, then we allow the last method call to spill over multiple lines without
/// forcing the rest of the chain to be split.


use Indent;
use rewrite::{Rewrite, RewriteContext};
use utils::{wrap_str, first_line_width};
Expand Down Expand Up @@ -109,8 +108,16 @@ pub fn rewrite_chain(expr: &ast::Expr,
// put the first non-parent item on the same line as the parent.
let (indent, extend) = if !parent_rewrite.contains('\n') && is_continuable(parent) ||
parent_rewrite.len() <= context.config.tab_spaces {
// Try and put at least the first two items on the same line.
(chain_indent(context, offset + Indent::new(0, parent_rewrite.len())), true)
// <<<<<<< HEAD
// // Try and put at least the first two items on the same line.
// (chain_indent(context, offset + Indent::new(0, parent_rewrite.len())), true)
// =======
let indent = if let ast::ExprKind::Try(..) = subexpr_list.last().unwrap().node {
parent_block_indent.block_indent(context.config)
} else {
offset + Indent::new(0, parent_rewrite.len())
};
(indent, true)
} else if is_block_expr(parent, &parent_rewrite) {
// The parent is a block, so align the rest of the chain with the closing
// brace.
Expand Down Expand Up @@ -184,12 +191,27 @@ pub fn rewrite_chain(expr: &ast::Expr,
wrap_str(format!("{}{}{}",
parent_rewrite,
first_connector,
rewrites.join(&connector)),
join_rewrites(&rewrites, &subexpr_list, &connector)),
context.config.max_width,
width,
offset)
}

fn join_rewrites(rewrites: &[String], subexps: &[&ast::Expr], connector: &str) -> String {
let mut rewrite_iter = rewrites.iter();
let mut result = rewrite_iter.next().unwrap().clone();

for (rewrite, expr) in rewrite_iter.zip(subexps.iter()) {
match expr.node {
ast::ExprKind::Try(_) => (),
_ => result.push_str(connector),
};
result.push_str(&rewrite[..]);
Copy link
Member

Choose a reason for hiding this comment

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

nit: probably don't need the [..]

}

result
}

// States whether an expression's last line exclusively consists of closing
// parens, braces, and brackets in its idiomatic formatting.
fn is_block_expr(expr: &ast::Expr, repr: &str) -> bool {
Expand Down Expand Up @@ -293,6 +315,16 @@ fn rewrite_method_call_with_overflow(expr_kind: &ast::ExprKind,
}
}

fn pop_expr_chain(expr: &ast::Expr) -> Option<&ast::Expr> {
match expr.node {
ast::ExprKind::MethodCall(_, _, ref expressions) => Some(&expressions[0]),
ast::ExprKind::TupField(ref subexpr, _) |
ast::ExprKind::Field(ref subexpr, _) |
ast::ExprKind::Try(ref subexpr) => Some(subexpr),
_ => None,
}
}

// Rewrite the last element in the chain `expr`. E.g., given `a.b.c` we rewrite
// `.c`.
fn rewrite_chain_subexpr(expr: &ast::Expr,
Expand Down Expand Up @@ -328,6 +360,13 @@ fn rewrite_chain_subexpr(expr: &ast::Expr,
None
}
}
ast::ExprKind::Try(_) => {
if width >= 1 {
Some("?".into())
} else {
None
}
}
_ => unreachable!(),
}
}
Expand Down
29 changes: 22 additions & 7 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ impl Rewrite for ast::Expr {
ast::ExprKind::Closure(capture, ref fn_decl, ref body, _) => {
rewrite_closure(capture, fn_decl, body, self.span, context, width, offset)
}
// ast::ExprKind::Try(..) |
Copy link
Contributor

Choose a reason for hiding this comment

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

is this meant to be commented out?

ast::ExprKind::Field(..) |
ast::ExprKind::TupField(..) |
ast::ExprKind::MethodCall(..) => rewrite_chain(self, context, width, offset),
Expand Down Expand Up @@ -199,21 +200,20 @@ impl Rewrite for ast::Expr {
rewrite_unary_prefix(context, delim, &**rhs, width, offset)
}
(Some(ref lhs), None) => {
Some(format!("{}{}",
try_opt!(lhs.rewrite(context,
try_opt!(width.checked_sub(delim.len())),
offset)),
delim))
rewrite_unary_suffix(context, delim, &**lhs, width, offset)
}
(None, None) => wrap_str(delim.into(), context.config.max_width, width, offset),
}
}
ast::ExprKind::Try(ref expr) => {
rewrite_unary_suffix(context, "?", &**expr, width, offset)
}
// We do not format these expressions yet, but they should still
// satisfy our width restrictions.
ast::ExprKind::InPlace(..) |
ast::ExprKind::InlineAsm(..) |
// TODO(#867): Handle try shorthand
ast::ExprKind::Try(_) => {
// TODO(#848): Handle type ascription
ast::ExprKind::Type(_, _) => {
wrap_str(context.snippet(self.span),
context.config.max_width,
width,
Expand Down Expand Up @@ -1762,6 +1762,21 @@ pub fn rewrite_unary_prefix<R: Rewrite>(context: &RewriteContext,
.map(|r| format!("{}{}", prefix, r))
}

// FIXME: this is probably not correct for multi-line Rewrites. we should
// subtract suffix.len() from the last line budget, not the first!
pub fn rewrite_unary_suffix<R: Rewrite>(context: &RewriteContext,
suffix: &str,
rewrite: &R,
width: usize,
offset: Indent)
-> Option<String> {
rewrite.rewrite(context, try_opt!(width.checked_sub(suffix.len())), offset)
.map(|mut r| {
r.push_str(suffix);
r
})
}

fn rewrite_unary_op(context: &RewriteContext,
op: &ast::UnOp,
expr: &ast::Expr,
Expand Down
9 changes: 9 additions & 0 deletions tests/source/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,12 @@ fn issue587() {

std::mem::transmute(dl.symbol::<()>("init").unwrap())
}

fn try_shorthand() {
let x = expr?;
let y = expr.kaas()?.test();
let loooooooooooooooooooooooooooooooooooooooooong = does_this?.look?.good?.should_we_break?.after_the_first_question_mark?;
let yyyy = expr?.another?.another?.another?.another?.another?.another?.another?.another?.test();
let zzzz = expr?.another?.another?.another?.another?;
let aaa = x ???????????? ?????????????? ???? ????? ?????????????? ????????? ?????????????? ??;
}
13 changes: 13 additions & 0 deletions tests/target/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,16 @@ fn issue587() {

std::mem::transmute(dl.symbol::<()>("init").unwrap())
}

fn try_shorthand() {
let x = expr?;
let y = expr.kaas()?.test();
let loooooooooooooooooooooooooooooooooooooooooong = does_this?
.look?
.good?
.should_we_break?
.after_the_first_question_mark?;
let yyyy = expr?.another?.another?.another?.another?.another?.another?.another?.another?.test();
let zzzz = expr?.another?.another?.another?.another?;
let aaa = x??????????????????????????????????????????????????????????????????????????;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

output looks ok to me!