Skip to content

Commit 42e38e2

Browse files
committed
Fix for issue 4499 - Skip in struct impl (v2)
1 parent 87e9602 commit 42e38e2

12 files changed

+462
-20
lines changed

src/formatting/items.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,8 +892,14 @@ pub(crate) fn format_impl(
892892
let inner_indent_str = visitor.block_indent.to_string_with_newline(context.config);
893893
let outer_indent_str = offset.block_only().to_string_with_newline(context.config);
894894

895-
result.push_str(&inner_indent_str);
896-
result.push_str(visitor.buffer.trim());
895+
if visitor.buffer.starts_with("\n") {
896+
// Starts with kipped code - first line not formatted
897+
result.push_str(visitor.buffer.trim_end());
898+
} else {
899+
// First line not kipped code - first line is formatted
900+
result.push_str(&inner_indent_str);
901+
result.push_str(visitor.buffer.trim());
902+
}
897903
result.push_str(&outer_indent_str);
898904
} else if need_newline || !context.config.empty_item_single_line() {
899905
result.push_str(&sep);

src/formatting/missed_spans.rs

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,15 @@ impl<'a> FmtVisitor<'a> {
5050
self.last_pos = end;
5151
return;
5252
}
53-
self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet));
53+
self.format_missing_inner(end, true, |this, last_snippet, _| {
54+
this.push_str(last_snippet)
55+
});
5456
self.normalize_vertical_spaces = false;
5557
}
5658

5759
pub(crate) fn format_missing_with_indent(&mut self, end: BytePos) {
5860
let config = self.config;
59-
self.format_missing_inner(end, |this, last_snippet, snippet| {
61+
self.format_missing_inner(end, true, |this, last_snippet, snippet| {
6062
this.push_str(last_snippet.trim_end());
6163
if last_snippet == snippet && !this.output_at_start() {
6264
// No new lines in the snippet.
@@ -69,15 +71,23 @@ impl<'a> FmtVisitor<'a> {
6971
}
7072

7173
pub(crate) fn format_missing_no_indent(&mut self, end: BytePos) {
72-
self.format_missing_inner(end, |this, last_snippet, _| {
74+
self.format_missing_inner(end, true, |this, last_snippet, _| {
7375
this.push_str(last_snippet.trim_end());
7476
});
7577
self.normalize_vertical_spaces = false;
7678
}
7779

80+
pub(crate) fn format_missing_before_skip(&mut self, end: BytePos) {
81+
self.format_missing_inner(end, false, |this, last_snippet, _| {
82+
this.push_str(last_snippet);
83+
});
84+
self.normalize_vertical_spaces = false;
85+
}
86+
7887
fn format_missing_inner<F: Fn(&mut FmtVisitor<'_>, &str, &str)>(
7988
&mut self,
8089
end: BytePos,
90+
trim_last_line: bool, // Keep last line contents when snippet is empty
8191
process_last_snippet: F,
8292
) {
8393
let start = self.last_pos;
@@ -110,7 +120,17 @@ impl<'a> FmtVisitor<'a> {
110120
if snippet.trim().is_empty() && !out_of_file_lines_range!(self, span) {
111121
// Keep vertical spaces within range.
112122
self.push_vertical_spaces(count_newlines(snippet));
113-
process_last_snippet(self, "", snippet);
123+
if trim_last_line {
124+
process_last_snippet(self, "", snippet);
125+
} else {
126+
let last_nl = snippet.rfind('\n');
127+
let first = if let Some(last) = last_nl {
128+
last + 1
129+
} else {
130+
0
131+
};
132+
process_last_snippet(self, &snippet[first..], "");
133+
}
114134
} else {
115135
self.write_snippet(span, &process_last_snippet);
116136
}

src/formatting/visitor.rs

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
154154
ast::StmtKind::Local(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => {
155155
let attrs = get_attrs_from_stmt(stmt.as_ast_node());
156156
if contains_skip(attrs) {
157-
self.push_skipped_with_span(
157+
self.push_skipped_with_span_non_impl(
158158
attrs,
159159
stmt.span(),
160160
get_span_without_attrs(stmt.as_ast_node()),
@@ -167,7 +167,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
167167
}
168168
ast::StmtKind::MacCall(ref mac_stmt) => {
169169
if self.visit_attrs(&mac_stmt.attrs, ast::AttrStyle::Outer) {
170-
self.push_skipped_with_span(
170+
self.push_skipped_with_span_non_impl(
171171
&mac_stmt.attrs,
172172
stmt.span(),
173173
get_span_without_attrs(stmt.as_ast_node()),
@@ -486,7 +486,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
486486
// For use/extern crate items, skip rewriting attributes but check for a skip attribute.
487487
ast::ItemKind::Use(..) | ast::ItemKind::ExternCrate(_) => {
488488
if contains_skip(attrs) {
489-
self.push_skipped_with_span(attrs.as_slice(), item.span(), item.span());
489+
self.push_skipped_with_span_non_impl(attrs.as_slice(), item.span(), item.span);
490490
false
491491
} else {
492492
true
@@ -495,7 +495,11 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
495495
// Module is inline, in this case we treat it like any other item.
496496
_ if !is_mod_decl(item) => {
497497
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
498-
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
498+
self.push_skipped_with_span_non_impl(
499+
item.attrs.as_slice(),
500+
item.span(),
501+
item.span,
502+
);
499503
false
500504
} else {
501505
true
@@ -515,7 +519,11 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
515519
}
516520
_ => {
517521
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
518-
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
522+
self.push_skipped_with_span_non_impl(
523+
item.attrs.as_slice(),
524+
item.span(),
525+
item.span,
526+
);
519527
false
520528
} else {
521529
true
@@ -672,7 +680,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
672680
skip_out_of_file_lines_range_visitor!(self, ti.span);
673681

674682
if self.visit_attrs(&ti.attrs, ast::AttrStyle::Outer) {
675-
self.push_skipped_with_span(ti.attrs.as_slice(), ti.span, ti.span);
683+
self.push_skipped_with_span_non_impl(ti.attrs.as_slice(), ti.span(), ti.span);
676684
return;
677685
}
678686
let skip_context_outer = self.skip_context.clone();
@@ -732,7 +740,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
732740
skip_out_of_file_lines_range_visitor!(self, ii.span);
733741

734742
if self.visit_attrs(&ii.attrs, ast::AttrStyle::Outer) {
735-
self.push_skipped_with_span(ii.attrs.as_slice(), ii.span, ii.span);
743+
self.push_skipped_with_span_impl(ii.attrs.as_slice(), ii.span(), ii.span);
736744
return;
737745
}
738746
let skip_context_outer = self.skip_context.clone();
@@ -832,28 +840,31 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
832840
}
833841

834842
#[allow(clippy::needless_pass_by_value)]
835-
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>) {
843+
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>, trim_start: bool) {
836844
if let Some(ref s) = rewrite {
837845
self.push_str(s);
838846
} else {
839-
let snippet = self.snippet(span);
840-
self.push_str(snippet.trim());
847+
let mut snippet = self.snippet(span).trim_end();
848+
if trim_start {
849+
snippet = snippet.trim_start()
850+
};
851+
self.push_str(snippet);
841852
}
842853
self.last_pos = source!(self, span).hi();
843854
}
844855

845856
pub(crate) fn push_rewrite(&mut self, span: Span, rewrite: Option<String>) {
846857
self.format_missing_with_indent(source!(self, span).lo());
847-
self.push_rewrite_inner(span, rewrite);
858+
self.push_rewrite_inner(span, rewrite, true);
848859
}
849860

850861
pub(crate) fn push_skipped_with_span(
851862
&mut self,
852863
attrs: &[ast::Attribute],
853864
item_span: Span,
854865
main_span: Span,
866+
trim_start: bool,
855867
) {
856-
self.format_missing_with_indent(source!(self, item_span).lo());
857868
// do not take into account the lines with attributes as part of the skipped range
858869
let attrs_end = attrs
859870
.iter()
@@ -865,13 +876,33 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
865876
// or it can be on the same line as the last attribute.
866877
// So here we need to take a minimum between the two.
867878
let lo = std::cmp::min(attrs_end + 1, first_line);
868-
self.push_rewrite_inner(item_span, None);
879+
self.push_rewrite_inner(item_span, None, trim_start);
869880
let hi = self.line_number + 1;
870881
self.skipped_range
871882
.borrow_mut()
872883
.push(NonFormattedRange::new(lo, hi));
873884
}
874885

886+
pub(crate) fn push_skipped_with_span_impl(
887+
&mut self,
888+
attrs: &[ast::Attribute],
889+
item_span: Span,
890+
main_span: Span,
891+
) {
892+
self.format_missing_before_skip(source!(self, item_span).lo());
893+
self.push_skipped_with_span(attrs, item_span, main_span, false);
894+
}
895+
896+
pub(crate) fn push_skipped_with_span_non_impl(
897+
&mut self,
898+
attrs: &[ast::Attribute],
899+
item_span: Span,
900+
main_span: Span,
901+
) {
902+
self.format_missing_with_indent(source!(self, item_span).lo());
903+
self.push_skipped_with_span(attrs, item_span, main_span, true);
904+
}
905+
875906
pub(crate) fn from_context(ctx: &'a RewriteContext<'_>) -> FmtVisitor<'a> {
876907
let mut visitor = FmtVisitor::from_parse_sess(
877908
ctx.parse_sess,

tests/source/skip/skip-and-fn.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// *** "rustfmt::skip" and Function
2+
// *** All source lines starts with indent by 3 tabls.
3+
4+
// "rustfmt::skip" as first line;
5+
#[rustfmt::skip]
6+
/** Comment line 1
7+
comment line 2. */
8+
fn main() {
9+
x = y;
10+
}
11+
12+
// "rustfmt::skip" whithin a block of attributes
13+
fn main() {
14+
#[rustfmt::skip]
15+
// Comment 1
16+
#[allow(non_snake_case)]
17+
// Comment 2
18+
#[allow(non_snake_case)]
19+
pub fn foo(&self) {}
20+
}
21+
22+
fn main() {
23+
// Comment 1
24+
#[rustfmt::skip]
25+
#[allow(non_snake_case)]
26+
// Comment 2
27+
#[allow(non_snake_case)]
28+
pub fn foo(&self) {}
29+
}
30+
31+
fn main() {
32+
// Comment 1
33+
#[allow(non_snake_case)]
34+
// Comment 2
35+
#[allow(non_snake_case)]
36+
#[rustfmt::skip]
37+
#[allow(non_snake_case)]
38+
pub fn foo(&self) {}
39+
}
40+
41+
fn main() {
42+
#[rustfmt::skip]
43+
/** Comment line 1
44+
comment line 2. */
45+
#[allow(non_snake_case)]
46+
x = y;
47+
}
48+
49+
// fn with Doc comment which is attribute vs one-line comment
50+
fn main() {
51+
// Comment for `foo`
52+
#[rustfmt::skip] // comment on why use a skip here
53+
#[allow(non_snake_case)]
54+
pub fn foo(&self) {}
55+
}

tests/source/skip/skip-and-impl.rs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// *** "rustfmt::skip" and Impl
2+
3+
// Original code from issue #4499
4+
#[rustfmt::skip]
5+
/** Comment.
6+
7+
Rustfmt skips this as expected. */
8+
fn main() {
9+
for _ in 1..Foo::getRandomNumber() {
10+
println!("Hello, world!");
11+
}
12+
}
13+
14+
struct Foo;
15+
impl Foo {
16+
#[rustfmt::skip]
17+
/** Comment.
18+
19+
Rustfmt unindents this despite the ::skip above. */
20+
#[allow(non_snake_case)]
21+
pub fn getRandomNumber() -> i32 { 4 }
22+
}
23+
24+
// *** All source lines starts with indent by 3 tabls.
25+
26+
// "rustfmt::skip" as first line;
27+
#[rustfmt::skip]
28+
/** Comment line 1
29+
comment line 2. */
30+
#[allow(non_snake_case)]
31+
impl Foo {
32+
pub fn getRandomNumber() -> i32 { 4 }
33+
}
34+
35+
// "rustfmt::skip" whithin a block of attributes
36+
impl Foo {
37+
#[rustfmt::skip]
38+
// Comment 1
39+
#[allow(non_snake_case)]
40+
// Comment 2
41+
#[allow(non_snake_case)]
42+
pub fn foo(&self) {}
43+
}
44+
45+
impl Foo {
46+
// Comment 1
47+
#[rustfmt::skip]
48+
#[allow(non_snake_case)]
49+
// Comment 2
50+
#[allow(non_snake_case)]
51+
pub fn foo(&self) {}
52+
}
53+
54+
impl Foo {
55+
// Comment 1
56+
#[allow(non_snake_case)]
57+
// Comment 2
58+
#[allow(non_snake_case)]
59+
#[rustfmt::skip]
60+
#[allow(non_snake_case)]
61+
pub fn foo(&self) {}
62+
}
63+
64+
impl Foo {
65+
#[rustfmt::skip]
66+
/** Comment line 1
67+
comment line 2. */
68+
#[allow(non_snake_case)]
69+
pub fn getRandomNumber() -> i32 { 4 }
70+
}
71+
72+
// impl with Doc comment which is attribute vs one-line comment
73+
impl Struct {
74+
/// Documentation for `foo`
75+
#[rustfmt::skip]
76+
#[allow(non_snake_case)]
77+
pub fn foo(&self) {}
78+
}
79+
80+
impl Struct {
81+
// Comment for `foo`
82+
#[rustfmt::skip]
83+
#[allow(non_snake_case)]
84+
pub fn foo(&self) {}
85+
}
86+

tests/source/skip/skip-and-import.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// *** "rustfmt::skip" and Imports Use/Extern
2+
// *** All source lines starts with indent by 3 tabls.
3+
4+
#[rustfmt::skip]
5+
use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit};
6+
use crate::config::{BraceStyle, Config};
7+
use crate::formatting::{
8+
attr::*,
9+
modules::{FileModMap, Module},
10+
};
11+
12+
use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit};
13+
#[rustfmt::skip]
14+
use crate::config::{BraceStyle, Config};
15+
use crate::formatting::{
16+
attr::*,
17+
modules::{FileModMap, Module},
18+
};

0 commit comments

Comments
 (0)