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
104 changes: 92 additions & 12 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 Down Expand Up @@ -647,28 +648,107 @@ impl<'a> FmtVisitor<'a> {
// If one of the variants use multiple lines, use multi-lined formatting for all variants.
let is_multi_line_variant = |item: &ListItem| -> bool {
let variant_str = item.inner_as_ref();
if self.config.style_edition() < StyleEdition::Edition2024 {
if self.config.style_edition() <= StyleEdition::Edition2021 {
// Fall back to previous naive implementation (#5662) because of
// rustfmt's stability guarantees
return variant_str.contains('\n');
}

let mut first_line_is_read = false;
for line in variant_str.split('\n') {
if first_line_is_read {
return false;
// First exclude all outer one-line doc comments
let mut lines = variant_str
.split('\n')
.filter(|line| !line.trim().starts_with("///"));

let mut variant_str = lines.join("\n");

// Then exclude all outer documentation blocks
// Exclude one block per loop iteration
loop {
let mut block_found = false;
let mut chars = variant_str.chars().enumerate();
'block: while let Some((i, c)) = chars.next() {
if c != '/' {
continue;
}
let block_start = i;
if let Some((_, '*')) = chars.next() {
if let Some((_, '*')) = chars.next() {
while let Some((_, c)) = chars.next() {
if c == '*' {
if let Some((i, '/')) = chars.next() {
// block was found and ends at the i-th position
// We remove it from variant_str
let mut s = variant_str[..block_start].trim().to_owned();
s.push_str(variant_str[(i + 1)..].trim());
variant_str = s;
block_found = true;
break 'block;
}
}
}
}
}
}

// skip rustdoc comments and macro attributes
let line = line.trim_start();
if line.starts_with("///") || line.starts_with("#") {
continue;
} else {
first_line_is_read = true;
if !block_found {
break;
}
}

// Skip macro attributes in variant_str
// We skip one macro attribute per loop iteration
loop {
let mut macro_attribute_found = false;
let mut macro_attribute_start_i = 0;
let mut bracket_count = 0;
let mut chars = variant_str.chars().enumerate();
while let Some((i, c)) = chars.next() {
match c {
'#' => {
if let Some((_, '[')) = chars.next() {
macro_attribute_start_i = i;
bracket_count += 1;
}
}
'[' => bracket_count += 1,
']' => {
bracket_count -= 1;
if bracket_count == 0 {
// Macro attribute was found and ends at the i-th position
// We remove it from variant_str
let mut s =
variant_str[..macro_attribute_start_i].trim().to_owned();
s.push_str(variant_str[(i + 1)..].trim());
variant_str = s;
macro_attribute_found = true;
break;
}
}
'\'' => {
// Handle char in attribute
chars.next();
chars.next();
}
'"' => {
// Handle quoted strings within attribute
while let Some((_, c)) = chars.next() {
if c == '\\' {
chars.next(); // Skip escaped character
} else if c == '"' {
break; // end of string
}
}
}
_ => {}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This seemed pretty complicated and it was hard to tell how accurate this was. I thought it might be simpler if we just got the multi-lined property when rewriting the variant body so we wouldn't need to parse it out after the fact. I've pushed one commit as a proof of concept / reference for you to take a look at. Hopefully it'll serve as a good jumping off point for you.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. Will look at it when I have time (probably next week).


if !macro_attribute_found {
break;
}
}

true
variant_str.contains('\n')
};
let has_multiline_variant = items.iter().any(is_multi_line_variant);
let has_single_line_variant = items.iter().any(|item| !is_multi_line_variant(item));
Expand Down
3 changes: 2 additions & 1 deletion tests/target/attribute-in-enum/horizontal-with-doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
enum MyType {
A { field1: bool, field2: bool },
B { field1: bool, field2: bool },
/// OMG a comment
/// 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,
},
}
Loading