From e05badf1fe82276e9c6a9cc25dc622ab8918c9cf Mon Sep 17 00:00:00 2001 From: ding-young Date: Sun, 25 Aug 2024 15:42:46 +0900 Subject: [PATCH] impl rewrite_result for ast::Expr refactor - formatting refactor - tests refactor - updated dir value refactor - updated tests refactor - updated tests --- src/config/mod.rs | 50 ++++++------ src/expr.rs | 174 +++++++++++++++++++----------------------- src/ignore_path.rs | 9 ++- src/matches.rs | 23 +++--- src/parse/session.rs | 4 +- src/stmt.rs | 4 +- src/types.rs | 4 + src/utils.rs | 8 -- tests/rustfmt/main.rs | 11 ++- 9 files changed, 139 insertions(+), 148 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index 9243ea2332d..5e6acc2b2fe 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -394,12 +394,11 @@ impl Config { if !err.is_empty() { eprint!("{err}"); } - Ok(parsed_config.to_parsed_config( - style_edition, - edition, - version, - file_path.parent().unwrap_or(Path::new("")), - )) + let dir = file_path.parent().ok_or_else(|| { + format!("failed to get parent directory for {}", file_path.display()) + })?; + + Ok(parsed_config.to_parsed_config(style_edition, edition, version, dir)) } Err(e) => { let err_msg = format!( @@ -675,7 +674,7 @@ mod test { #[test] fn test_was_set() { - let config = Config::from_toml("hard_tabs = true", Path::new("")).unwrap(); + let config = Config::from_toml("hard_tabs = true", Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.was_set().hard_tabs(), true); assert_eq!(config.was_set().verbose(), false); @@ -934,7 +933,8 @@ make_backup = false #[nightly_only_test] #[test] fn test_unstable_from_toml() { - let config = Config::from_toml("unstable_features = true", Path::new("")).unwrap(); + let config = + Config::from_toml("unstable_features = true", Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.was_set().unstable_features(), true); assert_eq!(config.unstable_features(), true); } @@ -964,7 +964,7 @@ make_backup = false unstable_features = true merge_imports = true "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.imports_granularity(), ImportGranularity::Crate); } @@ -976,7 +976,7 @@ make_backup = false merge_imports = true imports_granularity = "Preserve" "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); } @@ -987,7 +987,7 @@ make_backup = false unstable_features = true merge_imports = true "#; - let mut config = Config::from_toml(toml, Path::new("")).unwrap(); + let mut config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); config.override_value("imports_granularity", "Preserve"); assert_eq!(config.imports_granularity(), ImportGranularity::Preserve); } @@ -999,7 +999,7 @@ make_backup = false unstable_features = true imports_granularity = "Module" "#; - let mut config = Config::from_toml(toml, Path::new("")).unwrap(); + let mut config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); config.override_value("merge_imports", "true"); // no effect: the new option always takes precedence assert_eq!(config.imports_granularity(), ImportGranularity::Module); @@ -1016,7 +1016,7 @@ make_backup = false use_small_heuristics = "Default" max_width = 200 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), 120); assert_eq!(config.attr_fn_like_width(), 140); assert_eq!(config.chain_width(), 120); @@ -1032,7 +1032,7 @@ make_backup = false use_small_heuristics = "Max" max_width = 120 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), 120); assert_eq!(config.attr_fn_like_width(), 120); assert_eq!(config.chain_width(), 120); @@ -1048,7 +1048,7 @@ make_backup = false use_small_heuristics = "Off" max_width = 100 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), usize::max_value()); assert_eq!(config.attr_fn_like_width(), usize::max_value()); assert_eq!(config.chain_width(), usize::max_value()); @@ -1070,7 +1070,7 @@ make_backup = false struct_lit_width = 30 struct_variant_width = 34 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), 20); assert_eq!(config.attr_fn_like_width(), 40); assert_eq!(config.chain_width(), 20); @@ -1092,7 +1092,7 @@ make_backup = false struct_lit_width = 30 struct_variant_width = 34 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), 20); assert_eq!(config.attr_fn_like_width(), 40); assert_eq!(config.chain_width(), 20); @@ -1114,7 +1114,7 @@ make_backup = false struct_lit_width = 30 struct_variant_width = 34 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), 20); assert_eq!(config.attr_fn_like_width(), 40); assert_eq!(config.chain_width(), 20); @@ -1130,7 +1130,7 @@ make_backup = false max_width = 90 fn_call_width = 95 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.fn_call_width(), 90); } @@ -1140,7 +1140,7 @@ make_backup = false max_width = 80 attr_fn_like_width = 90 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.attr_fn_like_width(), 80); } @@ -1150,7 +1150,7 @@ make_backup = false max_width = 78 struct_lit_width = 90 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.struct_lit_width(), 78); } @@ -1160,7 +1160,7 @@ make_backup = false max_width = 80 struct_variant_width = 90 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.struct_variant_width(), 80); } @@ -1170,7 +1170,7 @@ make_backup = false max_width = 60 array_width = 80 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.array_width(), 60); } @@ -1180,7 +1180,7 @@ make_backup = false max_width = 80 chain_width = 90 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.chain_width(), 80); } @@ -1190,7 +1190,7 @@ make_backup = false max_width = 70 single_line_if_else_max_width = 90 "#; - let config = Config::from_toml(toml, Path::new("")).unwrap(); + let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); assert_eq!(config.single_line_if_else_max_width(), 70); } diff --git a/src/expr.rs b/src/expr.rs index 5bd87d00b1d..32bfc8b4722 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -39,6 +39,10 @@ use crate::visitor::FmtVisitor; impl Rewrite for ast::Expr { fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { + self.rewrite_result(context, shape).ok() + } + + fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult { format_expr(self, ExprType::SubExpression, context, shape) } } @@ -58,14 +62,14 @@ pub(crate) fn format_expr( expr_type: ExprType, context: &RewriteContext<'_>, shape: Shape, -) -> Option { - skip_out_of_file_lines_range!(context, expr.span); +) -> RewriteResult { + skip_out_of_file_lines_range_err!(context, expr.span); if contains_skip(&*expr.attrs) { - return Some(context.snippet(expr.span()).to_owned()); + return Ok(context.snippet(expr.span()).to_owned()); } let shape = if expr_type == ExprType::Statement && semicolon_for_expr(context, expr) { - shape.sub_width(1)? + shape.sub_width(1).max_width_error(shape.width, expr.span)? } else { shape }; @@ -79,41 +83,38 @@ pub(crate) fn format_expr( shape, choose_separator_tactic(context, expr.span), None, - ) - .ok(), + ), ast::ExprKind::Lit(token_lit) => { if let Ok(expr_rw) = rewrite_literal(context, token_lit, expr.span, shape) { - Some(expr_rw) + Ok(expr_rw) } else { if let LitKind::StrRaw(_) = token_lit.kind { - Some(context.snippet(expr.span).trim().into()) + Ok(context.snippet(expr.span).trim().into()) } else { - None + Err(RewriteError::Unknown) } } } ast::ExprKind::Call(ref callee, ref args) => { let inner_span = mk_sp(callee.span.hi(), expr.span.hi()); - let callee_str = callee.rewrite(context, shape)?; - rewrite_call(context, &callee_str, args, inner_span, shape).ok() + let callee_str = callee.rewrite_result(context, shape)?; + rewrite_call(context, &callee_str, args, inner_span, shape) } - ast::ExprKind::Paren(ref subexpr) => rewrite_paren(context, subexpr, shape, expr.span).ok(), + ast::ExprKind::Paren(ref subexpr) => rewrite_paren(context, subexpr, shape, expr.span), ast::ExprKind::Binary(op, ref lhs, ref rhs) => { // FIXME: format comments between operands and operator - rewrite_all_pairs(expr, shape, context) - .or_else(|_| { - rewrite_pair( - &**lhs, - &**rhs, - PairParts::infix(&format!(" {} ", context.snippet(op.span))), - context, - shape, - context.config.binop_separator(), - ) - }) - .ok() + rewrite_all_pairs(expr, shape, context).or_else(|_| { + rewrite_pair( + &**lhs, + &**rhs, + PairParts::infix(&format!(" {} ", context.snippet(op.span))), + context, + shape, + context.config.binop_separator(), + ) + }) } - ast::ExprKind::Unary(op, ref subexpr) => rewrite_unary_op(context, op, subexpr, shape).ok(), + ast::ExprKind::Unary(op, ref subexpr) => rewrite_unary_op(context, op, subexpr, shape), ast::ExprKind::Struct(ref struct_expr) => { let ast::StructExpr { qself, @@ -131,19 +132,17 @@ pub(crate) fn format_expr( expr.span, shape, ) - .ok() } ast::ExprKind::Tup(ref items) => { - rewrite_tuple(context, items.iter(), expr.span, shape, items.len() == 1).ok() - } - ast::ExprKind::Let(ref pat, ref expr, _span, _) => { - rewrite_let(context, shape, pat, expr).ok() + rewrite_tuple(context, items.iter(), expr.span, shape, items.len() == 1) } + ast::ExprKind::Let(ref pat, ref expr, _span, _) => rewrite_let(context, shape, pat, expr), ast::ExprKind::If(..) | ast::ExprKind::ForLoop { .. } | ast::ExprKind::Loop(..) | ast::ExprKind::While(..) => to_control_flow(expr, expr_type) - .and_then(|control_flow| control_flow.rewrite(context, shape)), + .unknown_error() + .and_then(|control_flow| control_flow.rewrite_result(context, shape)), ast::ExprKind::ConstBlock(ref anon_const) => { let rewrite = match anon_const.value.kind { ast::ExprKind::Block(ref block, opt_label) => { @@ -151,24 +150,24 @@ pub(crate) fn format_expr( // not the `ast::Block` node we're about to rewrite. To prevent dropping inner // attributes call `rewrite_block` directly. // See https://github.com/rust-lang/rustfmt/issues/6158 - rewrite_block(block, Some(&expr.attrs), opt_label, context, shape).ok()? + rewrite_block(block, Some(&expr.attrs), opt_label, context, shape)? } - _ => anon_const.rewrite(context, shape)?, + _ => anon_const.rewrite_result(context, shape)?, }; - Some(format!("const {}", rewrite)) + Ok(format!("const {}", rewrite)) } ast::ExprKind::Block(ref block, opt_label) => { match expr_type { ExprType::Statement => { if is_unsafe_block(block) { - rewrite_block(block, Some(&expr.attrs), opt_label, context, shape).ok() - } else if let rw @ Some(_) = + rewrite_block(block, Some(&expr.attrs), opt_label, context, shape) + } else if let Some(rw) = rewrite_empty_block(context, block, Some(&expr.attrs), opt_label, "", shape) { // Rewrite block without trying to put it in a single line. - rw + Ok(rw) } else { - let prefix = block_prefix(context, block, shape).ok()?; + let prefix = block_prefix(context, block, shape)?; rewrite_block_with_visitor( context, @@ -179,32 +178,31 @@ pub(crate) fn format_expr( shape, true, ) - .ok() } } ExprType::SubExpression => { - rewrite_block(block, Some(&expr.attrs), opt_label, context, shape).ok() + rewrite_block(block, Some(&expr.attrs), opt_label, context, shape) } } } ast::ExprKind::Match(ref cond, ref arms, kind) => { - rewrite_match(context, cond, arms, shape, expr.span, &expr.attrs, kind).ok() + rewrite_match(context, cond, arms, shape, expr.span, &expr.attrs, kind) } ast::ExprKind::Path(ref qself, ref path) => { - rewrite_path(context, PathContext::Expr, qself, path, shape).ok() + rewrite_path(context, PathContext::Expr, qself, path, shape) } ast::ExprKind::Assign(ref lhs, ref rhs, _) => { - rewrite_assignment(context, lhs, rhs, None, shape).ok() + rewrite_assignment(context, lhs, rhs, None, shape) } ast::ExprKind::AssignOp(ref op, ref lhs, ref rhs) => { - rewrite_assignment(context, lhs, rhs, Some(op), shape).ok() + rewrite_assignment(context, lhs, rhs, Some(op), shape) } ast::ExprKind::Continue(ref opt_label) => { let id_str = match *opt_label { Some(label) => format!(" {}", label.ident), None => String::new(), }; - Some(format!("continue{id_str}")) + Ok(format!("continue{id_str}")) } ast::ExprKind::Break(ref opt_label, ref opt_expr) => { let id_str = match *opt_label { @@ -213,16 +211,16 @@ pub(crate) fn format_expr( }; if let Some(ref expr) = *opt_expr { - rewrite_unary_prefix(context, &format!("break{id_str} "), &**expr, shape).ok() + rewrite_unary_prefix(context, &format!("break{id_str} "), &**expr, shape) } else { - Some(format!("break{id_str}")) + Ok(format!("break{id_str}")) } } ast::ExprKind::Yield(ref opt_expr) => { if let Some(ref expr) = *opt_expr { - rewrite_unary_prefix(context, "yield ", &**expr, shape).ok() + rewrite_unary_prefix(context, "yield ", &**expr, shape) } else { - Some("yield".to_string()) + Ok("yield".to_string()) } } ast::ExprKind::Closure(ref cl) => closures::rewrite_closure( @@ -236,37 +234,32 @@ pub(crate) fn format_expr( expr.span, context, shape, - ) - .ok(), + ), ast::ExprKind::Try(..) | ast::ExprKind::Field(..) | ast::ExprKind::MethodCall(..) - | ast::ExprKind::Await(_, _) => rewrite_chain(expr, context, shape).ok(), + | ast::ExprKind::Await(_, _) => rewrite_chain(expr, context, shape), ast::ExprKind::MacCall(ref mac) => { - rewrite_macro(mac, None, context, shape, MacroPosition::Expression) - .or_else(|_| { - wrap_str( - context.snippet(expr.span).to_owned(), - context.config.max_width(), - shape, - ) - .max_width_error(shape.width, expr.span) - }) - .ok() + rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|_| { + wrap_str( + context.snippet(expr.span).to_owned(), + context.config.max_width(), + shape, + ) + .max_width_error(shape.width, expr.span) + }) } - ast::ExprKind::Ret(None) => Some("return".to_owned()), + ast::ExprKind::Ret(None) => Ok("return".to_owned()), ast::ExprKind::Ret(Some(ref expr)) => { - rewrite_unary_prefix(context, "return ", &**expr, shape).ok() - } - ast::ExprKind::Become(ref expr) => { - rewrite_unary_prefix(context, "become ", &**expr, shape).ok() + rewrite_unary_prefix(context, "return ", &**expr, shape) } - ast::ExprKind::Yeet(None) => Some("do yeet".to_owned()), + ast::ExprKind::Become(ref expr) => rewrite_unary_prefix(context, "become ", &**expr, shape), + ast::ExprKind::Yeet(None) => Ok("do yeet".to_owned()), ast::ExprKind::Yeet(Some(ref expr)) => { - rewrite_unary_prefix(context, "do yeet ", &**expr, shape).ok() + rewrite_unary_prefix(context, "do yeet ", &**expr, shape) } ast::ExprKind::AddrOf(borrow_kind, mutability, ref expr) => { - rewrite_expr_addrof(context, borrow_kind, mutability, expr, shape).ok() + rewrite_expr_addrof(context, borrow_kind, mutability, expr, shape) } ast::ExprKind::Cast(ref expr, ref ty) => rewrite_pair( &**expr, @@ -275,10 +268,9 @@ pub(crate) fn format_expr( context, shape, SeparatorPlace::Front, - ) - .ok(), + ), ast::ExprKind::Index(ref expr, ref index, _) => { - rewrite_index(&**expr, &**index, context, shape).ok() + rewrite_index(&**expr, &**index, context, shape) } ast::ExprKind::Repeat(ref expr, ref repeats) => rewrite_pair( &**expr, @@ -287,8 +279,7 @@ pub(crate) fn format_expr( context, shape, SeparatorPlace::Back, - ) - .ok(), + ), ast::ExprKind::Range(ref lhs, ref rhs, limits) => { let delim = match limits { ast::RangeLimits::HalfOpen => "..", @@ -341,7 +332,6 @@ pub(crate) fn format_expr( shape, context.config.binop_separator(), ) - .ok() } (None, Some(rhs)) => { let sp_delim = if context.config.spaces_around_ranges() { @@ -349,7 +339,7 @@ pub(crate) fn format_expr( } else { default_sp_delim(None, Some(rhs)) }; - rewrite_unary_prefix(context, &sp_delim, &*rhs, shape).ok() + rewrite_unary_prefix(context, &sp_delim, &*rhs, shape) } (Some(lhs), None) => { let sp_delim = if context.config.spaces_around_ranges() { @@ -357,25 +347,25 @@ pub(crate) fn format_expr( } else { default_sp_delim(Some(lhs), None) }; - rewrite_unary_suffix(context, &sp_delim, &*lhs, shape).ok() + rewrite_unary_suffix(context, &sp_delim, &*lhs, shape) } - (None, None) => Some(delim.to_owned()), + (None, None) => Ok(delim.to_owned()), } } // We do not format these expressions yet, but they should still // satisfy our width restrictions. // Style Guide RFC for InlineAsm variant pending // https://github.com/rust-dev-tools/fmt-rfcs/issues/152 - ast::ExprKind::InlineAsm(..) => Some(context.snippet(expr.span).to_owned()), + ast::ExprKind::InlineAsm(..) => Ok(context.snippet(expr.span).to_owned()), ast::ExprKind::TryBlock(ref block) => { if let rw @ Ok(_) = rewrite_single_line_block(context, "try ", block, Some(&expr.attrs), None, shape) { - rw.ok() + rw } else { // 9 = `try ` let budget = shape.width.saturating_sub(9); - Some(format!( + Ok(format!( "{}{}", "try ", rewrite_block( @@ -384,8 +374,7 @@ pub(crate) fn format_expr( None, context, Shape::legacy(budget, shape.indent) - ) - .ok()? + )? )) } } @@ -403,11 +392,11 @@ pub(crate) fn format_expr( None, shape, ) { - rw.ok() + rw } else { // 6 = `async ` let budget = shape.width.saturating_sub(6); - Some(format!( + Ok(format!( "{kind} {mover}{}", rewrite_block( block, @@ -415,12 +404,11 @@ pub(crate) fn format_expr( None, context, Shape::legacy(budget, shape.indent) - ) - .ok()? + )? )) } } - ast::ExprKind::Underscore => Some("_".to_owned()), + ast::ExprKind::Underscore => Ok("_".to_owned()), ast::ExprKind::FormatArgs(..) | ast::ExprKind::Type(..) | ast::ExprKind::IncludedBytes(..) @@ -429,22 +417,21 @@ pub(crate) fn format_expr( // rustfmt tries to parse macro arguments when formatting macros, so it's not totally // impossible for rustfmt to come across one of these nodes when formatting a file. // Also, rustfmt might get passed the output from `-Zunpretty=expanded`. - None + Err(RewriteError::Unknown) } - ast::ExprKind::Err(_) | ast::ExprKind::Dummy => None, + ast::ExprKind::Err(_) | ast::ExprKind::Dummy => Err(RewriteError::Unknown), }; expr_rw .map(|expr_str| recover_comment_removed(expr_str, expr.span, context)) .and_then(|expr_str| { let attrs = outer_attributes(&expr.attrs); - let attrs_str = attrs.rewrite(context, shape)?; + let attrs_str = attrs.rewrite_result(context, shape)?; let span = mk_sp( attrs.last().map_or(expr.span.lo(), |attr| attr.span.hi()), expr.span.lo(), ); combine_strs_with_missing_comments(context, &attrs_str, &expr_str, span, shape, false) - .ok() }) } @@ -1184,7 +1171,6 @@ impl<'a> Rewrite for ControlFlow<'a> { ..shape }; format_expr(else_block, ExprType::Statement, context, else_shape) - .unknown_error() } }; diff --git a/src/ignore_path.rs b/src/ignore_path.rs index 5c25f233ce3..2f804ef5a25 100644 --- a/src/ignore_path.rs +++ b/src/ignore_path.rs @@ -41,8 +41,11 @@ mod test { use crate::ignore_path::IgnorePathSet; use std::path::{Path, PathBuf}; - let config = - Config::from_toml(r#"ignore = ["foo.rs", "bar_dir/*"]"#, Path::new("")).unwrap(); + let config = Config::from_toml( + r#"ignore = ["foo.rs", "bar_dir/*"]"#, + Path::new("./rustfmt.toml"), + ) + .unwrap(); let ignore_path_set = IgnorePathSet::from_ignore_list(&config.ignore()).unwrap(); assert!(ignore_path_set.is_match(&FileName::Real(PathBuf::from("src/foo.rs")))); @@ -59,7 +62,7 @@ mod test { let config = Config::from_toml( r#"ignore = ["foo.rs", "bar_dir/*", "!bar_dir/*/what.rs"]"#, - Path::new(""), + Path::new("./rustfmt.toml"), ) .unwrap(); let ignore_path_set = IgnorePathSet::from_ignore_list(&config.ignore()).unwrap(); diff --git a/src/matches.rs b/src/matches.rs index 8de92eb5538..ee631fa1b47 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -516,7 +516,7 @@ fn rewrite_match_body( .offset_left(extra_offset(pats_str, shape) + 4) .and_then(|shape| shape.sub_width(comma.len())); let orig_body = if forbid_same_line || !arrow_comment.is_empty() { - None + Err(RewriteError::Unknown) } else if let Some(body_shape) = orig_body_shape { let rewrite = nop_block_collapse( format_expr(body, ExprType::Statement, context, body_shape), @@ -524,7 +524,7 @@ fn rewrite_match_body( ); match rewrite { - Some(ref body_str) + Ok(ref body_str) if is_block || (!body_str.contains('\n') && unicode_str_width(body_str) <= body_shape.width) => @@ -534,7 +534,7 @@ fn rewrite_match_body( _ => rewrite, } } else { - None + Err(RewriteError::Unknown) }; let orig_budget = orig_body_shape.map_or(0, |shape| shape.width); @@ -545,20 +545,23 @@ fn rewrite_match_body( next_line_body_shape.width, ); match (orig_body, next_line_body) { - (Some(ref orig_str), Some(ref next_line_str)) + (Ok(ref orig_str), Ok(ref next_line_str)) if prefer_next_line(orig_str, next_line_str, RhsTactics::Default) => { combine_next_line_body(next_line_str) } - (Some(ref orig_str), _) if extend && first_line_width(orig_str) <= orig_budget => { + (Ok(ref orig_str), _) if extend && first_line_width(orig_str) <= orig_budget => { combine_orig_body(orig_str) } - (Some(ref orig_str), Some(ref next_line_str)) if orig_str.contains('\n') => { + (Ok(ref orig_str), Ok(ref next_line_str)) if orig_str.contains('\n') => { combine_next_line_body(next_line_str) } - (None, Some(ref next_line_str)) => combine_next_line_body(next_line_str), - (None, None) => Err(RewriteError::Unknown), - (Some(ref orig_str), _) => combine_orig_body(orig_str), + (Err(_), Ok(ref next_line_str)) => combine_next_line_body(next_line_str), + // When both orig_body and next_line_body result in errors, we currently propagate the + // error from the second attempt since it is more generous with width constraints. + // This decision is somewhat arbitrary and is open to change. + (Err(_), Err(next_line_err)) => Err(next_line_err), + (Ok(ref orig_str), _) => combine_orig_body(orig_str), } } @@ -605,7 +608,7 @@ fn rewrite_guard( } } -fn nop_block_collapse(block_str: Option, budget: usize) -> Option { +fn nop_block_collapse(block_str: RewriteResult, budget: usize) -> RewriteResult { debug!("nop_block_collapse {:?} {}", block_str, budget); block_str.map(|block_str| { if block_str.starts_with('{') diff --git a/src/parse/session.rs b/src/parse/session.rs index 4160e02b8f8..9f27a05939e 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -395,7 +395,9 @@ mod tests { } fn get_ignore_list(config: &str) -> IgnoreList { - Config::from_toml(config, Path::new("")).unwrap().ignore() + Config::from_toml(config, Path::new("./rustfmt.toml")) + .unwrap() + .ignore() } #[test] diff --git a/src/stmt.rs b/src/stmt.rs index 426bf89fc16..2788159018d 100644 --- a/src/stmt.rs +++ b/src/stmt.rs @@ -135,9 +135,7 @@ fn format_stmt( let shape = shape .sub_width(suffix.len()) .max_width_error(shape.width, ex.span())?; - format_expr(ex, expr_type, context, shape) - .map(|s| s + suffix) - .unknown_error() + format_expr(ex, expr_type, context, shape).map(|s| s + suffix) } ast::StmtKind::MacCall(..) | ast::StmtKind::Item(..) | ast::StmtKind::Empty => { Err(RewriteError::Unknown) diff --git a/src/types.rs b/src/types.rs index 5477942f82e..1aa3f60f868 100644 --- a/src/types.rs +++ b/src/types.rs @@ -584,6 +584,10 @@ fn rewrite_bounded_lifetime( impl Rewrite for ast::AnonConst { fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { + self.rewrite_result(context, shape).ok() + } + + fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult { format_expr(&self.value, ExprType::SubExpression, context, shape) } } diff --git a/src/utils.rs b/src/utils.rs index be21e89f760..d1cfc6acc49 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -367,14 +367,6 @@ macro_rules! out_of_file_lines_range { }; } -macro_rules! skip_out_of_file_lines_range { - ($self:ident, $span:expr) => { - if out_of_file_lines_range!($self, $span) { - return None; - } - }; -} - macro_rules! skip_out_of_file_lines_range_err { ($self:ident, $span:expr) => { if out_of_file_lines_range!($self, $span) { diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs index 04aae6034c5..a9f58b9328e 100644 --- a/tests/rustfmt/main.rs +++ b/tests/rustfmt/main.rs @@ -219,13 +219,16 @@ fn rustfmt_generates_no_error_if_failed_format_code_in_doc_comments() { assert!(stderr.is_empty()); assert!(stdout.is_empty()); } + #[test] fn rustfmt_error_improvement_regarding_invalid_toml() { - let args = ["--config-path", "tests/config/issue-6302.toml"]; + // See also https://github.com/rust-lang/rustfmt/issues/6302 + let invalid_toml_config = "tests/config/issue-6302.toml"; + let args = ["--config-path", invalid_toml_config]; let (_stdout, stderr) = rustfmt(&args); - let expected_error_message = "The file `rustfmt/tests/config/issue-6302.toml` failed to parse.\n\ - Error details: invalid type: integer `2019`, expected string in `edition`"; + let toml_path = Path::new(invalid_toml_config).canonicalize().unwrap(); + let expected_error_message = format!("The file `{}` failed to parse", toml_path.display()); - assert!(stderr.contains(expected_error_message), "Unexpected error message.\nExpected:\n{}\nGot:\n{}", expected_error_message, stderr); + assert!(stderr.contains(&expected_error_message)); }