Skip to content

Skip in struct impl and other cases #4707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: rustfmt-2.0.0-rc.2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/formatting/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,8 +892,14 @@ pub(crate) fn format_impl(
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);

result.push_str(&inner_indent_str);
result.push_str(visitor.buffer.trim());
if visitor.buffer.starts_with("\n") {
// Starts with kipped code - first line not formatted
result.push_str(visitor.buffer.trim_end());
} else {
// First line not kipped code - first line is formatted
result.push_str(&inner_indent_str);
result.push_str(visitor.buffer.trim());
}
result.push_str(&outer_indent_str);
} else if need_newline || !context.config.empty_item_single_line() {
result.push_str(&sep);
Expand Down
28 changes: 24 additions & 4 deletions src/formatting/missed_spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ impl<'a> FmtVisitor<'a> {
self.last_pos = end;
return;
}
self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet));
self.format_missing_inner(end, true, |this, last_snippet, _| {
this.push_str(last_snippet)
});
self.normalize_vertical_spaces = false;
}

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

pub(crate) fn format_missing_no_indent(&mut self, end: BytePos) {
self.format_missing_inner(end, |this, last_snippet, _| {
self.format_missing_inner(end, true, |this, last_snippet, _| {
this.push_str(last_snippet.trim_end());
});
self.normalize_vertical_spaces = false;
}

pub(crate) fn format_missing_before_skip(&mut self, end: BytePos) {
self.format_missing_inner(end, false, |this, last_snippet, _| {
this.push_str(last_snippet);
});
self.normalize_vertical_spaces = false;
}

fn format_missing_inner<F: Fn(&mut FmtVisitor<'_>, &str, &str)>(
&mut self,
end: BytePos,
trim_last_line: bool, // Keep last line contents when snippet is empty
process_last_snippet: F,
) {
let start = self.last_pos;
Expand Down Expand Up @@ -110,7 +120,17 @@ impl<'a> FmtVisitor<'a> {
if snippet.trim().is_empty() && !out_of_file_lines_range!(self, span) {
// Keep vertical spaces within range.
self.push_vertical_spaces(count_newlines(snippet));
process_last_snippet(self, "", snippet);
if trim_last_line {
process_last_snippet(self, "", snippet);
} else {
let last_nl = snippet.rfind('\n');
let first = if let Some(last) = last_nl {
last + 1
} else {
0
};
process_last_snippet(self, &snippet[first..], "");
}
} else {
self.write_snippet(span, &process_last_snippet);
}
Expand Down
57 changes: 44 additions & 13 deletions src/formatting/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
ast::StmtKind::Local(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => {
let attrs = get_attrs_from_stmt(stmt.as_ast_node());
if contains_skip(attrs) {
self.push_skipped_with_span(
self.push_skipped_with_span_non_impl(
attrs,
stmt.span(),
get_span_without_attrs(stmt.as_ast_node()),
Expand All @@ -167,7 +167,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}
ast::StmtKind::MacCall(ref mac_stmt) => {
if self.visit_attrs(&mac_stmt.attrs, ast::AttrStyle::Outer) {
self.push_skipped_with_span(
self.push_skipped_with_span_non_impl(
&mac_stmt.attrs,
stmt.span(),
get_span_without_attrs(stmt.as_ast_node()),
Expand Down Expand Up @@ -486,7 +486,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
// For use/extern crate items, skip rewriting attributes but check for a skip attribute.
ast::ItemKind::Use(..) | ast::ItemKind::ExternCrate(_) => {
if contains_skip(attrs) {
self.push_skipped_with_span(attrs.as_slice(), item.span(), item.span());
self.push_skipped_with_span_non_impl(attrs.as_slice(), item.span(), item.span);
false
} else {
true
Expand All @@ -495,7 +495,11 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
// Module is inline, in this case we treat it like any other item.
_ if !is_mod_decl(item) => {
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
self.push_skipped_with_span_non_impl(
item.attrs.as_slice(),
item.span(),
item.span,
);
false
} else {
true
Expand All @@ -515,7 +519,11 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}
_ => {
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
self.push_skipped_with_span(item.attrs.as_slice(), item.span(), item.span());
self.push_skipped_with_span_non_impl(
item.attrs.as_slice(),
item.span(),
item.span,
);
false
} else {
true
Expand Down Expand Up @@ -672,7 +680,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_non_impl(ti.attrs.as_slice(), ti.span(), ti.span);
return;
}
let skip_context_outer = self.skip_context.clone();
Expand Down Expand Up @@ -732,7 +740,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_impl(ii.attrs.as_slice(), ii.span(), ii.span);
return;
}
let skip_context_outer = self.skip_context.clone();
Expand Down Expand Up @@ -832,28 +840,31 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}

#[allow(clippy::needless_pass_by_value)]
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>) {
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>, trim_start: bool) {
if let Some(ref s) = rewrite {
self.push_str(s);
} else {
let snippet = self.snippet(span);
self.push_str(snippet.trim());
let mut snippet = self.snippet(span).trim_end();
if trim_start {
snippet = snippet.trim_start()
};
self.push_str(snippet);
}
self.last_pos = source!(self, span).hi();
}

pub(crate) fn push_rewrite(&mut self, span: Span, rewrite: Option<String>) {
self.format_missing_with_indent(source!(self, span).lo());
self.push_rewrite_inner(span, rewrite);
self.push_rewrite_inner(span, rewrite, true);
}

pub(crate) fn push_skipped_with_span(
&mut self,
attrs: &[ast::Attribute],
item_span: Span,
main_span: Span,
trim_start: bool,
) {
self.format_missing_with_indent(source!(self, item_span).lo());
// do not take into account the lines with attributes as part of the skipped range
let attrs_end = attrs
.iter()
Expand All @@ -865,13 +876,33 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
// 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);
self.push_rewrite_inner(item_span, None);
self.push_rewrite_inner(item_span, None, trim_start);
let hi = self.line_number + 1;
self.skipped_range
.borrow_mut()
.push(NonFormattedRange::new(lo, hi));
}

pub(crate) fn push_skipped_with_span_impl(
&mut self,
attrs: &[ast::Attribute],
item_span: Span,
main_span: Span,
) {
self.format_missing_before_skip(source!(self, item_span).lo());
self.push_skipped_with_span(attrs, item_span, main_span, false);
}

pub(crate) fn push_skipped_with_span_non_impl(
&mut self,
attrs: &[ast::Attribute],
item_span: Span,
main_span: Span,
) {
self.format_missing_with_indent(source!(self, item_span).lo());
self.push_skipped_with_span(attrs, item_span, main_span, true);
}

pub(crate) fn from_context(ctx: &'a RewriteContext<'_>) -> FmtVisitor<'a> {
let mut visitor = FmtVisitor::from_parse_sess(
ctx.parse_sess,
Expand Down
55 changes: 55 additions & 0 deletions tests/source/skip/skip-and-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// *** "rustfmt::skip" and Function
// *** All source lines starts with indent by 3 tabls.

// "rustfmt::skip" as first line;
#[rustfmt::skip]
/** Comment line 1
comment line 2. */
fn main() {
x = y;
}

// "rustfmt::skip" whithin a block of attributes
fn main() {
#[rustfmt::skip]
// Comment 1
#[allow(non_snake_case)]
// Comment 2
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

fn main() {
// Comment 1
#[rustfmt::skip]
#[allow(non_snake_case)]
// Comment 2
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

fn main() {
// Comment 1
#[allow(non_snake_case)]
// Comment 2
#[allow(non_snake_case)]
#[rustfmt::skip]
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

fn main() {
#[rustfmt::skip]
/** Comment line 1
comment line 2. */
#[allow(non_snake_case)]
x = y;
}

// fn with Doc comment which is attribute vs one-line comment
fn main() {
// Comment for `foo`
#[rustfmt::skip] // comment on why use a skip here
#[allow(non_snake_case)]
pub fn foo(&self) {}
}
86 changes: 86 additions & 0 deletions tests/source/skip/skip-and-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// *** "rustfmt::skip" and Impl

// Original code from issue #4499
#[rustfmt::skip]
/** Comment.

Rustfmt skips this as expected. */
fn main() {
for _ in 1..Foo::getRandomNumber() {
println!("Hello, world!");
}
}

struct Foo;
impl Foo {
#[rustfmt::skip]
/** Comment.

Rustfmt unindents this despite the ::skip above. */
#[allow(non_snake_case)]
pub fn getRandomNumber() -> i32 { 4 }
}

// *** All source lines starts with indent by 3 tabls.

// "rustfmt::skip" as first line;
#[rustfmt::skip]
/** Comment line 1
comment line 2. */
#[allow(non_snake_case)]
impl Foo {
pub fn getRandomNumber() -> i32 { 4 }
}

// "rustfmt::skip" whithin a block of attributes
impl Foo {
#[rustfmt::skip]
// Comment 1
#[allow(non_snake_case)]
// Comment 2
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

impl Foo {
// Comment 1
#[rustfmt::skip]
#[allow(non_snake_case)]
// Comment 2
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

impl Foo {
// Comment 1
#[allow(non_snake_case)]
// Comment 2
#[allow(non_snake_case)]
#[rustfmt::skip]
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

impl Foo {
#[rustfmt::skip]
/** Comment line 1
comment line 2. */
#[allow(non_snake_case)]
pub fn getRandomNumber() -> i32 { 4 }
}

// impl with Doc comment which is attribute vs one-line comment
impl Struct {
/// Documentation for `foo`
#[rustfmt::skip]
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

impl Struct {
// Comment for `foo`
#[rustfmt::skip]
#[allow(non_snake_case)]
pub fn foo(&self) {}
}

18 changes: 18 additions & 0 deletions tests/source/skip/skip-and-import.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// *** "rustfmt::skip" and Imports Use/Extern
// *** All source lines starts with indent by 3 tabls.

#[rustfmt::skip]
use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit};
use crate::config::{BraceStyle, Config};
use crate::formatting::{
attr::*,
modules::{FileModMap, Module},
};

use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit};
#[rustfmt::skip]
use crate::config::{BraceStyle, Config};
use crate::formatting::{
attr::*,
modules::{FileModMap, Module},
};
Loading