Skip to content

Commit

Permalink
Backport: Do not touch module with #![rustfmt::skip] (4297)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ytmimi authored and calebcartwright committed Dec 8, 2021
1 parent 8da8371 commit f40b1d9
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 25 deletions.
52 changes: 41 additions & 11 deletions src/formatting.rs
Expand Up @@ -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;
Expand All @@ -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};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)?;
}
Expand Down
21 changes: 13 additions & 8 deletions src/test/configuration_snippet.rs
Expand Up @@ -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!(
Expand All @@ -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!(
Expand Down Expand Up @@ -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));

Expand Down
9 changes: 9 additions & 0 deletions src/test/mod_resolver.rs
Expand Up @@ -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"],
);
}
13 changes: 7 additions & 6 deletions src/visitor.rs
Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions tests/mod-resolver/skip-files-issue-5065/foo.rs
@@ -0,0 +1,5 @@
#![rustfmt::skip]

mod bar {

mod baz;}
1 change: 1 addition & 0 deletions tests/mod-resolver/skip-files-issue-5065/foo/bar/baz.rs
@@ -0,0 +1 @@
fn baz() { }
9 changes: 9 additions & 0 deletions 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
1 change: 1 addition & 0 deletions tests/mod-resolver/skip-files-issue-5065/one.rs
@@ -0,0 +1 @@
struct One { value: String }
7 changes: 7 additions & 0 deletions tests/target/skip/preserve_trailing_comment.rs
@@ -0,0 +1,7 @@
#![rustfmt::skip]

fn main() {
println!("Hello, world!");
}

// Trailing Comment

0 comments on commit f40b1d9

Please sign in to comment.