Merge remote-tracking branch 'upstream/master' into subtree-sync-2021-12-19

calebcartwright · calebcartwright · commit 8bf82aeb830d · 2021-12-19T15:22:45.000-06:00
diff --git 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
@@ -2003,9 +2003,7 @@ fn choose_rhs<R: Rewrite>(
     has_rhs_comment: bool,
 ) -> Option<String> {
     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
@@ -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<T: FormatHandler>(
+    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<T: FormatHandler>(
     input: Input,
@@ -97,23 +131,19 @@ fn format_project<T: FormatHandler>(
         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::<Vec<_>>();
 
     timer = timer.done_parsing();
 
     // Suppress error output if we have to do any further parsing.
     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
@@ -1535,15 +1535,15 @@ 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)
         }
         (Item(_) | AssocTraitItem(_) | ForeignItem(_), None) => {
             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
@@ -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
@@ -318,19 +318,20 @@ 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');
             fields_str.push_str(&nested_shape.indent.to_string(context.config));
         } 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
@@ -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<T: Write>(&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
@@ -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
@@ -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
@@ -0,0 +1,5 @@
+#![rustfmt::skip]
+
+mod bar {
+
+        mod baz;}
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
@@ -0,0 +1 @@
+fn    baz()    {       }
diff --git a/tests/mod-resolver/skip-files-issue-5065/main.rs 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
@@ -0,0 +1 @@
+struct One {   value:    String }
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,7 @@
+#![rustfmt::skip]
+
+fn main() {
+    println!("Hello, world!");
+}
+
+// Trailing Comment

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ fn main() {`
`26`	`26`	`let exit_code = match execute(&opts) {`
`27`	`27`	`Ok(code) => code,`
`28`	`28`	`Err(e) => {`
`29`		`- eprintln!("{}", e.to_string());`
	`29`	`+ eprintln!("{}", e);`
`30`	`30`	`1`
`31`	`31`	`}`
`32`	`32`	`};`