diff --git a/src/bin/main.rs b/src/bin/main.rs index 9d2e97c9479f..4d845547cdfe 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -26,7 +26,7 @@ fn main() { let exit_code = match execute(&opts) { Ok(code) => code, Err(e) => { - eprintln!("{}", e.to_string()); + eprintln!("{}", e); 1 } }; diff --git a/src/expr.rs b/src/expr.rs index 5fd86c1a4ead..edd004ac63f0 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -2003,9 +2003,7 @@ fn choose_rhs( has_rhs_comment: bool, ) -> Option { match orig_rhs { - Some(ref new_str) if new_str.is_empty() => { - return Some(String::new()); - } + Some(ref new_str) if new_str.is_empty() => Some(String::new()), Some(ref new_str) if !new_str.contains('\n') && unicode_str_width(new_str) <= shape.width => { diff --git a/src/formatting.rs b/src/formatting.rs index 7d0facb8f12c..b39480a0ef90 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -5,6 +5,7 @@ use std::io::{self, Write}; use std::time::{Duration, Instant}; use rustc_ast::ast; +use rustc_ast::AstLike; use rustc_span::Span; use self::newline_style::apply_newline_style; @@ -15,7 +16,7 @@ use crate::issues::BadIssueSeeker; use crate::modules::Module; use crate::syntux::parser::{DirectoryOwnership, Parser, ParserError}; use crate::syntux::session::ParseSess; -use crate::utils::count_newlines; +use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session}; @@ -58,6 +59,39 @@ impl<'b, T: Write + 'b> Session<'b, T> { } } +/// Determine if a module should be skipped. True if the module should be skipped, false otherwise. +fn should_skip_module( + config: &Config, + context: &FormatContext<'_, T>, + input_is_stdin: bool, + main_file: &FileName, + path: &FileName, + module: &Module<'_>, +) -> bool { + if contains_skip(module.attrs()) { + return true; + } + + if config.skip_children() && path != main_file { + return true; + } + + if !input_is_stdin && context.ignore_file(path) { + return true; + } + + if !config.format_generated_files() { + let source_file = context.parse_session.span_to_file_contents(module.span); + let src = source_file.src.as_ref().expect("SourceFile without src"); + + if is_generated_file(src) { + return true; + } + } + + false +} + // Format an entire crate (or subset of the module tree). fn format_project( input: Input, @@ -97,7 +131,12 @@ fn format_project( directory_ownership.unwrap_or(DirectoryOwnership::UnownedViaBlock), !input_is_stdin && !config.skip_children(), ) - .visit_crate(&krate)?; + .visit_crate(&krate)? + .into_iter() + .filter(|(path, module)| { + !should_skip_module(config, &context, input_is_stdin, &main_file, path, module) + }) + .collect::>(); timer = timer.done_parsing(); @@ -105,15 +144,6 @@ fn format_project( context.parse_session.set_silent_emitter(); for (path, module) in files { - let source_file = context.parse_session.span_to_file_contents(module.span); - let src = source_file.src.as_ref().expect("SourceFile without src"); - - let should_ignore = (!input_is_stdin && context.ignore_file(&path)) - || (!config.format_generated_files() && is_generated_file(src)); - - if (config.skip_children() && path != main_file) || should_ignore { - continue; - } should_emit_verbose(input_is_stdin, config, || println!("Formatting {}", path)); context.format_file(path, &module, is_macro_def)?; } diff --git a/src/items.rs b/src/items.rs index b7dd6b06ff82..babc56f86edc 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1535,7 +1535,7 @@ pub(crate) fn rewrite_type_alias<'a, 'b>( // https://rustc-dev-guide.rust-lang.org/opaque-types-type-alias-impl-trait.html // https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/items.md#type-aliases match (visitor_kind, &op_ty) { - (Item(_) | AssocTraitItem(_) | ForeignItem(_), Some(ref op_bounds)) => { + (Item(_) | AssocTraitItem(_) | ForeignItem(_), Some(op_bounds)) => { let op = OpaqueType { bounds: op_bounds }; rewrite_ty(rw_info, Some(bounds), Some(&op), vis) } @@ -1543,7 +1543,7 @@ pub(crate) fn rewrite_type_alias<'a, 'b>( rewrite_ty(rw_info, Some(bounds), ty_opt, vis) } (AssocImplItem(_), _) => { - let result = if let Some(ref op_bounds) = op_ty { + let result = if let Some(op_bounds) = op_ty { let op = OpaqueType { bounds: op_bounds }; rewrite_ty(rw_info, Some(bounds), Some(&op), &DEFAULT_VISIBILITY) } else { @@ -3124,7 +3124,7 @@ impl Rewrite for ast::ForeignItem { let inner_attrs = inner_attributes(&self.attrs); let fn_ctxt = visit::FnCtxt::Foreign; visitor.visit_fn( - visit::FnKind::Fn(fn_ctxt, self.ident, &sig, &self.vis, Some(body)), + visit::FnKind::Fn(fn_ctxt, self.ident, sig, &self.vis, Some(body)), generics, &sig.decl, self.span, @@ -3137,7 +3137,7 @@ impl Rewrite for ast::ForeignItem { context, shape.indent, self.ident, - &FnSig::from_method_sig(&sig, generics, &self.vis), + &FnSig::from_method_sig(sig, generics, &self.vis), span, FnBraceStyle::None, ) @@ -3166,7 +3166,7 @@ impl Rewrite for ast::ForeignItem { .map(|s| s + ";") } ast::ForeignItemKind::TyAlias(ref ty_alias) => { - let (kind, span) = (&ItemVisitorKind::ForeignItem(&self), self.span); + let (kind, span) = (&ItemVisitorKind::ForeignItem(self), self.span); rewrite_type_alias(ty_alias, context, shape.indent, kind, span) } ast::ForeignItemKind::MacCall(ref mac) => { diff --git a/src/lists.rs b/src/lists.rs index 3515dd172510..7aa0315f18c2 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -448,10 +448,8 @@ where true } else if starts_with_newline(comment) { false - } else if comment.trim().contains('\n') || comment.trim().len() > width { - true } else { - false + comment.trim().contains('\n') || comment.trim().len() > width }; rewrite_comment( diff --git a/src/patterns.rs b/src/patterns.rs index a80d63201f98..9b74b35f3141 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -318,10 +318,12 @@ fn rewrite_struct_pat( let mut fields_str = write_list(&item_vec, &fmt)?; let one_line_width = h_shape.map_or(0, |shape| shape.width); + let has_trailing_comma = fmt.needs_trailing_separator(); + if ellipsis { if fields_str.contains('\n') || fields_str.len() > one_line_width { // Add a missing trailing comma. - if context.config.trailing_comma() == SeparatorTactic::Never { + if !has_trailing_comma { fields_str.push(','); } fields_str.push('\n'); @@ -329,8 +331,7 @@ fn rewrite_struct_pat( } else { if !fields_str.is_empty() { // there are preceding struct fields being matched on - if tactic == DefinitiveListTactic::Vertical { - // if the tactic is Vertical, write_list already added a trailing , + if has_trailing_comma { fields_str.push(' '); } else { fields_str.push_str(", "); diff --git a/src/test/configuration_snippet.rs b/src/test/configuration_snippet.rs index ef7dd0ddcd12..92949ab576a6 100644 --- a/src/test/configuration_snippet.rs +++ b/src/test/configuration_snippet.rs @@ -110,14 +110,7 @@ impl ConfigCodeBlock { assert!(self.code_block.is_some() && self.code_block_start.is_some()); // See if code block begins with #![rustfmt::skip]. - let fmt_skip = self - .code_block - .as_ref() - .unwrap() - .lines() - .nth(0) - .unwrap_or("") - == "#![rustfmt::skip]"; + let fmt_skip = self.fmt_skip(); if self.config_name.is_none() && !fmt_skip { write_message(&format!( @@ -138,6 +131,17 @@ impl ConfigCodeBlock { true } + /// True if the code block starts with #![rustfmt::skip] + fn fmt_skip(&self) -> bool { + self.code_block + .as_ref() + .unwrap() + .lines() + .nth(0) + .unwrap_or("") + == "#![rustfmt::skip]" + } + fn has_parsing_errors(&self, session: &Session<'_, T>) -> bool { if session.has_parsing_errors() { write_message(&format!( @@ -251,6 +255,7 @@ fn configuration_snippet_tests() { let blocks = get_code_blocks(); let failures = blocks .iter() + .filter(|block| !block.fmt_skip()) .map(ConfigCodeBlock::formatted_is_idempotent) .fold(0, |acc, r| acc + (!r as u32)); diff --git a/src/test/mod_resolver.rs b/src/test/mod_resolver.rs index ae4a0d0fccb1..ec9ed0f0b8d6 100644 --- a/src/test/mod_resolver.rs +++ b/src/test/mod_resolver.rs @@ -41,3 +41,12 @@ fn out_of_line_nested_inline_within_out_of_line() { ], ); } + +#[test] +fn skip_out_of_line_nested_inline_within_out_of_line() { + // See also https://github.com/rust-lang/rustfmt/issues/5065 + verify_mod_resolution( + "tests/mod-resolver/skip-files-issue-5065/main.rs", + &["tests/mod-resolver/skip-files-issue-5065/one.rs"], + ); +} diff --git a/src/visitor.rs b/src/visitor.rs index e4a7be742abc..1896a4744fe4 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -552,7 +552,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { _ => visit::FnCtxt::Foreign, }; self.visit_fn( - visit::FnKind::Fn(fn_ctxt, item.ident, &sig, &item.vis, Some(body)), + visit::FnKind::Fn(fn_ctxt, item.ident, sig, &item.vis, Some(body)), generics, &sig.decl, item.span, @@ -562,14 +562,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } else { let indent = self.block_indent; let rewrite = self.rewrite_required_fn( - indent, item.ident, &sig, &item.vis, generics, item.span, + indent, item.ident, sig, &item.vis, generics, item.span, ); self.push_rewrite(item.span, rewrite); } } ast::ItemKind::TyAlias(ref ty_alias) => { use ItemVisitorKind::Item; - self.visit_ty_alias_kind(ty_alias, &Item(&item), item.span); + self.visit_ty_alias_kind(ty_alias, &Item(item), item.span); } ast::ItemKind::GlobalAsm(..) => { let snippet = Some(self.snippet(item.span).to_owned()); @@ -619,17 +619,17 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { skip_out_of_file_lines_range_visitor!(self, ai.span); if self.visit_attrs(&ai.attrs, ast::AttrStyle::Outer) { - self.push_skipped_with_span(&ai.attrs.as_slice(), skip_span, skip_span); + self.push_skipped_with_span(ai.attrs.as_slice(), skip_span, skip_span); return; } // TODO(calebcartwright): consider enabling box_patterns feature gate match (&ai.kind, visitor_kind) { (ast::AssocItemKind::Const(..), AssocTraitItem(_)) => { - self.visit_static(&StaticParts::from_trait_item(&ai)) + self.visit_static(&StaticParts::from_trait_item(ai)) } (ast::AssocItemKind::Const(..), AssocImplItem(_)) => { - self.visit_static(&StaticParts::from_impl_item(&ai)) + self.visit_static(&StaticParts::from_impl_item(ai)) } (ast::AssocItemKind::Fn(ref fn_kind), _) => { let ast::Fn { @@ -948,12 +948,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { pub(crate) fn format_separate_mod(&mut self, m: &Module<'_>, end_pos: BytePos) { self.block_indent = Indent::empty(); - if self.visit_attrs(m.attrs(), ast::AttrStyle::Inner) { - self.push_skipped_with_span(m.attrs(), m.span, m.span); - } else { - self.walk_mod_items(&m.items); - self.format_missing_with_indent(end_pos); - } + let skipped = self.visit_attrs(m.attrs(), ast::AttrStyle::Inner); + assert!( + !skipped, + "Skipping module must be handled before reaching this line." + ); + self.walk_mod_items(&m.items); + self.format_missing_with_indent(end_pos); } pub(crate) fn skip_empty_lines(&mut self, end_pos: BytePos) { diff --git a/tests/mod-resolver/skip-files-issue-5065/foo.rs b/tests/mod-resolver/skip-files-issue-5065/foo.rs new file mode 100644 index 000000000000..74889acf0c38 --- /dev/null +++ b/tests/mod-resolver/skip-files-issue-5065/foo.rs @@ -0,0 +1,5 @@ +#![rustfmt::skip] + +mod bar { + + mod baz;} \ No newline at end of file diff --git a/tests/mod-resolver/skip-files-issue-5065/foo/bar/baz.rs b/tests/mod-resolver/skip-files-issue-5065/foo/bar/baz.rs new file mode 100644 index 000000000000..3519b0ee59c8 --- /dev/null +++ b/tests/mod-resolver/skip-files-issue-5065/foo/bar/baz.rs @@ -0,0 +1 @@ +fn baz() { } \ No newline at end of file diff --git a/tests/mod-resolver/skip-files-issue-5065/main.rs b/tests/mod-resolver/skip-files-issue-5065/main.rs new file mode 100644 index 000000000000..3122e4f220f6 --- /dev/null +++ b/tests/mod-resolver/skip-files-issue-5065/main.rs @@ -0,0 +1,9 @@ +#![rustfmt::skip] + +mod foo; +mod one; + +fn main() {println!("Hello, world!"); +} + +// trailing commet diff --git a/tests/mod-resolver/skip-files-issue-5065/one.rs b/tests/mod-resolver/skip-files-issue-5065/one.rs new file mode 100644 index 000000000000..e7eb2c2d64dd --- /dev/null +++ b/tests/mod-resolver/skip-files-issue-5065/one.rs @@ -0,0 +1 @@ +struct One { value: String } \ No newline at end of file diff --git a/tests/target/issue-5066/multi_line_struct_trailing_comma_always_struct_lit_width_0.rs b/tests/target/issue-5066/multi_line_struct_trailing_comma_always_struct_lit_width_0.rs new file mode 100644 index 000000000000..c7122c676237 --- /dev/null +++ b/tests/target/issue-5066/multi_line_struct_trailing_comma_always_struct_lit_width_0.rs @@ -0,0 +1,10 @@ +// rustfmt-trailing_comma: Always +// rustfmt-struct_lit_single_line: false +// rustfmt-struct_lit_width: 0 + +fn main() { + let Foo { + a, + .. + } = b; +} diff --git a/tests/target/issue-5066/multi_line_struct_trailing_comma_never_struct_lit_width_0.rs b/tests/target/issue-5066/multi_line_struct_trailing_comma_never_struct_lit_width_0.rs new file mode 100644 index 000000000000..68e89c4179f7 --- /dev/null +++ b/tests/target/issue-5066/multi_line_struct_trailing_comma_never_struct_lit_width_0.rs @@ -0,0 +1,10 @@ +// rustfmt-trailing_comma: Never +// rustfmt-struct_lit_single_line: false +// rustfmt-struct_lit_width: 0 + +fn main() { + let Foo { + a, + .. + } = b; +} diff --git a/tests/target/issue-5066/multi_line_struct_with_trailing_comma_always.rs b/tests/target/issue-5066/multi_line_struct_with_trailing_comma_always.rs new file mode 100644 index 000000000000..3368f0703868 --- /dev/null +++ b/tests/target/issue-5066/multi_line_struct_with_trailing_comma_always.rs @@ -0,0 +1,10 @@ +// rustfmt-trailing_comma: Always +// rustfmt-struct_lit_single_line: false + +// There is an issue with how this is formatted. +// formatting should look like ./multi_line_struct_trailing_comma_always_struct_lit_width_0.rs +fn main() { + let Foo { + a, .. + } = b; +} diff --git a/tests/target/issue-5066/multi_line_struct_with_trailing_comma_never.rs b/tests/target/issue-5066/multi_line_struct_with_trailing_comma_never.rs new file mode 100644 index 000000000000..cf63c4c983c4 --- /dev/null +++ b/tests/target/issue-5066/multi_line_struct_with_trailing_comma_never.rs @@ -0,0 +1,10 @@ +// rustfmt-trailing_comma: Never +// rustfmt-struct_lit_single_line: false + +// There is an issue with how this is formatted. +// formatting should look like ./multi_line_struct_trailing_comma_never_struct_lit_width_0.rs +fn main() { + let Foo { + a, .. + } = b; +} diff --git a/tests/target/issue-5066/with_trailing_comma_always.rs b/tests/target/issue-5066/with_trailing_comma_always.rs new file mode 100644 index 000000000000..e20bcec93169 --- /dev/null +++ b/tests/target/issue-5066/with_trailing_comma_always.rs @@ -0,0 +1,5 @@ +// rustfmt-trailing_comma: Always + +fn main() { + let Foo { a, .. } = b; +} diff --git a/tests/target/issue-5066/with_trailing_comma_never.rs b/tests/target/issue-5066/with_trailing_comma_never.rs new file mode 100644 index 000000000000..8b95bb137bca --- /dev/null +++ b/tests/target/issue-5066/with_trailing_comma_never.rs @@ -0,0 +1,5 @@ +// rustfmt-trailing_comma: Never + +fn main() { + let Foo { a, .. } = b; +} diff --git a/tests/target/skip/preserve_trailing_comment.rs b/tests/target/skip/preserve_trailing_comment.rs new file mode 100644 index 000000000000..f85de33257cc --- /dev/null +++ b/tests/target/skip/preserve_trailing_comment.rs @@ -0,0 +1,7 @@ +#![rustfmt::skip] + +fn main() { + println!("Hello, world!"); +} + +// Trailing Comment