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

Fix handling of attribute in enum #6286

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
12 changes: 9 additions & 3 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,17 @@ enum Operation {
/// Print version information
Version,
/// Output default config to a file, or stdout if None
ConfigOutputDefault { path: Option<String> },
ConfigOutputDefault {
path: Option<String>,
},
/// Output current config (as if formatting to a file) to stdout
ConfigOutputCurrent { path: Option<String> },
ConfigOutputCurrent {
path: Option<String>,
},
/// No file specified, read from stdin
Stdin { input: String },
Stdin {
input: String,
},
}

/// Rustfmt operations errors.
Expand Down
85 changes: 60 additions & 25 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::borrow::Cow;
use std::cmp::{max, min, Ordering};

use itertools::Itertools;
use regex::Regex;
use rustc_ast::visit;
use rustc_ast::{ast, ptr};
Expand All @@ -21,7 +22,9 @@ use crate::expr::{
rewrite_assign_rhs_with_comments, rewrite_else_kw_with_comments, rewrite_let_else_block,
RhsAssignKind, RhsTactics,
};
use crate::lists::{definitive_tactic, itemize_list, write_list, ListFormatting, Separator};
use crate::lists::{
definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator,
};
use crate::macros::{rewrite_macro, MacroPosition};
use crate::overflow;
use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
Expand Down Expand Up @@ -617,7 +620,10 @@ impl<'a> FmtVisitor<'a> {
.unwrap_or(&0);

let itemize_list_with = |one_line_width: usize| {
itemize_list(
let mut has_multiline_variant = false;
let mut has_single_line_variant = false;

let items: Vec<_> = itemize_list(
self.snippet_provider,
enum_def.variants.iter(),
"}",
Expand All @@ -631,22 +637,26 @@ impl<'a> FmtVisitor<'a> {
},
|f| f.span.hi(),
|f| {
self.format_variant(f, one_line_width, pad_discrim_ident_to)
.unknown_error()
let (result, is_multi_line) =
self.format_variant(f, one_line_width, pad_discrim_ident_to);
has_multiline_variant |= is_multi_line;
has_single_line_variant |= !is_multi_line;
result.unknown_error()
},
body_lo,
body_hi,
false,
)
.collect()
.collect();

(items, has_multiline_variant, has_single_line_variant)
};
let mut items: Vec<_> = itemize_list_with(self.config.struct_variant_width());

// If one of the variants use multiple lines, use multi-lined formatting for all variants.
let has_multiline_variant = items.iter().any(|item| item.inner_as_ref().contains('\n'));
let has_single_line_variant = items.iter().any(|item| !item.inner_as_ref().contains('\n'));
let (mut items, has_multiline_variant, has_single_line_variant) =
itemize_list_with(self.config.struct_variant_width());

if has_multiline_variant && has_single_line_variant {
items = itemize_list_with(0);
(items, _, _) = itemize_list_with(0);
}

let shape = self.shape().sub_width(2)?;
Expand All @@ -667,23 +677,35 @@ impl<'a> FmtVisitor<'a> {
field: &ast::Variant,
one_line_width: usize,
pad_discrim_ident_to: usize,
) -> Option<String> {
) -> (Option<String>, bool) {
// Makes the early return a little more ergonomic since we can't use `?`
macro_rules! unwrap_early_return {
($option:expr, $is_multi_line:expr) => {
match $option {
Some(v) => v,
None => return (None, $is_multi_line),
}
};
}

let mut is_variant_multi_line = self.snippet(field.span).contains('\n');
if contains_skip(&field.attrs) {
let lo = field.attrs[0].span.lo();
let span = mk_sp(lo, field.span.hi());
return Some(self.snippet(span).to_owned());
return (Some(self.snippet(span).to_owned()), is_variant_multi_line);
}

let context = self.get_context();
let shape = self.shape();
let attrs_str = if context.config.style_edition() >= StyleEdition::Edition2024 {
field.attrs.rewrite(&context, shape)?
unwrap_early_return!(field.attrs.rewrite(&context, shape), is_variant_multi_line)
} else {
// StyleEdition::Edition20{15|18|21} formatting that was off by 1. See issue #5801
field.attrs.rewrite(&context, shape.sub_width(1)?)?
let shape = unwrap_early_return!(shape.sub_width(1), is_variant_multi_line);
unwrap_early_return!(field.attrs.rewrite(&context, shape), is_variant_multi_line)
};
// sub_width(1) to take the trailing comma into account
let shape = shape.sub_width(1)?;
let shape = unwrap_early_return!(shape.sub_width(1), is_variant_multi_line);

let lo = field
.attrs
Expand All @@ -692,32 +714,45 @@ impl<'a> FmtVisitor<'a> {
let span = mk_sp(lo, field.span.lo());

let variant_body = match field.data {
ast::VariantData::Tuple(..) | ast::VariantData::Struct { .. } => format_struct(
&context,
&StructParts::from_variant(field, &context),
self.block_indent,
Some(one_line_width),
)?,
ast::VariantData::Tuple(..) | ast::VariantData::Struct { .. } => {
let rewrite = format_struct(
&context,
&StructParts::from_variant(field, &context),
self.block_indent,
Some(one_line_width),
);
unwrap_early_return!(rewrite, is_variant_multi_line)
}
ast::VariantData::Unit(..) => rewrite_ident(&context, field.ident).to_owned(),
};

let variant_body = if let Some(ref expr) = field.disr_expr {
let lhs = format!("{variant_body:pad_discrim_ident_to$} =");
let ex = &*expr.value;
rewrite_assign_rhs_with(
let rewrite = rewrite_assign_rhs_with(
&context,
lhs,
ex,
shape,
&RhsAssignKind::Expr(&ex.kind, ex.span),
RhsTactics::AllowOverflow,
)?
);
unwrap_early_return!(rewrite, is_variant_multi_line)
} else {
variant_body
};

combine_strs_with_missing_comments(&context, &attrs_str, &variant_body, span, shape, false)
.ok()
is_variant_multi_line = variant_body.contains('\n');
let rewirte = combine_strs_with_missing_comments(
&context,
&attrs_str,
&variant_body,
span,
shape,
false,
)
.ok();
(rewirte, is_variant_multi_line)
}

fn visit_impl_items(&mut self, items: &[ptr::P<ast::AssocItem>]) {
Expand Down
4 changes: 2 additions & 2 deletions src/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ where
I: Iterator<Item = T>,
F1: Fn(&T) -> BytePos,
F2: Fn(&T) -> BytePos,
F3: Fn(&T) -> RewriteResult,
F3: FnMut(&T) -> RewriteResult,
{
type Item = ListItem;

Expand Down Expand Up @@ -810,7 +810,7 @@ where
I: Iterator<Item = T>,
F1: Fn(&T) -> BytePos,
F2: Fn(&T) -> BytePos,
F3: Fn(&T) -> RewriteResult,
F3: FnMut(&T) -> RewriteResult,
{
ListItems {
snippet_provider,
Expand Down
8 changes: 6 additions & 2 deletions src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,14 @@ pub struct ModuleResolutionError {
pub(crate) enum ModuleResolutionErrorKind {
/// Find a file that cannot be parsed.
#[error("cannot parse {file}")]
ParseError { file: PathBuf },
ParseError {
file: PathBuf,
},
/// File cannot be found.
#[error("{file} does not exist")]
NotFound { file: PathBuf },
NotFound {
file: PathBuf,
},
/// File a.rs and a/mod.rs both exist
#[error("file for module found at both {default_path:?} and {secondary_path:?}")]
MultipleCandidates {
Expand Down
7 changes: 7 additions & 0 deletions tests/target/attribute-in-enum/horizontal-no-doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// rustfmt-style_edition: 2024
enum MyType {
A { field1: bool, field2: bool },
B { field1: bool, field2: bool },
C { field1: bool, field2: bool },
D { field1: bool, field2: bool },
}
9 changes: 9 additions & 0 deletions tests/target/attribute-in-enum/horizontal-with-doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// rustfmt-style_edition: 2024
enum MyType {
A { field1: bool, field2: bool },
B { field1: bool, field2: bool },
/// One-line doc comment
C { field1: bool, field2: bool },
/** Documentation block */
D { field1: bool, field2: bool },
}
44 changes: 44 additions & 0 deletions tests/target/attribute-in-enum/vertical-macro-multi-line.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// rustfmt-style_edition: 2024
enum A {
B {
a: usize,
b: usize,
c: usize,
d: usize,
},

#[multiline_macro_attribute(
very_very_long_option1,
very_very_long_option2,
very_very_long_option3
)]
C {
a: usize,
},

#[attr_with_expression1(x = ']')]
D1 {
a: usize,
},

#[attr_with_expression2(x = vec![])]
D2 {
a: usize,
},

#[attr_with_expression3(x = "]")]
D3 {
a: usize,
},

#[attr_with_expression4(x = "\"]")]
D4 {
a: usize,
},

#[attr1]
#[attr2]
D5 {
a: usize,
},
}
14 changes: 14 additions & 0 deletions tests/target/attribute-in-enum/vertical-macro-one-line.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// rustfmt-style_edition: 2024
enum A {
B {
a: usize,
b: usize,
c: usize,
d: usize,
},

#[attr]
C {
a: usize,
},
}
13 changes: 13 additions & 0 deletions tests/target/attribute-in-enum/vertical-no-doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// rustfmt-style_edition: 2024
enum A {
B {
a: usize,
b: usize,
c: usize,
d: usize,
},

C {
a: usize,
},
}
14 changes: 14 additions & 0 deletions tests/target/attribute-in-enum/vertical-with-doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// rustfmt-style_edition: 2024
enum A {
B {
a: usize,
b: usize,
c: usize,
d: usize,
},

/// C
C {
a: usize,
},
}