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

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

// Trailing Comment

2 comments on commit f40b1d9

@laa774gma
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laa774gma
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.