Fix to some rustfmt::skip issues

src/formatting.rs

@@ -164,6 +164,15 @@ fn format_project(
         )
     });
 
+    // Debug messages with skipped ranges for all input files
+    for (filename, format_result) in format_report.format_result_as_rc().borrow().iter() {
+        let skipped_ranges = &format_result.formatted_snippet().non_formatted_ranges;
+        debug!(
+            "format_project: filename \"{:?}\" skipped_ranges \"{:?}\"",
+            filename, skipped_ranges
+        );
+    }
+
     Ok(format_report)
 }
 

src/formatting/items.rs

@@ -889,6 +889,11 @@ pub(crate) fn format_impl(
 
             visitor.format_missing(item.span.hi() - BytePos(1));
 
+            context
+                .skipped_range
+                .borrow_mut()
+                .append(&mut visitor.skipped_range.borrow_mut());
+
             let inner_indent_str = visitor.block_indent.to_string_with_newline(context.config);
             let outer_indent_str = offset.block_only().to_string_with_newline(context.config);
 
@@ -1261,6 +1266,11 @@ pub(crate) fn format_trait(
 
             visitor.format_missing(item.span.hi() - BytePos(1));
 
+            context
+                .skipped_range
+                .borrow_mut()
+                .append(&mut visitor.skipped_range.borrow_mut());
+
             let inner_indent_str = visitor.block_indent.to_string_with_newline(context.config);
 
             result.push_str(&inner_indent_str);

src/formatting/macros.rs

@@ -1568,6 +1568,11 @@ fn rewrite_macro_with_items(
         visitor.visit_item(&item, false);
     }
 
+    context
+        .skipped_range
+        .borrow_mut()
+        .append(&mut visitor.skipped_range.borrow_mut());
+
     let mut result = String::with_capacity(256);
     result.push_str(&macro_name);
     result.push_str(opener);

src/formatting/report.rs

@@ -28,7 +28,7 @@ pub struct FormatResult {
 }
 
 /// The inclusive range of the input which was not formatted, represented by a pair of line numbers.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)]
 pub(crate) struct NonFormattedRange {
     lo: usize,
     hi: usize,
@@ -262,6 +262,7 @@ impl NonFormattedRange {
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::formatting::utils::format_snippet;
 
     #[cfg(test)]
     mod has_failing_errors {
@@ -419,4 +420,117 @@ mod test {
             assert!(report.has_failing_errors(vec![(file_name, &config)].into_iter().collect()));
         }
     }
+
+    #[test]
+    fn test_skipped_ranges() {
+        #[rustfmt::skip]
+        let snippet = "// Impl - original code from issue #4706
+            impl      Foo          {
+            #[rustfmt::skip] // Lines 4-7
+            fn foo() {
+            Bar     
+            //     ^ there is whitespace here
+            }
+
+            fn bar() {
+            Qux // asdf
+            }
+            }
+
+// Impl - all skipped lines end with white spaces
+            #[rustfmt::skip] // Lines 15-22
+            /// DOC1   
+            /// DOC2   
+            impl      Foo1          {     
+            fn foo1() {
+            Bar1     
+            }   
+            }     
+
+            impl      Foo2          {
+            #[rustfmt::skip] // Lines 26-30
+            /// DOC1   
+            /// DOC2   
+            fn foo2() {    
+            Bar2     
+            }       
+            }
+
+            impl      Foo3          {
+            fn foo3() {
+            #[rustfmt::skip] // Lines 36-38
+            /// DOC1   
+            /// DOC2        
+            Bar2     
+            }  
+            }
+
+// fn - all skipped lines end with white spaces            
+            #[rustfmt::skip] // Lines 43-48
+            /// DOC1   
+            /// DOC2   
+            fn foo2() {    
+            Bar2     
+            }       
+
+            fn foo3() {
+            #[rustfmt::skip] // Lines 52-54
+            /// DOC1   
+            /// DOC2        
+            Bar2     
+            }
+
+// Trait - all skipped lines end with white spaces
+
+            // All skipped lines end with white spaces
+            #[rustfmt::skip] // Lines 60-66
+            #[allow(non_snake_case)]    
+            trait Animal1 {    
+            fn new(name: &'static str) -> Self;    
+
+            fn talk(&self) {}     
+            }    
+
+            // All skipped lines end with white spaces
+            #[allow(non_snake_case)]
+            trait Animal1 {
+            #[rustfmt::skip] // Lines 72-72
+            fn new(name: &'static str) -> Self;    
+
+            fn talk(&self) {}    
+            }
+
+            // Internal skipped line
+            #[allow(non_snake_case)]
+            trait Animal3 {
+            fn new(name: &'static str) -> Self;
+
+            fn talk(&self) {
+            #[rustfmt::skip] // Lines 84-84
+            let x = 1;    
+            //          ^ there is whitespace here
+            }
+            }
+
+// Macro - all skipped lines end with white spaces.
+            #[rustfmt::skip] // Lines  90-93
+            macro_rules! my_macro1 {    
+            () => {};    
+
+        }";
+
+        let mut actual: Vec<NonFormattedRange> = vec![];
+        #[rustfmt::skip]
+        let expected: Vec<NonFormattedRange> = vec![NonFormattedRange { lo: 4, hi: 7 }, NonFormattedRange { lo: 15, hi: 22 }, NonFormattedRange { lo: 26, hi: 30 }, NonFormattedRange { lo: 36, hi: 38 }, NonFormattedRange { lo: 43, hi: 48 }, NonFormattedRange { lo: 52, hi: 54 }, NonFormattedRange { lo: 60, hi: 66 }, NonFormattedRange { lo: 72, hi: 72 }, NonFormattedRange { lo: 84, hi: 84 }, NonFormattedRange { lo: 90, hi: 94 }];
+
+        let output = format_snippet(snippet, &Config::default(), false);
+
+        let output_ok = output.map_or(false, |o| {
+            actual = o.non_formatted_ranges;
+            (actual.len() == expected.len())
+                && actual.iter().zip(expected.clone()).all(|(a, b)| *a == b)
+        });
+
+        assert!(output_ok, "\nexpect={:?},\nactual={:?}", expected, actual);
+    }
 }

src/formatting/visitor.rs

@@ -671,7 +671,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.attrs.as_slice(), ti.span, ti.span);
+            self.push_skipped_with_span(ti.attrs.as_slice(), ti.span(), ti.span);
             return;
         }
         let skip_context_outer = self.skip_context.clone();
@@ -731,7 +731,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.attrs.as_slice(), ii.span, ii.span);
+            self.push_skipped_with_span(ii.attrs.as_slice(), ii.span(), ii.span);
             return;
         }
         let skip_context_outer = self.skip_context.clone();
@@ -853,19 +853,30 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         main_span: Span,
     ) {
         self.format_missing_with_indent(source!(self, item_span).lo());
+        let init_line_number = self.line_number;
+
+        /* FIXME: is the following comment correct, i.e all attributes are not skipped?
+         * If not correct, then comment should be removed;
+         * If correct, then skipped range should depend on end of attributes.
+         * Currently, skipped range starts after the first attribute line.
+         */
         // do not take into account the lines with attributes as part of the skipped range
-        let attrs_end = attrs
-            .iter()
-            .map(|attr| self.parse_sess.line_of_byte_pos(attr.span.hi()))
-            .max()
-            .unwrap_or(1);
         let first_line = self.parse_sess.line_of_byte_pos(main_span.lo());
+        let attrs_start = attrs
+            .first()
+            .map(|attr| self.parse_sess.line_of_byte_pos(attr.span.lo()))
+            .unwrap_or(1);
+
         // Statement can start after some newlines and/or spaces
         // or it can be on the same line as the last attribute.
         // So here we need to take a minimum between the two.
-        let lo = std::cmp::min(attrs_end + 1, first_line);
+        let lo = std::cmp::min(attrs_start + 1, first_line);
         self.push_rewrite_inner(item_span, None);
-        let hi = self.line_number + 1;
+        let hi = std::cmp::max(
+            self.line_number + 1,
+            attrs_start + self.line_number - init_line_number,
+        );
+
         self.skipped_range
             .borrow_mut()
             .push(NonFormattedRange::new(lo, hi));

tests/source/skip/skip-and-whitespaces-at-line-end.rs

@@ -0,0 +1,106 @@
+// *** "rustfmt::skip" and Whitespaces at nd of line
+// *** All source lines starts with indent by 3 tabls.
+
+// Impl - original code from issue #4706
+            impl      Foo          {
+            #[rustfmt::skip]
+            fn foo() {
+            Bar     
+            //     ^ there is whitespace here
+            }
+
+            fn bar() {
+            Qux // asdf
+            }
+            }
+
+// Impl - all skipped lines end with white spaces
+            #[rustfmt::skip]
+            /// DOC1   
+            /// DOC2   
+            impl      Foo1          {     
+            fn foo1() {
+            Bar1     
+            }   
+            }     
+
+            impl      Foo2          {
+            #[rustfmt::skip]
+            /// DOC1   
+            /// DOC2   
+            fn foo2() {    
+            Bar2     
+            }       
+            }
+
+            impl      Foo3          {
+            fn foo3() {
+            #[rustfmt::skip]
+            /// DOC1   
+            /// DOC2        
+            Bar2     
+            }  
+            }
+
+// fn - all skipped lines end with white spaces            
+            #[rustfmt::skip]
+            /// DOC1   
+            /// DOC2   
+            fn foo2() {    
+            Bar2     
+            }       
+
+            fn foo3() {
+            #[rustfmt::skip]
+            /// DOC1   
+            /// DOC2        
+            Bar2     
+            }
+
+// Trait - all skipped lines end with white spaces
+
+            // All skipped lines end with white spaces
+            #[rustfmt::skip]
+            #[allow(non_snake_case)]    
+            trait Animal1 {    
+            fn new(name: &'static str) -> Self;    
+
+            fn talk(&self) {}     
+            }    
+
+            // All skipped lines end with white spaces
+            #[allow(non_snake_case)]
+            trait Animal1 {
+            #[rustfmt::skip]
+            fn new(name: &'static str) -> Self;    
+
+            fn talk(&self) {}    
+            }
+
+            // Internal skipped line
+            #[allow(non_snake_case)]
+            trait Animal3 {
+            fn new(name: &'static str) -> Self;
+
+            fn talk(&self) {
+            #[rustfmt::skip]
+            let x = 1;    
+            //          ^ there is whitespace here
+            }
+            }
+
+// Macro - all skipped lines end with white spaces.
+            #[rustfmt::skip]
+            macro_rules! my_macro1 {    
+            () => {};    
+            }    
+
+            // Skipped range in macro definitin body does **NOT** enter into final `skipped_range`
+            // list since macro definition is formatted as a stand alone `snippet`.
+            macro_rules! my_macro2 {
+            ($param) => {
+            #[rustfmt::skip]
+            $param
+            //     ^ there are **NO** trailing whitespaces here 
+            };
+            }    

tests/target/issue-4398.rs

@@ -6,7 +6,7 @@ impl Struct {
 
 impl Struct {
     /// Documentation for `foo`
-    #[rustfmt::skip] // comment on why use a skip here
+       #[rustfmt::skip] // comment on why use a skip here
     pub fn foo(&self) {}
 }