Skip to content

Commit

Permalink
struct_field_align with shorthand and rest/base
Browse files Browse the repository at this point in the history
This works with the new target files, but breaks in surprising
previously working places.

test with
cargo +nightly-2025-01-02 test system_tests -- --nocapture
  • Loading branch information
janos-r committed Feb 5, 2025
1 parent 6f7aeed commit 6475e44
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 20 deletions.
17 changes: 6 additions & 11 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1620,10 +1620,6 @@ fn rewrite_index(
}
}

fn struct_lit_can_be_aligned(fields: &[ast::ExprField], has_base: bool) -> bool {
!has_base && fields.iter().all(|field| !field.is_shorthand)
}

fn rewrite_struct_lit<'a>(
context: &RewriteContext<'_>,
path: &ast::Path,
Expand Down Expand Up @@ -1660,15 +1656,14 @@ fn rewrite_struct_lit<'a>(

let one_line_width = h_shape.map_or(0, |shape| shape.width);
let body_lo = context.snippet_provider.span_after(span, "{");
let fields_str = if struct_lit_can_be_aligned(fields, has_base_or_rest)
&& context.config.struct_field_align_threshold() > 0
{
let fields_str = if context.config.struct_field_align_threshold() > 0 {
rewrite_with_alignment(
fields,
context,
v_shape,
mk_sp(body_lo, span.hi()),
one_line_width,
Some(struct_rest),
)
.unknown_error()?
} else {
Expand Down Expand Up @@ -1720,10 +1715,10 @@ fn rewrite_struct_lit<'a>(
body_lo,
span.hi(),
false,
);
let item_vec = items.collect::<Vec<_>>();
)
.collect::<Vec<_>>();

let tactic = struct_lit_tactic(h_shape, context, &item_vec);
let tactic = struct_lit_tactic(h_shape, context, &items);
let nested_shape = shape_for_tactic(tactic, h_shape, v_shape);

let ends_with_comma = span_ends_with_comma(context, span);
Expand All @@ -1736,7 +1731,7 @@ fn rewrite_struct_lit<'a>(
force_no_trailing_comma || has_base_or_rest || !context.use_block_indent(),
);

write_list(&item_vec, &fmt)?
write_list(&items, &fmt)?
};

let fields_str =
Expand Down
1 change: 1 addition & 0 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,7 @@ pub(crate) fn format_struct_struct(
Shape::indented(offset.block_indent(context.config), context.config).sub_width_opt(1)?,
mk_sp(body_lo, span.hi()),
one_line_budget,
None
)?;

if !items_str.contains('\n')
Expand Down
8 changes: 7 additions & 1 deletion src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,13 @@ fn system_tests() {
init_log();
run_test_with(&TestSetting::default(), || {
// Get all files in the tests/source directory.
let files = get_test_files(Path::new("tests/source"), true);
// let files = get_test_files(Path::new("tests/source"), true);
let files = vec![
// PathBuf::from("tests/source/issue_6096.rs"),
// PathBuf::from("tests/source/issue_6080.rs")
PathBuf::from("tests/source/configs/struct_field_align_threshold/20.rs")
];
// dbg!(&files);
let (_reports, count, fails) = check_files(files, &None);

// Display results.
Expand Down
81 changes: 73 additions & 8 deletions src/vertical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use itertools::Itertools;
use rustc_ast::ast;
use rustc_span::{BytePos, Span};

use crate::comment::combine_strs_with_missing_comments;
use crate::comment::{FindUncommented, combine_strs_with_missing_comments};
use crate::config::lists::*;
use crate::expr::rewrite_field;
use crate::items::{rewrite_struct_field, rewrite_struct_field_prefix};
Expand Down Expand Up @@ -108,12 +108,13 @@ impl AlignedItem for ast::ExprField {
}
}

pub(crate) fn rewrite_with_alignment<T: AlignedItem>(
pub(crate) fn rewrite_with_alignment<T: AlignedItem + std::fmt::Debug>(
fields: &[T],
context: &RewriteContext<'_>,
shape: Shape,
span: Span,
one_line_width: usize,
struct_rest: Option<&ast::StructRest>,
) -> Option<String> {
let (spaces, group_index) = if context.config.struct_field_align_threshold() > 0 {
group_aligned_items(context, fields)
Expand Down Expand Up @@ -170,12 +171,15 @@ pub(crate) fn rewrite_with_alignment<T: AlignedItem>(
shape.indent,
one_line_width,
force_separator,
struct_rest,
shape,
)?;
if rest.is_empty() {
Some(result + spaces)
} else {
let rest_span = mk_sp(init_last_pos, span.hi());
let rest_str = rewrite_with_alignment(rest, context, shape, rest_span, one_line_width)?;
let rest_str =
rewrite_with_alignment(rest, context, shape, rest_span, one_line_width, struct_rest)?;
Some(format!(
"{}{}\n{}{}",
result,
Expand Down Expand Up @@ -211,6 +215,8 @@ fn rewrite_aligned_items_inner<T: AlignedItem>(
offset: Indent,
one_line_width: usize,
force_trailing_separator: bool,
struct_rest: Option<&ast::StructRest>,
shape: Shape,
) -> Option<String> {
// 1 = ","
let item_shape = Shape::indented(offset, context.config).sub_width_opt(1)?;
Expand All @@ -221,14 +227,71 @@ fn rewrite_aligned_items_inner<T: AlignedItem>(
field_prefix_max_width = 0;
}

enum StructLitField<'a, T> {
Regular(&'a T),
Base(&'a ast::Expr),
Rest(Span),
}

let has_base_or_rest = match struct_rest {
Some(rest) => match rest {
// ast::StructRest::None if fields.is_empty() => return format!("{path_str} {{}}"),
// ast::StructRest::Rest(_) if fields.is_empty() => {
// return Ok(format!("{path_str} {{ .. }}"));
// }
ast::StructRest::Rest(_) | ast::StructRest::Base(_) => true,
_ => false,
},
None => false,
};

let field_iter = fields.iter().map(StructLitField::Regular).chain(
struct_rest
.and_then(|rest| match rest {
ast::StructRest::Base(expr) => Some(StructLitField::Base(&**expr)),
ast::StructRest::Rest(span) => Some(StructLitField::Rest(*span)),
ast::StructRest::None => None,
})
.into_iter(),
);

let span_lo = |item: &StructLitField<'_, T>| match *item {
StructLitField::Regular(field) => field.get_span().lo(),
StructLitField::Base(expr) => {
let last_field_hi = fields
.last()
.map_or(span.lo(), |field| field.get_span().hi());
let snippet = context.snippet(mk_sp(last_field_hi, expr.span.lo()));
let pos = snippet.find_uncommented("..").unwrap();
last_field_hi + BytePos(pos as u32)
}
StructLitField::Rest(span) => span.lo(),
};
let span_hi = |item: &StructLitField<'_, T>| match *item {
StructLitField::Regular(field) => field.get_span().hi(),
StructLitField::Base(expr) => expr.span.hi(),
StructLitField::Rest(span) => span.hi(),
};
let rewrite = |item: &StructLitField<'_, T>| match *item {
StructLitField::Regular(field) => {
field.rewrite_aligned_item(context, item_shape, field_prefix_max_width)
}
StructLitField::Base(expr) => {
// 2 = ..
expr.rewrite_result(context, shape.offset_left(2, span)?)
.map(|s| format!("..{}", s))
}
StructLitField::Rest(_) => Ok("..".to_owned()),
};

let mut items = itemize_list(
context.snippet_provider,
fields.iter(),
field_iter,
"}",
",",
|field| field.get_span().lo(),
|field| field.get_span().hi(),
|field| field.rewrite_aligned_item(context, item_shape, field_prefix_max_width),
span_lo,
span_hi,
rewrite,
span.lo(),
span.hi(),
false,
Expand Down Expand Up @@ -258,6 +321,8 @@ fn rewrite_aligned_items_inner<T: AlignedItem>(

let separator_tactic = if force_trailing_separator {
SeparatorTactic::Always
} else if has_base_or_rest {
SeparatorTactic::Never
} else {
context.config.trailing_comma()
};
Expand All @@ -277,7 +342,7 @@ fn group_aligned_items<T: AlignedItem>(
fields: &[T],
) -> (&'static str, usize) {
let mut index = 0;
for i in 0..fields.len() - 1 {
for i in 0..fields.len().saturating_sub(1) {
if fields[i].skip() {
return ("", index);
}
Expand Down
16 changes: 16 additions & 0 deletions tests/source/issue_6080.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// rustfmt-struct_field_align_threshold: 30

#[derive(Default)]
struct Foo {
id: u8,
age: u8,
name: String,
}

fn main() {
foo = Foo {
id: 5,
name: "John".into(),
..Default::default()
};
}
17 changes: 17 additions & 0 deletions tests/source/issue_6096.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-struct_field_align_threshold: 30

struct Foo {
id: u8,
name: String,
x: u8,
}

fn main() {
let x = 5;

foo = Foo {
id: 3,
x,
name: "John".into(),
};
}
16 changes: 16 additions & 0 deletions tests/target/issue_6080.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// rustfmt-struct_field_align_threshold: 30

#[derive(Default)]
struct Foo {
id: u8,
age: u8,
name: String,
}

fn main() {
foo = Foo {
id: 5,
name: "John".into(),
..Default::default()
};
}
17 changes: 17 additions & 0 deletions tests/target/issue_6096.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-struct_field_align_threshold: 30

struct Foo {
id: u8,
name: String,
x: u8,
}

fn main() {
let x = 5;

foo = Foo {
id: 3,
x,
name: "John".into(),
};
}

0 comments on commit 6475e44

Please sign in to comment.