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

Infinite macro body indentation #4629

Closed
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
39 changes: 38 additions & 1 deletion src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,45 @@ pub(crate) mod visitor;

pub(crate) mod report;

// As `catch_unwind` in `format_snippet()` does not allow the the of a `mut bool`,
// this function is used to `format_input_inner()` using a reference to a new
// `bool` variable, and return its value as part of the function return value.
pub(crate) fn format_input_inner_for_catch_unwind(
input: Input,
config: &Config,
operation_setting: OperationSetting,
is_macro_def: bool,
) -> (Result<FormatReport, OperationError>, bool) {
let mut macro_original_code_was_used = false;
let result = crate::format_input_inner(
input,
&config,
operation_setting,
is_macro_def,
&mut macro_original_code_was_used,
);
return (result, macro_original_code_was_used);
}

pub(crate) fn format_input_inner(
input: Input,
config: &Config,
operation_setting: OperationSetting,
is_macro_def: bool,
macro_original_code_was_used: &mut bool,
) -> Result<FormatReport, OperationError> {
if !config.version_meets_requirement() {
return Err(OperationError::VersionMismatch);
}

rustc_span::with_session_globals(config.edition().into(), || {
format_project(input, config, operation_setting, is_macro_def)
format_project(
input,
config,
operation_setting,
is_macro_def,
macro_original_code_was_used,
)
})
}

Expand All @@ -78,6 +105,7 @@ fn format_project(
config: &Config,
operation_setting: OperationSetting,
is_macro_def: bool,
macro_original_code_was_used: &mut bool,
) -> Result<FormatReport, OperationError> {
let mut timer = Timer::start();

Expand All @@ -87,6 +115,7 @@ fn format_project(
let input_is_stdin = main_file == FileName::Stdin;

let mut parse_session = ParseSess::new(config)?;

if !operation_setting.recursive && parse_session.ignore_file(&main_file) {
format_report.add_ignored_file(main_file);
return Ok(format_report);
Expand Down Expand Up @@ -152,6 +181,7 @@ fn format_project(
&files,
original_snippet.clone(),
is_macro_def,
macro_original_code_was_used,
)?;
}
timer = timer.done_formatting();
Expand All @@ -177,15 +207,18 @@ fn format_file(
file_mod_map: &FileModMap<'_>,
original_snippet: Option<String>,
is_macro_def: bool,
macro_original_code_was_used: &mut bool,
) -> Result<(), OperationError> {
let snippet_provider = parse_session.snippet_provider(module.as_ref().inner);

let mut visitor = FmtVisitor::from_parse_sess(
&parse_session,
config,
&snippet_provider,
file_mod_map,
report.clone(),
);

visitor.is_macro_def = is_macro_def;
visitor.skip_context.update_with_attrs(&krate.attrs);
visitor.last_pos = snippet_provider.start_pos();
Expand Down Expand Up @@ -237,6 +270,10 @@ fn format_file(
);
report.add_format_result(path.clone(), format_result);

if visitor.macro_original_code_was_used.get() {
*macro_original_code_was_used = true;
}

Ok(())
}

Expand Down
11 changes: 8 additions & 3 deletions src/formatting/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,9 +687,13 @@ impl<'a> CommentRewrite<'a> {
let mut config = self.fmt.config.clone();
config.set().wrap_comments(false);
if config.format_code_in_doc_comments() {
if let Some(s) =
format_code_block(&self.code_block_buffer, &config, false)
{
let mut macro_original_code_was_used = false;
if let Some(s) = format_code_block(
&self.code_block_buffer,
&config,
false,
&mut macro_original_code_was_used,
) {
trim_custom_comment_prefix(s.as_ref())
} else {
trim_custom_comment_prefix(&self.code_block_buffer)
Expand Down Expand Up @@ -1604,6 +1608,7 @@ pub(crate) fn recover_comment_removed(
let snippet = context.snippet(span);
let includes_comment = contains_comment(snippet);
if snippet != new && includes_comment && changed_comment_content(snippet, &new) {
context.macro_original_code_was_used.replace(true);
snippet.to_owned()
} else {
new
Expand Down
5 changes: 5 additions & 0 deletions src/formatting/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,11 @@ pub(crate) fn rewrite_block_with_visitor(
.skipped_range
.borrow_mut()
.append(&mut visitor_context.skipped_range.borrow_mut());

if visitor.macro_original_code_was_used.get() {
context.macro_original_code_was_used.replace(true);
}

Some(format!("{}{}{}", prefix, label_str, visitor.buffer))
}

Expand Down
13 changes: 13 additions & 0 deletions src/formatting/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,10 @@ pub(crate) fn format_impl(
result.push_str(&inner_indent_str);
result.push_str(visitor.buffer.trim());
result.push_str(&outer_indent_str);

if visitor.macro_original_code_was_used.get() {
context.macro_original_code_was_used.replace(true);
}
} else if need_newline || !context.config.empty_item_single_line() {
result.push_str(&sep);
}
Expand Down Expand Up @@ -1266,6 +1270,10 @@ pub(crate) fn format_trait(
result.push_str(&inner_indent_str);
result.push_str(visitor.buffer.trim());
result.push_str(&outer_indent_str);

if visitor.macro_original_code_was_used.get() {
context.macro_original_code_was_used.replace(true);
}
} else if result.contains('\n') {
result.push_str(&outer_indent_str);
}
Expand Down Expand Up @@ -3264,6 +3272,11 @@ impl Rewrite for ast::ForeignItem {
defaultness,
Some(&inner_attrs),
);

if visitor.macro_original_code_was_used.get() {
context.macro_original_code_was_used.replace(true);
}

Some(visitor.buffer.to_owned())
}
ast::ForeignItemKind::Fn(_, ref fn_sig, ref generics, None) => rewrite_fn_base(
Expand Down
71 changes: 54 additions & 17 deletions src/formatting/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ fn return_macro_parse_failure_fallback(
) -> Option<String> {
// Mark this as a failure however we format it
context.macro_rewrite_failure.replace(true);

context.macro_original_code_was_used.replace(true);
// Heuristically determine whether the last line of the macro uses "Block" style
// rather than using "Visual" style, or another indentation style.
let is_like_block_indent_style = context
Expand Down Expand Up @@ -229,6 +229,7 @@ pub(crate) fn rewrite_macro(
guard.is_nested(),
)
}));

match result {
Err(..) | Ok(None) => {
context.macro_rewrite_failure.replace(true);
Expand Down Expand Up @@ -510,13 +511,17 @@ pub(crate) fn rewrite_macro_def(
) -> Option<String> {
let snippet = Some(remove_trailing_white_spaces(context.snippet(span)));
if snippet.as_ref().map_or(true, |s| s.ends_with(';')) {
context.macro_original_code_was_used.replace(true);
return snippet;
}
let ts = def.body.inner_tokens();
let mut parser = MacroParser::new(ts.into_trees());
let parsed_def = match parser.parse(def.macro_rules) {
Some(def) => def,
None => return snippet,
None => {
context.macro_original_code_was_used.replace(true);
return snippet;
}
};

let mut result = if def.macro_rules {
Expand All @@ -527,7 +532,6 @@ pub(crate) fn rewrite_macro_def(

result += " ";
result += rewrite_ident(context, ident);

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

let arm_shape = if multi_branch_style {
Expand All @@ -550,6 +554,7 @@ pub(crate) fn rewrite_macro_def(
// if the rewrite returned None because a macro could not be rewritten, then return the
// original body
None if context.macro_rewrite_failure.get() => {
context.macro_original_code_was_used.replace(true);
Some(context.snippet(branch.body).trim().to_string())
}
None => None,
Expand All @@ -576,14 +581,16 @@ pub(crate) fn rewrite_macro_def(

match write_list(&branch_items, &fmt) {
Some(ref s) => result += s,
None => return snippet,
None => {
context.macro_original_code_was_used.replace(true);
return snippet;
}
}

if multi_branch_style {
result += &indent.to_string_with_newline(context.config);
result += "}";
}

Some(result)
}

Expand Down Expand Up @@ -1274,7 +1281,8 @@ impl MacroParser {
branches.push(self.parse_branch(is_macro_rules)?);
}

Some(Macro { branches })
let ret = Some(Macro { branches });
ret
}

// `(` ... `)` `=>` `{` ... `}`
Expand Down Expand Up @@ -1356,6 +1364,10 @@ impl MacroBranch {
result += " =>";
}

// Prepare for restoring original macro body if original code was use during formatting
// (since in this case indentation done by this function doesn't work properly).
let result_without_body = result.clone();

if !context.config.format_macro_bodies() {
result += " ";
result += context.snippet(self.whole_body);
Expand Down Expand Up @@ -1386,23 +1398,44 @@ impl MacroBranch {
config.set().max_width(new_width);

// First try to format as items, then as statements.
let new_body_snippet = match format_snippet(&body_str, &config, true) {
Some(new_body) => new_body,
None => {
let new_width = new_width + config.tab_spaces();
config.set().max_width(new_width);
match format_code_block(&body_str, &config, true) {
Some(new_body) => new_body,
None => return None,

let mut macro_original_code_was_used = false;

let new_body_snippet =
match format_snippet(&body_str, &config, true, &mut macro_original_code_was_used) {
Some(new_body) => new_body,
None => {
let new_width = new_width + config.tab_spaces();
config.set().max_width(new_width);
match format_code_block(
&body_str,
&config,
true,
&mut macro_original_code_was_used,
) {
Some(new_body) => new_body,
None => {
return None;
}
}
}
}
};
};

let new_body = wrap_str(
new_body_snippet.as_ref().to_owned(),
config.max_width(),
shape,
)?;

// If original code was used during the macro body formatting -
// use whole original body without formatting
if macro_original_code_was_used {
result = result_without_body;
result += " ";
result += context.snippet(self.whole_body);
return Some(result);
}

// Indent the body since it is in a block.
let indent_str = body_indent.to_string(&config);
let mut new_body = LineClasses::new(new_body.trim_end())
Expand All @@ -1425,7 +1458,6 @@ impl MacroBranch {
// FIXME: this could be *much* more efficient.
for (old, new) in &substs {
if old_body.contains(new) {
debug!("rewrite_macro_def: bailing matching variable: `{}`", new);
return None;
}
new_body = new_body.replace(new, old);
Expand Down Expand Up @@ -1576,6 +1608,11 @@ fn rewrite_macro_with_items(
result.push_str(&shape.indent.to_string_with_newline(context.config));
result.push_str(closer);
result.push_str(trailing_semicolon);

if visitor.macro_original_code_was_used.get() {
context.macro_original_code_was_used.replace(true);
}

Some(result)
}

Expand Down
1 change: 1 addition & 0 deletions src/formatting/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub(crate) struct RewriteContext<'a> {
pub(crate) snippet_provider: &'a SnippetProvider,
// Used for `format_snippet`
pub(crate) macro_rewrite_failure: Cell<bool>,
pub(crate) macro_original_code_was_used: Cell<bool>,
pub(crate) report: FormatReport,
pub(crate) skip_context: SkipContext,
pub(crate) skipped_range: Rc<RefCell<Vec<NonFormattedRange>>>,
Expand Down
Loading