diff --git a/src/formatting.rs b/src/formatting.rs index 348c7694faa..6b6c7512217 100644 --- a/src/formatting.rs +++ b/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) } diff --git a/src/formatting/items.rs b/src/formatting/items.rs index d2e5e70fce7..27b6ec744e9 100644 --- a/src/formatting/items.rs +++ b/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); diff --git a/src/formatting/macros.rs b/src/formatting/macros.rs index 53032f58c7a..99ecbc2a41e 100644 --- a/src/formatting/macros.rs +++ b/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(¯o_name); result.push_str(opener); diff --git a/src/formatting/report.rs b/src/formatting/report.rs index 0588bea486e..842969fb3cb 100644 --- a/src/formatting/report.rs +++ b/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 = vec![]; + #[rustfmt::skip] + let expected: Vec = 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); + } } diff --git a/src/formatting/visitor.rs b/src/formatting/visitor.rs index bcda80e17d3..e129620b966 100644 --- a/src/formatting/visitor.rs +++ b/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)); diff --git a/tests/source/skip/skip-and-whitespaces-at-line-end.rs b/tests/source/skip/skip-and-whitespaces-at-line-end.rs new file mode 100644 index 00000000000..682752b74d7 --- /dev/null +++ b/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 + }; + } diff --git a/tests/target/issue-4398.rs b/tests/target/issue-4398.rs index 2ca894528e9..b0095aaac79 100644 --- a/tests/target/issue-4398.rs +++ b/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) {} } diff --git a/tests/target/skip/skip-and-whitespaces-at-line-end.rs b/tests/target/skip/skip-and-whitespaces-at-line-end.rs new file mode 100644 index 00000000000..b6aaa067d32 --- /dev/null +++ b/tests/target/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 + }; +}