diff --git a/src/comment.rs b/src/comment.rs index 358c18e1b95..e76bb331856 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -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 { @@ -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, @@ -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 { - 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::>() - .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 { @@ -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), } } @@ -912,7 +871,7 @@ fn light_rewrite_comment( offset: Indent, config: &Config, is_doc_comment: bool, -) -> Option { +) -> String { let lines: Vec<&str> = orig .lines() .map(|l| { @@ -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. diff --git a/src/formatting.rs b/src/formatting.rs index d2748b35736..039276ba6e8 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -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) diff --git a/src/lib.rs b/src/lib.rs index 6f366f0b64a..2dcbf823d55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -144,6 +144,34 @@ impl From 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`. @@ -151,15 +179,21 @@ impl From for ErrorKind { pub struct FormatReport { // Maps stringified file paths to their associated formatting errors. internal: Rc>, + 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) { self.track_errors(&v); self.internal @@ -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 { +fn format_snippet(snippet: &str, config: &Config) -> Option { let mut config = config.clone(); - let out = panic::catch_unwind(|| { + panic::catch_unwind(|| { let mut out: Vec = 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 { +fn format_code_block(code_snippet: &str, config: &Config) -> Option { const FN_MAIN_PREFIX: &str = "fn main() {\n"; fn enclose_in_main_block(s: &str, config: &Config) -> String { @@ -412,13 +453,18 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option { 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 { @@ -451,7 +497,10 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option { 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. @@ -571,10 +620,10 @@ mod unit_tests { fn test_format_inner(formatter: F, input: &str, expected: &str) -> bool where - F: Fn(&str, &Config) -> Option, + F: Fn(&str, &Config) -> Option, { let output = formatter(input, &Config::default()); - output.is_some() && output.unwrap() == expected + output.is_some() && output.unwrap().snippet == expected } #[test] diff --git a/src/macros.rs b/src/macros.rs index 13b761029e2..d378bb8ffb1 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -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(); @@ -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('\\')) diff --git a/src/utils.rs b/src/utils.rs index d90bcc5931d..a2d15b820eb 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -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"; @@ -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::>() + .concat(); + format!("{}{}", first_line, rest) +} + #[test] fn test_remove_trailing_white_spaces() { let s = " r#\"\n test\n \"#"; diff --git a/src/visitor.rs b/src/visitor.rs index 9ad6d3ccaa8..b286465b276 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -71,6 +71,8 @@ pub struct FmtVisitor<'a> { pub is_if_else_block: bool, pub snippet_provider: &'a SnippetProvider<'a>, pub line_number: usize, + /// List of 1-based line ranges which were annotated with skip + /// Both bounds are inclusifs. pub skipped_range: Vec<(usize, usize)>, pub macro_rewrite_failure: bool, pub(crate) report: FormatReport, @@ -109,8 +111,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.format_missing(stmt.span.hi()); } ast::StmtKind::Local(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => { - if contains_skip(get_attrs_from_stmt(stmt)) { - self.push_skipped_with_span(stmt.span()); + let attrs = get_attrs_from_stmt(stmt); + if contains_skip(attrs) { + self.push_skipped_with_span(attrs, stmt.span()); } else { let shape = self.shape(); let rewrite = self.with_context(|ctx| stmt.rewrite(&ctx, shape)); @@ -120,7 +123,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { ast::StmtKind::Mac(ref mac) => { let (ref mac, _macro_style, ref attrs) = **mac; if self.visit_attrs(attrs, ast::AttrStyle::Outer) { - self.push_skipped_with_span(stmt.span()); + self.push_skipped_with_span(attrs, stmt.span()); } else { self.visit_mac(mac, None, MacroPosition::Statement); } @@ -328,14 +331,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // For use items, skip rewriting attributes. Just check for a skip attribute. ast::ItemKind::Use(..) => { if contains_skip(attrs) { - self.push_skipped_with_span(item.span()); + self.push_skipped_with_span(attrs.as_slice(), item.span()); return; } } // Module is inline, in this case we treat it like any other item. _ if !is_mod_decl(item) => { if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) { - self.push_skipped_with_span(item.span()); + self.push_skipped_with_span(item.attrs.as_slice(), item.span()); return; } } @@ -354,7 +357,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } _ => { if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) { - self.push_skipped_with_span(item.span()); + self.push_skipped_with_span(item.attrs.as_slice(), item.span()); return; } } @@ -471,7 +474,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { skip_out_of_file_lines_range_visitor!(self, ti.span); if self.visit_attrs(&ti.attrs, ast::AttrStyle::Outer) { - self.push_skipped_with_span(ti.span()); + self.push_skipped_with_span(ti.attrs.as_slice(), ti.span()); return; } @@ -515,7 +518,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { skip_out_of_file_lines_range_visitor!(self, ii.span); if self.visit_attrs(&ii.attrs, ast::AttrStyle::Outer) { - self.push_skipped_with_span(ii.span()); + self.push_skipped_with_span(ii.attrs.as_slice(), ii.span()); return; } @@ -589,10 +592,17 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_rewrite_inner(span, rewrite); } - pub fn push_skipped_with_span(&mut self, span: Span) { - self.format_missing_with_indent(source!(self, span).lo()); - let lo = self.line_number + 1; - self.push_rewrite_inner(span, None); + pub fn push_skipped_with_span(&mut self, attrs: &[ast::Attribute], item_span: Span) { + self.format_missing_with_indent(source!(self, item_span).lo()); + // do not take into account the lines with attributes as part of the skipped range + let attrs_end = attrs + .iter() + .map(|attr| self.source_map.lookup_char_pos(attr.span().hi()).line) + .max() + .unwrap_or(1); + // Add 1 to get the line past the last attribute + let lo = attrs_end + 1; + self.push_rewrite_inner(item_span, None); let hi = self.line_number + 1; self.skipped_range.push((lo, hi)); } diff --git a/tests/target/issue-3105.rs b/tests/target/issue-3105.rs new file mode 100644 index 00000000000..4f1123805b8 --- /dev/null +++ b/tests/target/issue-3105.rs @@ -0,0 +1,48 @@ +// rustfmt-wrap_comments: true + +/// Although the indentation of the skipped method is off, it shouldn't be +/// changed. +/// +/// ``` +/// pub unsafe fn _mm256_shufflehi_epi16(a: __m256i, imm8: i32) -> __m256i { +/// let imm8 = (imm8 & 0xFF) as u8; +/// let a = a.as_i16x16(); +/// macro_rules! shuffle_done { +/// ($x01:expr, $x23:expr, $x45:expr, $x67:expr) => { +/// #[cfg_attr(rustfmt, rustfmt_skip)] +/// simd_shuffle16(a, a, [ +/// 0, 1, 2, 3, 4+$x01, 4+$x23, 4+$x45, 4+$x67, +/// 8, 9, 10, 11, 12+$x01, 12+$x23, 12+$x45, 12+$x67 +/// ]); +/// }; +/// } +/// } +/// ``` +pub unsafe fn _mm256_shufflehi_epi16(a: __m256i, imm8: i32) -> __m256i { + let imm8 = (imm8 & 0xFF) as u8; + let a = a.as_i16x16(); + macro_rules! shuffle_done { + ($x01:expr, $x23:expr, $x45:expr, $x67:expr) => { + #[cfg_attr(rustfmt, rustfmt_skip)] + simd_shuffle16(a, a, [ + 0, 1, 2, 3, 4+$x01, 4+$x23, 4+$x45, 4+$x67, + 8, 9, 10, 11, 12+$x01, 12+$x23, 12+$x45, 12+$x67 + ]); + }; + } +} + +/// The skipped method shouldn't right-shift +pub unsafe fn _mm256_shufflehi_epi32(a: __m256i, imm8: i32) -> __m256i { + let imm8 = (imm8 & 0xFF) as u8; + let a = a.as_i16x16(); + macro_rules! shuffle_done { + ($x01:expr, $x23:expr, $x45:expr, $x67:expr) => { + #[cfg_attr(rustfmt, rustfmt_skip)] + simd_shuffle32(a, a, [ + 0, 1, 2, 3, 4+$x01, 4+$x23, 4+$x45, 4+$x67, + 8, 9, 10, 11, 12+$x01, 12+$x23, 12+$x45, 12+$x67 + ]); + }; + } +}