Backport: Do not touch module with #![rustfmt::skip] (4297)

Although the implementation is slightly different than the original PR,
the general idea is the same. After collecting all modules we want to
exclude formatting those that contain the #![rustfmt::skip] attribute.

Author: ytmimi

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,32 @@ 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;
+    }
+
+    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 {
+        return true;
+    }
+
+    false
+}
+
 // Format an entire crate (or subset of the module tree).
 fn format_project<T: FormatHandler>(
     input: Input,
@@ -97,23 +124,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)?;
     }

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));
 

src/test/mod_resolver.rs

@@ -41,3 +41,14 @@ 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",
+        ]
+    );
+}

src/visitor.rs

@@ -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) {

tests/target/skip/preserve_trailing_comment.rs

@@ -0,0 +1,7 @@
+#![rustfmt::skip]
+
+fn main() {
+    println!("Hello, world!");
+}
+
+// Trailing Comment