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

Reapply "Initial work on formatting headers" #6457

Open
wants to merge 1 commit 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
111 changes: 111 additions & 0 deletions src/header.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
//! headers are sets of consecutive keywords and tokens, such as
//! `pub const unsafe fn foo` and `pub(crate) unsafe trait Bar`.
//!
//! This module contains general logic for formatting such headers,
//! where they are always placed on a single line except when there
//! are comments between parts of the header.

use std::borrow::Cow;

use rustc_ast as ast;
use rustc_span::Span;
use rustc_span::symbol::Ident;
use tracing::debug;

use crate::comment::{combine_strs_with_missing_comments, contains_comment};
use crate::rewrite::RewriteContext;
use crate::shape::Shape;
use crate::utils::rewrite_ident;

pub(crate) fn format_header(
context: &RewriteContext<'_>,
shape: Shape,
parts: Vec<HeaderPart>,
) -> String {
debug!(?parts, "format_header");
let shape = shape.infinite_width();

// Empty `HeaderPart`s are ignored.
let mut parts = parts.into_iter().filter(|x| !x.snippet.is_empty());
let Some(part) = parts.next() else {
return String::new();
};

let mut result = part.snippet.into_owned();
let mut span = part.span;

for part in parts {
debug!(?result, "before combine");
let comments_span = span.between(part.span);
let comments_snippet = context.snippet(comments_span);
result = if contains_comment(comments_snippet) {
// FIXME(fee1-dead): preserve (potentially misaligned) comments instead of reformatting
// them. Revisit this once we have a strategy for properly dealing with them.
format!("{result}{comments_snippet}{}", part.snippet)
} else {
combine_strs_with_missing_comments(
context,
&result,
&part.snippet,
comments_span,
shape,
true,
)
.unwrap_or_else(|_| format!("{} {}", &result, part.snippet))
};
debug!(?result);
span = part.span;
}

result
}

#[derive(Debug)]
pub(crate) struct HeaderPart {
/// snippet of this part without surrounding space
snippet: Cow<'static, str>,
span: Span,
}
Comment on lines +63 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the snippet need to be a Cow<'static, str>? Could we make it a Cow<'a, str> or are there lifetime issues that prevent that?


impl HeaderPart {
pub(crate) fn new(snippet: impl Into<Cow<'static, str>>, span: Span) -> Self {
Self {
snippet: snippet.into(),
span,
}
}

pub(crate) fn ident(context: &RewriteContext<'_>, ident: Ident) -> Self {
Self {
snippet: rewrite_ident(context, ident).to_owned().into(),
span: ident.span,
}
Comment on lines +79 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be implemented in terms of HeaderPart::new? Would save you needing to call .into().

Suggested change
Self {
snippet: rewrite_ident(context, ident).to_owned().into(),
span: ident.span,
}
let snippet = rewrite_ident(context, ident).to_owned();
Self::new(snippet, ident.span)

If we can make HeaderPart hold a Cow<'a, str>, then this could probably be written as:

Self::new(rewrite_ident(context, ident), ident.span)

}

pub(crate) fn visibility(context: &RewriteContext<'_>, vis: &ast::Visibility) -> Self {
let snippet = match vis.kind {
ast::VisibilityKind::Public => Cow::from("pub"),
ast::VisibilityKind::Inherited => Cow::from(""),
ast::VisibilityKind::Restricted { ref path, .. } => {
let ast::Path { ref segments, .. } = **path;
let mut segments_iter =
segments.iter().map(|seg| rewrite_ident(context, seg.ident));
if path.is_global() {
segments_iter
.next()
.expect("Non-global path in pub(restricted)?");
}
let is_keyword = |s: &str| s == "crate" || s == "self" || s == "super";
let path = segments_iter.collect::<Vec<_>>().join("::");
let in_str = if is_keyword(&path) { "" } else { "in " };

Cow::from(format!("pub({}{})", in_str, path))
Copy link
Contributor

@ytmimi ytmimi Jan 30, 2025

Choose a reason for hiding this comment

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

Not something that we need to fix right now, but we're potentially missing comments within the visibility modifier. e.g pub( /* comment */ crate /* comment */).

}
};

HeaderPart {
snippet,
span: vis.span,
}
}
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ mod emitter;
mod expr;
mod format_report_formatter;
pub(crate) mod formatting;
pub(crate) mod header;
mod ignore_path;
mod imports;
mod items;
Expand Down
20 changes: 14 additions & 6 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::comment::{
use crate::config::StyleEdition;
use crate::config::lists::*;
use crate::expr::{RhsAssignKind, rewrite_array, rewrite_assign_rhs};
use crate::header::{HeaderPart, format_header};
use crate::lists::{ListFormatting, itemize_list, write_list};
use crate::overflow;
use crate::parse::macros::lazy_static::parse_lazy_static;
Expand All @@ -39,7 +40,7 @@ use crate::shape::{Indent, Shape};
use crate::source_map::SpanUtils;
use crate::spanned::Spanned;
use crate::utils::{
NodeIdExt, filtered_str_fits, format_visibility, indent_next_line, is_empty_line, mk_sp,
NodeIdExt, filtered_str_fits, indent_next_line, is_empty_line, mk_sp,
remove_trailing_white_spaces, rewrite_ident, trim_left_preserve_layout,
};
use crate::visitor::FmtVisitor;
Expand Down Expand Up @@ -447,14 +448,21 @@ pub(crate) fn rewrite_macro_def(
None => return snippet,
};

let mut result = if def.macro_rules {
String::from("macro_rules!")
let mut header = if def.macro_rules {
let pos = context.snippet_provider.span_after(span, "macro_rules!");
vec![HeaderPart::new("macro_rules!", span.with_hi(pos))]
} else {
format!("{}macro", format_visibility(context, vis))
let macro_lo = context.snippet_provider.span_before(span, "macro");
let macro_hi = macro_lo + BytePos("macro".len() as u32);
vec![
HeaderPart::visibility(context, vis),
HeaderPart::new("macro", mk_sp(macro_lo, macro_hi)),
]
};

result += " ";
result += rewrite_ident(context, ident);
header.push(HeaderPart::ident(context, ident));

let mut result = format_header(context, shape, header);

let multi_branch_style = def.macro_rules || parsed_def.branches.len() != 1;

Expand Down
26 changes: 26 additions & 0 deletions tests/target/keywords.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
pub // a
macro // b
hi(
// c
) {
// d
}

macro_rules! // a
my_macro {
() => {};
}

// == comments don't get reformatted ==
macro_rules!// a
// b
// c
// d
my_macro {
() => {};
}

macro_rules! /* a block comment */
my_macro {
() => {};
}
Loading