Skip to content

Commit 4f2febe

Browse files
committed
Fix for issue 4499 - Skip in struct impl
1 parent 87e9602 commit 4f2febe

File tree

4 files changed

+371
-6
lines changed

4 files changed

+371
-6
lines changed

src/formatting/visitor.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,8 @@ 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+
// `item.span()` includes the attributes, while `item.span` does not.
490+
self.push_skipped_with_span(attrs.as_slice(), item.span(), item.span);
490491
false
491492
} else {
492493
true
@@ -495,7 +496,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
495496
// Module is inline, in this case we treat it like any other item.
496497
_ if !is_mod_decl(item) => {
497498
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
498-
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
499+
// `item.span()` includes the attributes, while `item.span` does not.
500+
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span);
499501
false
500502
} else {
501503
true
@@ -515,7 +517,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
515517
}
516518
_ => {
517519
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
518-
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
520+
// `item.span()` includes the attributes, while `item.span` does not.
521+
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span);
519522
false
520523
} else {
521524
true
@@ -672,7 +675,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
672675
skip_out_of_file_lines_range_visitor!(self, ti.span);
673676

674677
if self.visit_attrs(&ti.attrs, ast::AttrStyle::Outer) {
675-
self.push_skipped_with_span(ti.attrs.as_slice(), ti.span, ti.span);
678+
// `ti.span()` includes the attributes, while `ti.span` does not.
679+
self.push_skipped_with_span(ti.attrs.as_slice(), ti.span(), ti.span);
676680
return;
677681
}
678682
let skip_context_outer = self.skip_context.clone();
@@ -732,7 +736,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
732736
skip_out_of_file_lines_range_visitor!(self, ii.span);
733737

734738
if self.visit_attrs(&ii.attrs, ast::AttrStyle::Outer) {
735-
self.push_skipped_with_span(ii.attrs.as_slice(), ii.span, ii.span);
739+
// `ii.span()` includes the attributes, while `ii.span` does not.
740+
self.push_skipped_with_span(ii.attrs.as_slice(), ii.span(), ii.span);
736741
return;
737742
}
738743
let skip_context_outer = self.skip_context.clone();

tests/source/issue-4499.rs

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
// Different "rustfmt::skip" (not)formatting tests
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+
// ****** "rustfmt::skip" as first line
25+
#[rustfmt::skip]
26+
/** Comment.
27+
28+
Rustfmt unindents this despite the ::skip above. */
29+
#[allow(non_snake_case)]
30+
impl Foo {
31+
pub fn getRandomNumber() -> i32 { 4 }
32+
}
33+
34+
#[rustfmt::skip]
35+
/** Comment.
36+
37+
Rustfmt skips this as expected. */
38+
fn main() {
39+
for _ in 1..Foo::getRandomNumber() {
40+
println!("Hello, world!");
41+
}
42+
}
43+
44+
// ****** "rustfmt::skip" as first line inside a module
45+
impl Foo {
46+
#[rustfmt::skip]
47+
/** Comment.
48+
49+
Rustfmt unindents this despite the ::skip above. */
50+
#[allow(non_snake_case)]
51+
pub fn getRandomNumber() -> i32 { 4 }
52+
}
53+
54+
fn main() {
55+
#[rustfmt::skip]
56+
/** Comment.
57+
58+
Rustfmt unindents this despite the ::skip above. */
59+
#[allow(non_snake_case)]
60+
pub fn getRandomNumber() -> i32 { 4 }
61+
}
62+
63+
// ******** "rustfmt::skip" whithin a block of attributes
64+
fn main() {
65+
// Comment 1
66+
// Comment 2
67+
#[allow(non_snake_case)]
68+
// Comment 3
69+
#[allow(non_snake_case)]
70+
#[allow(non_snake_case)]
71+
#[allow(non_snake_case)]
72+
#[rustfmt::skip]
73+
#[allow(non_snake_case)]
74+
#[allow(non_snake_case)]
75+
#[allow(non_snake_case)]
76+
pub fn foo(&self) {}
77+
}
78+
79+
impl Foo {
80+
// Comment 1
81+
// Comment 2
82+
#[allow(non_snake_case)]
83+
// Comment 3
84+
#[allow(non_snake_case)]
85+
#[allow(non_snake_case)]
86+
#[allow(non_snake_case)]
87+
#[rustfmt::skip]
88+
#[allow(non_snake_case)]
89+
#[allow(non_snake_case)]
90+
#[allow(non_snake_case)]
91+
pub fn foo(&self) {}
92+
}
93+
94+
// ******* Doc comment which is attribute vs one-line comment
95+
impl Struct {
96+
/// Documentation for `foo`
97+
#[rustfmt::skip]
98+
#[allow(non_snake_case)]
99+
pub fn foo(&self) {}
100+
}
101+
102+
impl Struct {
103+
// Comment for `foo`
104+
#[rustfmt::skip]
105+
#[allow(non_snake_case)]
106+
pub fn foo(&self) {}
107+
}
108+
109+
fn main() {
110+
/// Documentation for `foo`
111+
#[rustfmt::skip]
112+
#[allow(non_snake_case)]
113+
pub fn foo(&self) {}
114+
}
115+
116+
fn main() {
117+
// Comment for `foo`
118+
#[rustfmt::skip] // comment on why use a skip here
119+
#[allow(non_snake_case)]
120+
pub fn foo(&self) {}
121+
}
122+
123+
// ******** "rustfmt::skip" and Use/Extern
124+
#[rustfmt::skip]
125+
use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit};
126+
use rustc_span::{symbol, BytePos, Pos, Span, DUMMY_SP};
127+
use crate::config::{BraceStyle, Config};
128+
use crate::formatting::{
129+
attr::*,
130+
macros::{macro_style, rewrite_macro, rewrite_macro_def, MacroPosition},
131+
modules::{FileModMap, Module},
132+
};
133+
134+
use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit};
135+
use rustc_span::{symbol, BytePos, Pos, Span, DUMMY_SP};
136+
#[rustfmt::skip]
137+
use crate::config::{BraceStyle, Config};
138+
use crate::formatting::{
139+
attr::*,
140+
macros::{macro_style, rewrite_macro, rewrite_macro_def, MacroPosition},
141+
modules::{FileModMap, Module},
142+
};
143+
144+
// ******** "rustfmt::skip" and `is_mod_decl(item)` ?????
145+
146+
// ******** "rustfmt::skip" and Trait with Attributes ?????????
147+
#[rustfmt::skip]
148+
#[allow(non_snake_case)]
149+
trait Animal1 {
150+
// Static method signature; `Self` refers to the implementor type.
151+
fn new(name: &'static str) -> Self;
152+
153+
// Instance method signatures; these will return a string.
154+
fn name(&self) -> &'static str;
155+
fn noise(&self) -> &'static str;
156+
157+
// Traits can provide default method definitions.
158+
fn talk(&self) {
159+
println!("{} says {}", self.name(), self.noise());
160+
}
161+
}
162+
163+
trait Animal2 {
164+
// Static method signature; `Self` refers to the implementor type.
165+
fn new(name: &'static str) -> Self;
166+
167+
// Instance method signatures; these will return a string.
168+
fn name(&self) -> &'static str;
169+
#[rustfmt::skip]
170+
#[allow(non_snake_case)]
171+
fn noise(&self) -> &'static str;
172+
173+
fn talk(&self) {
174+
// Traits can provide default method definitions.
175+
#[rustfmt::skip]
176+
#[allow(non_snake_case)]
177+
fn name(&self) -> &'static str;
178+
println!("{} says {}", self.name(), self.noise());
179+
}
180+
}

tests/target/issue-4398.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ impl Struct {
66

77
impl Struct {
88
/// Documentation for `foo`
9-
#[rustfmt::skip] // comment on why use a skip here
9+
#[rustfmt::skip] // comment on why use a skip here
1010
pub fn foo(&self) {}
1111
}
1212

0 commit comments

Comments
 (0)