Merge pull request rust-lang#3113 from scampi/issue3105

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

Author: nrc

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

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)

src/lib.rs

@@ -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
@@ -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 {
@@ -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 {
@@ -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.
@@ -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]

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('\\'))

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::<Vec<String>>()
+        .concat();
+    format!("{}{}", first_line, rest)
+}
+
 #[test]
 fn test_remove_trailing_white_spaces() {
     let s = "    r#\"\n        test\n    \"#";