Skip to content

Fix handling of code that is annotated with rustfmt::skip. #3113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 6 additions & 47 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use config::Config;
use rewrite::RewriteContext;
use shape::{Indent, Shape};
use string::{rewrite_string, StringFormat};
use utils::{count_newlines, first_line_width, last_line_width};
use utils::{count_newlines, first_line_width, last_line_width, trim_left_preserve_layout};
use {ErrorKind, FormattingError};

fn is_custom_comment(comment: &str) -> bool {
Expand Down Expand Up @@ -332,12 +332,12 @@ fn identify_comment(
let (first_group, rest) = orig.split_at(first_group_ending);
let rewritten_first_group =
if !config.normalize_comments() && has_bare_lines && style.is_block_comment() {
light_rewrite_block_comment_with_bare_lines(first_group, shape, config)?
trim_left_preserve_layout(first_group, &shape.indent, config)
} else if !config.normalize_comments()
&& !config.wrap_comments()
&& !config.format_doc_comments()
{
light_rewrite_comment(first_group, shape.indent, config, is_doc_comment)?
light_rewrite_comment(first_group, shape.indent, config, is_doc_comment)
} else {
rewrite_comment_inner(
first_group,
Expand Down Expand Up @@ -370,47 +370,6 @@ fn identify_comment(
}
}

/// Trims a minimum of leading whitespaces so that the content layout is kept and aligns to indent.
fn light_rewrite_block_comment_with_bare_lines(
orig: &str,
shape: Shape,
config: &Config,
) -> Option<String> {
let prefix_whitespace_min = orig
.lines()
// skip the line with the starting sigil since the leading whitespace is removed
// otherwise, the minimum would always be zero
.skip(1)
.filter(|line| !line.is_empty())
.map(|line| {
let mut width = 0;
for c in line.chars() {
match c {
' ' => width += 1,
'\t' => width += config.tab_spaces(),
_ => break,
}
}
width
})
.min()?;

let indent_str = shape.indent.to_string(config);
let mut lines = orig.lines();
let first_line = lines.next()?;
let rest = lines
.map(|line| {
if line.is_empty() {
line
} else {
&line[prefix_whitespace_min..]
}
})
.collect::<Vec<&str>>()
.join(&format!("\n{}", indent_str));
Some(format!("{}\n{}{}", first_line, indent_str, rest))
}

/// Attributes for code blocks in rustdoc.
/// See https://doc.rust-lang.org/rustdoc/print.html#attributes
enum CodeBlockAttribute {
Expand Down Expand Up @@ -661,7 +620,7 @@ impl<'a> CommentRewrite<'a> {
let mut config = self.fmt.config.clone();
config.set().format_doc_comments(false);
match ::format_code_block(&self.code_block_buffer, &config) {
Some(ref s) => trim_custom_comment_prefix(s),
Some(ref s) => trim_custom_comment_prefix(&s.snippet),
None => trim_custom_comment_prefix(&self.code_block_buffer),
}
}
Expand Down Expand Up @@ -912,7 +871,7 @@ fn light_rewrite_comment(
offset: Indent,
config: &Config,
is_doc_comment: bool,
) -> Option<String> {
) -> String {
let lines: Vec<&str> = orig
.lines()
.map(|l| {
Expand All @@ -933,7 +892,7 @@ fn light_rewrite_comment(
trim_right_unless_two_whitespaces(left_trimmed, is_doc_comment)
})
.collect();
Some(lines.join(&format!("\n{}", offset.to_string(config))))
lines.join(&format!("\n{}", offset.to_string(config)))
}

/// Trims comment characters and possibly a single space from the left of a string.
Expand Down
2 changes: 2 additions & 0 deletions src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
if visitor.macro_rewrite_failure {
self.report.add_macro_format_failure();
}
self.report
.add_non_formatted_ranges(visitor.skipped_range.clone());

self.handler
.handle_formatted_file(path, visitor.buffer.to_owned(), &mut self.report)
Expand Down
83 changes: 66 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,22 +144,56 @@ impl From<io::Error> for ErrorKind {
}
}

/// Result of formatting a snippet of code along with ranges of lines that didn't get formatted,
/// i.e., that got returned as they were originally.
#[derive(Debug)]
struct FormattedSnippet {
snippet: String,
non_formatted_ranges: Vec<(usize, usize)>,
}

impl FormattedSnippet {
/// In case the snippet needed to be wrapped in a function, this shifts down the ranges of
/// non-formatted code.
fn unwrap_code_block(&mut self) {
self.non_formatted_ranges
.iter_mut()
.for_each(|(low, high)| {
*low -= 1;
*high -= 1;
});
}

/// Returns true if the line n did not get formatted.
fn is_line_non_formatted(&self, n: usize) -> bool {
self.non_formatted_ranges
.iter()
.any(|(low, high)| *low <= n && n <= *high)
}
}

/// Reports on any issues that occurred during a run of Rustfmt.
///
/// Can be reported to the user via its `Display` implementation of `print_fancy`.
#[derive(Clone)]
pub struct FormatReport {
// Maps stringified file paths to their associated formatting errors.
internal: Rc<RefCell<(FormatErrorMap, ReportedErrors)>>,
non_formatted_ranges: Vec<(usize, usize)>,
}

impl FormatReport {
fn new() -> FormatReport {
FormatReport {
internal: Rc::new(RefCell::new((HashMap::new(), ReportedErrors::default()))),
non_formatted_ranges: Vec::new(),
}
}

fn add_non_formatted_ranges(&mut self, mut ranges: Vec<(usize, usize)>) {
self.non_formatted_ranges.append(&mut ranges);
}

fn append(&self, f: FileName, mut v: Vec<FormattingError>) {
self.track_errors(&v);
self.internal
Expand Down Expand Up @@ -349,37 +383,44 @@ impl fmt::Display for FormatReport {

/// Format the given snippet. The snippet is expected to be *complete* code.
/// When we cannot parse the given snippet, this function returns `None`.
fn format_snippet(snippet: &str, config: &Config) -> Option<String> {
fn format_snippet(snippet: &str, config: &Config) -> Option<FormattedSnippet> {
let mut config = config.clone();
let out = panic::catch_unwind(|| {
panic::catch_unwind(|| {
let mut out: Vec<u8> = Vec::with_capacity(snippet.len() * 2);
config.set().emit_mode(config::EmitMode::Stdout);
config.set().verbose(Verbosity::Quiet);
config.set().hide_parse_errors(true);
let formatting_error = {

let (formatting_error, result) = {
let input = Input::Text(snippet.into());
let mut session = Session::new(config, Some(&mut out));
let result = session.format(input);
session.errors.has_macro_format_failure
|| session.out.as_ref().unwrap().is_empty() && !snippet.is_empty()
|| result.is_err()
(
session.errors.has_macro_format_failure
|| session.out.as_ref().unwrap().is_empty() && !snippet.is_empty()
|| result.is_err(),
result,
)
};
if formatting_error {
None
} else {
Some(out)
String::from_utf8(out).ok().map(|snippet| FormattedSnippet {
snippet,
non_formatted_ranges: result.unwrap().non_formatted_ranges,
})
}
})
.ok()??; // The first try operator handles the error from catch_unwind,
// whereas the second one handles None from the closure.
String::from_utf8(out).ok()
// Discard panics encountered while formatting the snippet
// The ? operator is needed to remove the extra Option
.ok()?
}

/// Format the given code block. Mainly targeted for code block in comment.
/// The code block may be incomplete (i.e. parser may be unable to parse it).
/// To avoid panic in parser, we wrap the code block with a dummy function.
/// The returned code block does *not* end with newline.
fn format_code_block(code_snippet: &str, config: &Config) -> Option<String> {
fn format_code_block(code_snippet: &str, config: &Config) -> Option<FormattedSnippet> {
const FN_MAIN_PREFIX: &str = "fn main() {\n";

fn enclose_in_main_block(s: &str, config: &Config) -> String {
Expand Down Expand Up @@ -412,13 +453,18 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option<String> {
config_with_unix_newline
.set()
.newline_style(NewlineStyle::Unix);
let formatted = format_snippet(&snippet, &config_with_unix_newline)?;
let mut formatted = format_snippet(&snippet, &config_with_unix_newline)?;
// Remove wrapping main block
formatted.unwrap_code_block();

// Trim "fn main() {" on the first line and "}" on the last line,
// then unindent the whole code block.
let block_len = formatted.rfind('}').unwrap_or(formatted.len());
let block_len = formatted
.snippet
.rfind('}')
.unwrap_or(formatted.snippet.len());
let mut is_indented = true;
for (kind, ref line) in LineClasses::new(&formatted[FN_MAIN_PREFIX.len()..block_len]) {
for (kind, ref line) in LineClasses::new(&formatted.snippet[FN_MAIN_PREFIX.len()..block_len]) {
if !is_first {
result.push('\n');
} else {
Expand Down Expand Up @@ -451,7 +497,10 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option<String> {
result.push_str(trimmed_line);
is_indented = !kind.is_string() || line.ends_with('\\');
}
Some(result)
Some(FormattedSnippet {
snippet: result,
non_formatted_ranges: formatted.non_formatted_ranges,
})
}

/// A session is a run of rustfmt across a single or multiple inputs.
Expand Down Expand Up @@ -571,10 +620,10 @@ mod unit_tests {

fn test_format_inner<F>(formatter: F, input: &str, expected: &str) -> bool
where
F: Fn(&str, &Config) -> Option<String>,
F: Fn(&str, &Config) -> Option<FormattedSnippet>,
{
let output = formatter(input, &Config::default());
output.is_some() && output.unwrap() == expected
output.is_some() && output.unwrap().snippet == expected
}

#[test]
Expand Down
16 changes: 12 additions & 4 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,7 @@ impl MacroBranch {
config.set().max_width(new_width);

// First try to format as items, then as statements.
let new_body = match ::format_snippet(&body_str, &config) {
let new_body_snippet = match ::format_snippet(&body_str, &config) {
Some(new_body) => new_body,
None => {
let new_width = new_width + config.tab_spaces();
Expand All @@ -1334,15 +1334,23 @@ impl MacroBranch {
}
}
};
let new_body = wrap_str(new_body, config.max_width(), shape)?;
let new_body = wrap_str(
new_body_snippet.snippet.to_string(),
config.max_width(),
shape,
)?;

// Indent the body since it is in a block.
let indent_str = body_indent.to_string(&config);
let mut new_body = LineClasses::new(new_body.trim_right())
.enumerate()
.fold(
(String::new(), true),
|(mut s, need_indent), (kind, ref l)| {
if !l.is_empty() && need_indent {
|(mut s, need_indent), (i, (kind, ref l))| {
if !l.is_empty()
&& need_indent
&& !new_body_snippet.is_line_non_formatted(i + 1)
{
s += &indent_str;
}
(s + l + "\n", !kind.is_string() || l.ends_with('\\'))
Expand Down
41 changes: 40 additions & 1 deletion src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ use syntax::ptr;
use syntax::source_map::{BytePos, Span, NO_EXPANSION};

use comment::{filter_normal_code, CharClasses, FullCodeCharKind};
use config::Config;
use rewrite::RewriteContext;
use shape::Shape;
use shape::{Indent, Shape};

pub const DEPR_SKIP_ANNOTATION: &str = "rustfmt_skip";
pub const SKIP_ANNOTATION: &str = "rustfmt::skip";
Expand Down Expand Up @@ -482,6 +483,44 @@ pub fn remove_trailing_white_spaces(text: &str) -> String {
buffer
}

/// Trims a minimum of leading whitespaces so that the content layout is kept and aligns to indent.
pub fn trim_left_preserve_layout(orig: &str, indent: &Indent, config: &Config) -> String {
let prefix_whitespace_min = orig
.lines()
// skip the line with the starting sigil since the leading whitespace is removed
// otherwise, the minimum would always be zero
.skip(1)
.filter(|line| !line.is_empty())
.map(|line| {
let mut width = 0;
for c in line.chars() {
match c {
' ' => width += 1,
'\t' => width += config.tab_spaces(),
_ => break,
}
}
width
})
.min()
.unwrap_or(0);

let indent_str = indent.to_string(config);
let mut lines = orig.lines();
let first_line = lines.next().unwrap();
let rest = lines
.map(|line| {
if line.is_empty() {
String::from("\n")
} else {
format!("\n{}{}", indent_str, &line[prefix_whitespace_min..])
}
})
.collect::<Vec<String>>()
.concat();
format!("{}{}", first_line, rest)
}

#[test]
fn test_remove_trailing_white_spaces() {
let s = " r#\"\n test\n \"#";
Expand Down
Loading