Skip to content
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

remove trailing whitespace from multi-line tuple struct field prefix #5708

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
7 changes: 6 additions & 1 deletion src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,14 @@ impl UseTree {
context: &RewriteContext<'_>,
shape: Shape,
) -> Option<String> {
let vis = self.visibility.as_ref().map_or(Cow::from(""), |vis| {
let mut vis = self.visibility.as_ref().map_or(Cow::from(""), |vis| {
crate::utils::format_visibility(context, vis)
});

if !vis.is_empty() && context.config.version() == Version::Two {
vis += " ";
}

let use_str = self
.rewrite(context, shape.offset_left(vis.len())?)
.map(|s| {
Expand Down
130 changes: 91 additions & 39 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,12 @@ impl<'a> FnSig<'a> {
fn to_str(&self, context: &RewriteContext<'_>) -> String {
let mut result = String::with_capacity(128);
// Vis defaultness constness unsafety abi.
result.push_str(&*format_visibility(context, self.visibility));
let vis = format_visibility(context, self.visibility);
result.push_str(&vis);
if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
result.push(' ');
}
result.push_str(format_defaultness(self.defaultness));
result.push_str(format_constness(self.constness));
self.coroutine_kind
Expand Down Expand Up @@ -933,7 +938,12 @@ fn format_impl_ref_and_type(
} = *iimpl;
let mut result = String::with_capacity(128);

result.push_str(&format_visibility(context, &item.vis));
let vis = format_visibility(context, &item.vis);
result.push_str(&vis);
if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
result.push(' ');
}
result.push_str(format_defaultness(defaultness));
result.push_str(format_unsafety(unsafety));

Expand Down Expand Up @@ -1142,12 +1152,16 @@ pub(crate) fn format_trait(
} = **trait_kind;

let mut result = String::with_capacity(128);
let header = format!(
"{}{}{}trait ",
format_visibility(context, &item.vis),
format_unsafety(unsafety),
format_auto(is_auto),
);

let mut vis = format_visibility(context, &item.vis);
if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
vis += " ";
}
let unsafety = format_unsafety(unsafety);
let auto = format_auto(is_auto);

let header = format!("{vis}{unsafety}{auto}trait ");
result.push_str(&header);

let body_lo = context.snippet_provider.span_after(item.span, "{");
Expand Down Expand Up @@ -1350,8 +1364,13 @@ pub(crate) fn format_trait_alias(
// 6 = "trait ", 2 = " ="
let g_shape = shape.offset_left(6)?.sub_width(2)?;
let generics_str = rewrite_generics(context, alias, generics, g_shape)?;
let vis_str = format_visibility(context, vis);
let lhs = format!("{vis_str}trait {generics_str} =");
let vis = format_visibility(context, vis);
let lhs = if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
format!("{vis} trait {generics_str} =")
} else {
format!("{vis}trait {generics_str} =")
};
// 1 = ";"
let trait_alias_bounds = TraitAliasBounds {
generic_bounds,
Expand Down Expand Up @@ -1738,7 +1757,13 @@ fn rewrite_ty<R: Rewrite>(
if !after_where_predicates.is_empty() {
return None;
}
result.push_str(&format!("{}type ", format_visibility(context, vis)));
let vis = format_visibility(context, vis);
result.push_str(&vis);
if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
result.push(' ');
}
result.push_str("type ");
let ident_str = rewrite_ident(context, ident);

if generics.params.is_empty() {
Expand Down Expand Up @@ -1836,18 +1861,22 @@ fn type_annotation_spacing(config: &Config) -> (&str, &str) {
pub(crate) fn rewrite_struct_field_prefix(
context: &RewriteContext<'_>,
field: &ast::FieldDef,
) -> Option<String> {
) -> Option<Cow<'static, str>> {
let vis = format_visibility(context, &field.vis);
let type_annotation_spacing = type_annotation_spacing(context.config);
Some(match field.ident {
Some(name) => format!(
"{}{}{}:",
vis,
rewrite_ident(context, name),
type_annotation_spacing.0
),
None => vis.to_string(),
})

let Some(name) = field.ident else {
return Some(vis);
};

let (space_before_colon, _) = type_annotation_spacing(context.config);
let ident = rewrite_ident(context, name);

let prefix = if !vis.is_empty() && context.config.version() == Version::Two {
format!("{vis} {ident}{space_before_colon}:")
} else {
format!("{vis}{ident}{space_before_colon}:")
};
Some(Cow::from(prefix))
}

impl Rewrite for ast::FieldDef {
Expand Down Expand Up @@ -1899,6 +1928,14 @@ pub(crate) fn rewrite_struct_field(
if prefix.is_empty() && !attrs_str.is_empty() && attrs_extendable && spacing.is_empty() {
spacing.push(' ');
}

if !prefix.is_empty() && field.ident.is_none() && context.config.version() == Version::Two {
// For tuple struct fields, which only have a visibility modifier a space is needed
// when using Version::Two since rewrite_struct_field_prefix won't add a trailing space
// after the visibilty modifier.
spacing.push(' ');
}

let orig_ty = shape
.offset_left(overhead + spacing.len())
.and_then(|ty_shape| field.ty.rewrite(context, ty_shape));
Expand Down Expand Up @@ -1997,15 +2034,18 @@ fn rewrite_static(
offset: Indent,
) -> Option<String> {
let colon = colon_spaces(context.config);
let mut prefix = format!(
"{}{}{} {}{}{}",
format_visibility(context, static_parts.vis),
static_parts.defaultness.map_or("", format_defaultness),
static_parts.prefix,
format_mutability(static_parts.mutability),
rewrite_ident(context, static_parts.ident),
colon,
);

let vis = format_visibility(context, static_parts.vis);
let defaultness = static_parts.defaultness.map_or("", format_defaultness);
let static_prefix = static_parts.prefix;
let mutability = format_mutability(static_parts.mutability);
let ident = rewrite_ident(context, static_parts.ident);
let mut prefix = if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
format!("{vis} {defaultness}{static_prefix} {mutability}{ident}{colon}")
} else {
format!("{vis}{defaultness}{static_prefix} {mutability}{ident}{colon}")
};
// 2 = " =".len()
let ty_shape =
Shape::indented(offset.block_only(), context.config).offset_left(prefix.len() + 2)?;
Expand Down Expand Up @@ -3160,7 +3200,13 @@ fn format_header(
let mut result = String::with_capacity(128);
let shape = Shape::indented(offset, context.config);

result.push_str(format_visibility(context, vis).trim());
let visibility = format_visibility(context, vis);
if context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
result.push_str(&visibility);
} else {
result.push_str(visibility.trim());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that there were other places in the codebase that already dealt with trimming the trailing space coming back from format_visibility.

}

// Check for a missing comment between the visibility and the item name.
let after_vis = vis.span.hi();
Expand Down Expand Up @@ -3346,12 +3392,13 @@ impl Rewrite for ast::ForeignItem {
// function kw here.
let vis = format_visibility(context, &self.vis);
let mut_str = format_mutability(mutability);
let prefix = format!(
"{}static {}{}:",
vis,
mut_str,
rewrite_ident(context, self.ident)
);
let ident = rewrite_ident(context, self.ident);
let prefix = if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
format!("{vis} static {mut_str}{ident}:")
} else {
format!("{vis}static {mut_str}{ident}:")
};
// 1 = ;
rewrite_assign_rhs(
context,
Expand Down Expand Up @@ -3429,7 +3476,12 @@ pub(crate) fn rewrite_mod(
attrs_shape: Shape,
) -> Option<String> {
let mut result = String::with_capacity(32);
result.push_str(&*format_visibility(context, &item.vis));
let vis = format_visibility(context, &item.vis);
result.push_str(&vis);
if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
result.push(' ');
}
result.push_str("mod ");
result.push_str(rewrite_ident(context, item.ident));
result.push(';');
Expand Down
10 changes: 9 additions & 1 deletion src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::comment::{
contains_comment, CharClasses, FindUncommented, FullCodeCharKind, LineClasses,
};
use crate::config::lists::*;
use crate::config::Version;
use crate::expr::{rewrite_array, rewrite_assign_rhs, RhsAssignKind};
use crate::lists::{itemize_list, write_list, ListFormatting};
use crate::overflow;
Expand Down Expand Up @@ -419,6 +420,9 @@ pub(crate) fn rewrite_macro_def(

let mut result = if def.macro_rules {
String::from("macro_rules!")
} else if context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
format!("{} macro", format_visibility(context, vis))
} else {
format!("{}macro", format_visibility(context, vis))
};
Expand Down Expand Up @@ -1369,7 +1373,11 @@ fn format_lazy_static(
let last = parsed_elems.len() - 1;
for (i, (vis, id, ty, expr)) in parsed_elems.iter().enumerate() {
// Rewrite as a static item.
let vis = crate::utils::format_visibility(context, vis);
let mut vis = crate::utils::format_visibility(context, vis);
if !vis.is_empty() && context.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
vis += " ";
};
let mut stmt = String::with_capacity(128);
stmt.push_str(&format!(
"{}static ref {}: {} =",
Expand Down
7 changes: 6 additions & 1 deletion src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub(crate) fn format_visibility(
vis: &Visibility,
) -> Cow<'static, str> {
match vis.kind {
VisibilityKind::Public if context.config.version() == Version::Two => Cow::from("pub"),
VisibilityKind::Public => Cow::from("pub "),
VisibilityKind::Inherited => Cow::from(""),
VisibilityKind::Restricted { ref path, .. } => {
Expand All @@ -69,7 +70,11 @@ pub(crate) fn format_visibility(
let path = segments_iter.collect::<Vec<_>>().join("::");
let in_str = if is_keyword(&path) { "" } else { "in " };

Cow::from(format!("pub({in_str}{path}) "))
if context.config.version() == Version::Two {
Cow::from(format!("pub({in_str}{path})"))
} else {
Cow::from(format!("pub({in_str}{path}) "))
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,10 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
) {
let vis_str = utils::format_visibility(&self.get_context(), vis);
self.push_str(&*vis_str);
if !vis_str.is_empty() && self.config.version() == Version::Two {
// format_visibility doesn't have a trailing space in Version::Two
self.push_str(" ");
}
self.push_str(format_unsafety(unsafety));
self.push_str("mod ");
// Calling `to_owned()` to work around borrow checker.
Expand Down
15 changes: 15 additions & 0 deletions tests/source/issue_5997.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// rustfmt-version: Two

// This formats with two spaces:
pub struct Newtype(
/// Doc
#[doc()] //
pub Vec<u8>,
);

// This formats with one:
pub struct Newtype(
/// Doc
#[doc()]
pub Vec<u8>,
);
9 changes: 9 additions & 0 deletions tests/target/issue_5525.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// rustfmt-version: Two

pub struct SomeCallback(
pub extern "C" fn(
long_argument_name_to_avoid_wrap: u32,
second_long_argument_name: u32,
third_long_argument_name: u32,
),
);
7 changes: 7 additions & 0 deletions tests/target/issue_5703.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// rustfmt-version: Two

#[derive(Clone, Debug, Default)]
pub struct ReactionGroup(
pub(in crate::room::timeline)
IndexMap<(Option<OwnedTransactionId>, Option<OwnedEventId>), OwnedUserId>,
);
15 changes: 15 additions & 0 deletions tests/target/issue_5997.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// rustfmt-version: Two

// This formats with two spaces:
pub struct Newtype(
/// Doc
#[doc()] //
pub Vec<u8>,
);

// This formats with one:
pub struct Newtype(
/// Doc
#[doc()]
pub Vec<u8>,
);
Loading
Loading