Skip to content

Use blank_lines_* to control newlines between module level attrs #5438

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 3 commits into
base: master
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
17 changes: 17 additions & 0 deletions src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ use self::doc_comment::DocCommentFormatter;
use crate::comment::{contains_comment, rewrite_doc_comment, CommentStyle};
use crate::config::lists::*;
use crate::config::IndentStyle;
use crate::config::Version;
use crate::expr::rewrite_literal;
use crate::lists::{definitive_tactic, itemize_list, write_list, ListFormatting, Separator};
use crate::missed_spans::push_vertical_spaces;
use crate::overflow;
use crate::rewrite::{Rewrite, RewriteContext};
use crate::shape::Shape;
Expand Down Expand Up @@ -419,6 +421,7 @@ impl Rewrite for [ast::Attribute] {
shape.with_max_width(context.config),
context,
0,
false,
)?;
let comment = if comment.is_empty() {
format!("\n{}", mlb)
Expand Down Expand Up @@ -449,6 +452,7 @@ impl Rewrite for [ast::Attribute] {
shape.with_max_width(context.config),
context,
0,
false,
)?;
result.push_str(&comment);
if let Some(next) = attrs.get(derives.len()) {
Expand Down Expand Up @@ -476,12 +480,25 @@ impl Rewrite for [ast::Attribute] {
let missing_span = attrs
.get(1)
.map(|next| mk_sp(attrs[0].span.hi(), next.span.lo()));

let mut vertical_lines_pushed = 0;
if let Some(missing_span) = missing_span {
let snippet = context.snippet(missing_span);
if let Some((whitespace, _)) = snippet.split_once(|c: char| !c.is_whitespace()) {
let newlines = count_newlines(whitespace);
// Version gate this change as it leads to breaking changes with Version::One
if newlines > 0 && context.config.version() == Version::Two {
vertical_lines_pushed =
push_vertical_spaces(&mut result, context.config, newlines);
}
}
// remove leading newlines if we already pushed vertical space into the result
let comment = crate::comment::recover_missing_comment_in_span(
missing_span,
shape.with_max_width(context.config),
context,
0,
vertical_lines_pushed > 0,
)?;
result.push_str(&comment);
if let Some(next) = attrs.get(1) {
Expand Down
5 changes: 4 additions & 1 deletion src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,7 @@ pub(crate) fn recover_missing_comment_in_span(
shape: Shape,
context: &RewriteContext<'_>,
used_width: usize,
remove_leading_newlines: bool,
) -> Option<String> {
let missing_comment = rewrite_missing_comment(span, shape, context)?;
if missing_comment.is_empty() {
Expand All @@ -976,7 +977,9 @@ pub(crate) fn recover_missing_comment_in_span(
let total_width = missing_comment.len() + used_width + 1;
let force_new_line_before_comment =
missing_snippet[..pos].contains('\n') || total_width > context.config.max_width();
let sep = if force_new_line_before_comment {
let sep = if remove_leading_newlines && force_new_line_before_comment {
shape.indent.to_string(context.config)
} else if force_new_line_before_comment {
shape.indent.to_string_with_newline(context.config)
} else {
Cow::from(" ")
Expand Down
3 changes: 3 additions & 0 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,7 @@ pub(crate) fn format_impl(
Shape::indented(offset, context.config),
context,
last_line_width(&result),
false,
) {
Some(ref missing_comment) if !missing_comment.is_empty() => {
result.push_str(missing_comment);
Expand Down Expand Up @@ -1092,6 +1093,7 @@ pub(crate) fn format_trait(
Shape::indented(offset, context.config),
context,
last_line_width(&result),
false,
) {
Some(ref missing_comment) if !missing_comment.is_empty() => {
result.push_str(missing_comment);
Expand Down Expand Up @@ -2482,6 +2484,7 @@ fn rewrite_fn_base(
shape,
context,
last_line_width(&result),
false,
) {
Some(ref missing_comment) if !missing_comment.is_empty() => {
result.push_str(missing_comment);
Expand Down
53 changes: 32 additions & 21 deletions src/missed_spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::shape::{Indent, Shape};
use crate::source_map::LineRangeUtils;
use crate::utils::{count_lf_crlf, count_newlines, last_line_width, mk_sp};
use crate::visitor::FmtVisitor;
use crate::Config;

struct SnippetStatus {
/// An offset to the current line from the beginning of the original snippet.
Expand Down Expand Up @@ -112,27 +113,9 @@ impl<'a> FmtVisitor<'a> {
}
}

fn push_vertical_spaces(&mut self, mut newline_count: usize) {
let offset = self.buffer.chars().rev().take_while(|c| *c == '\n').count();
let newline_upper_bound = self.config.blank_lines_upper_bound() + 1;
let newline_lower_bound = self.config.blank_lines_lower_bound() + 1;

if newline_count + offset > newline_upper_bound {
if offset >= newline_upper_bound {
newline_count = 0;
} else {
newline_count = newline_upper_bound - offset;
}
} else if newline_count + offset < newline_lower_bound {
if offset >= newline_lower_bound {
newline_count = 0;
} else {
newline_count = newline_lower_bound - offset;
}
}

let blank_lines = "\n".repeat(newline_count);
self.push_str(&blank_lines);
fn push_vertical_spaces(&mut self, newline_count: usize) {
let newlines_pushed = push_vertical_spaces(&mut self.buffer, self.config, newline_count);
self.line_number += newlines_pushed;
}

fn write_snippet<F>(&mut self, span: Span, process_last_snippet: F)
Expand Down Expand Up @@ -361,3 +344,31 @@ impl<'a> FmtVisitor<'a> {
}
}
}

pub(crate) fn push_vertical_spaces(
buffer: &mut String,
config: &Config,
mut newline_count: usize,
) -> usize {
let offset = buffer.chars().rev().take_while(|c| *c == '\n').count();
let newline_upper_bound = config.blank_lines_upper_bound() + 1;
let newline_lower_bound = config.blank_lines_lower_bound() + 1;

if newline_count + offset > newline_upper_bound {
if offset >= newline_upper_bound {
newline_count = 0;
} else {
newline_count = newline_upper_bound - offset;
}
} else if newline_count + offset < newline_lower_bound {
if offset >= newline_lower_bound {
newline_count = 0;
} else {
newline_count = newline_lower_bound - offset;
}
}

let blank_lines = "\n".repeat(newline_count);
buffer.push_str(&blank_lines);
newline_count
}
18 changes: 18 additions & 0 deletions tests/source/issue-4082/version_one/original_example.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// rustfmt-version: One

#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]

// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]
19 changes: 19 additions & 0 deletions tests/source/issue-4082/version_two/add_newlines.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// rustfmt-version: Two
// rustfmt-blank_lines_upper_bound: 2
// rustfmt-blank_lines_lower_bound: 2
#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]

// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]
23 changes: 23 additions & 0 deletions tests/source/issue-4082/version_two/remove_newlines.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// rustfmt-version: Two
// rustfmt-blank_lines_upper_bound: 2
// rustfmt-blank_lines_lower_bound: 2
#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]





// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]
17 changes: 17 additions & 0 deletions tests/target/issue-4082/version_one/original_example.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-version: One

#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]
// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]
20 changes: 20 additions & 0 deletions tests/target/issue-4082/version_two/add_newlines.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// rustfmt-version: Two
// rustfmt-blank_lines_upper_bound: 2
// rustfmt-blank_lines_lower_bound: 2
#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]


// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]
18 changes: 18 additions & 0 deletions tests/target/issue-4082/version_two/original_example.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// rustfmt-version: Two

#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]

// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]
20 changes: 20 additions & 0 deletions tests/target/issue-4082/version_two/remove_newlines.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// rustfmt-version: Two
// rustfmt-blank_lines_upper_bound: 2
// rustfmt-blank_lines_lower_bound: 2
#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]
#![feature(panic_unwind)]
#![feature(staged_api)]
#![feature(std_internals)]
#![feature(unwind_attributes)]
#![feature(abi_thiscall)]
#![feature(rustc_attrs)]
#![feature(raw)]
#![panic_runtime]
#![feature(panic_runtime)]


// `real_imp` is unused with Miri, so silence warnings.
#![cfg_attr(miri, allow(dead_code))]