From a848e28ff25f3bbebbb04634bbb5dff58df41380 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Wed, 13 Mar 2024 14:52:35 -0400 Subject: [PATCH 1/5] Revert "Initial work on formatting headers" This reverts commit dd301b0c04d5b5ba0fba54671bc115fc34b40862. --- src/header.rs | 102 --------------------------------------- src/lib.rs | 1 - src/macros.rs | 22 +++------ tests/target/keywords.rs | 12 ----- 4 files changed, 7 insertions(+), 130 deletions(-) delete mode 100644 src/header.rs delete mode 100644 tests/target/keywords.rs diff --git a/src/header.rs b/src/header.rs deleted file mode 100644 index fbf4d127be5..00000000000 --- a/src/header.rs +++ /dev/null @@ -1,102 +0,0 @@ -//! 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::symbol::Ident; -use rustc_span::Span; - -use crate::comment::combine_strs_with_missing_comments; -use crate::rewrite::RewriteContext; -use crate::shape::Shape; -use crate::utils::rewrite_ident; - -pub(crate) fn format_header( - context: &RewriteContext<'_>, - shape: Shape, - parts: Vec, -) -> 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"); - result = combine_strs_with_missing_comments( - context, - &result, - &part.snippet, - span.between(part.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, -} - -impl HeaderPart { - pub(crate) fn new(snippet: impl Into>, 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, - } - } - - 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::>().join("::"); - let in_str = if is_keyword(&path) { "" } else { "in " }; - - Cow::from(format!("pub({}{})", in_str, path)) - } - }; - - HeaderPart { - snippet, - span: vis.span, - } - } -} diff --git a/src/lib.rs b/src/lib.rs index 0d1ac7ad958..a67adb1478f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,6 @@ mod emitter; mod expr; mod format_report_formatter; pub(crate) mod formatting; -pub(crate) mod header; mod ignore_path; mod imports; mod items; diff --git a/src/macros.rs b/src/macros.rs index 2cc5c692b22..6e114c76f26 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -27,7 +27,6 @@ use crate::comment::{ use crate::config::lists::*; use crate::config::Version; use crate::expr::{rewrite_array, rewrite_assign_rhs, RhsAssignKind}; -use crate::header::{format_header, HeaderPart}; use crate::lists::{itemize_list, write_list, ListFormatting}; use crate::overflow; use crate::parse::macros::lazy_static::parse_lazy_static; @@ -37,8 +36,8 @@ use crate::shape::{Indent, Shape}; use crate::source_map::SpanUtils; use crate::spanned::Spanned; use crate::utils::{ - filtered_str_fits, indent_next_line, is_empty_line, mk_sp, remove_trailing_white_spaces, - rewrite_ident, trim_left_preserve_layout, NodeIdExt, + filtered_str_fits, format_visibility, indent_next_line, is_empty_line, mk_sp, + remove_trailing_white_spaces, rewrite_ident, trim_left_preserve_layout, NodeIdExt, }; use crate::visitor::FmtVisitor; @@ -419,21 +418,14 @@ pub(crate) fn rewrite_macro_def( None => return snippet, }; - 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))] + let mut result = if def.macro_rules { + String::from("macro_rules!") } else { - 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)), - ] + format!("{}macro", format_visibility(context, vis)) }; - header.push(HeaderPart::ident(context, ident)); - - let mut result = format_header(context, shape, header); + result += " "; + result += rewrite_ident(context, ident); let multi_branch_style = def.macro_rules || parsed_def.branches.len() != 1; diff --git a/tests/target/keywords.rs b/tests/target/keywords.rs deleted file mode 100644 index 22bf7741936..00000000000 --- a/tests/target/keywords.rs +++ /dev/null @@ -1,12 +0,0 @@ -pub // a -macro // b -hi( - // c -) { - // d -} - -macro_rules! // a -my_macro { - () => {}; -} From 728939191e4218e2c1296c7ba3eb36590cbcb9bd Mon Sep 17 00:00:00 2001 From: hanghuge Date: Sat, 6 Apr 2024 20:30:17 +0800 Subject: [PATCH 2/5] chore: fix some typos Signed-off-by: hanghuge --- ci/check_diff.sh | 2 +- tests/mod-resolver/skip-files-issue-5065/main.rs | 2 +- tests/source/no_arg_with_commnet.rs | 2 +- tests/target/no_arg_with_commnet.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ci/check_diff.sh b/ci/check_diff.sh index 93de6040339..2a29cb138ef 100755 --- a/ci/check_diff.sh +++ b/ci/check_diff.sh @@ -87,7 +87,7 @@ function check_diff() { ) if [ -z "$diff" ]; then - echo "no diff detected between rustfmt and the feture branch" + echo "no diff detected between rustfmt and the feature branch" return 0 else echo "$diff" diff --git a/tests/mod-resolver/skip-files-issue-5065/main.rs b/tests/mod-resolver/skip-files-issue-5065/main.rs index 3122e4f220f..f102bf9d181 100644 --- a/tests/mod-resolver/skip-files-issue-5065/main.rs +++ b/tests/mod-resolver/skip-files-issue-5065/main.rs @@ -6,4 +6,4 @@ mod one; fn main() {println!("Hello, world!"); } -// trailing commet +// trailing comment diff --git a/tests/source/no_arg_with_commnet.rs b/tests/source/no_arg_with_commnet.rs index ea4ee0f1eee..41c3c6bea48 100644 --- a/tests/source/no_arg_with_commnet.rs +++ b/tests/source/no_arg_with_commnet.rs @@ -1,2 +1,2 @@ -fn foo( /* cooment */ +fn foo( /* comment */ ) {} diff --git a/tests/target/no_arg_with_commnet.rs b/tests/target/no_arg_with_commnet.rs index 69f61b60f29..21802d87f47 100644 --- a/tests/target/no_arg_with_commnet.rs +++ b/tests/target/no_arg_with_commnet.rs @@ -1 +1 @@ -fn foo(/* cooment */) {} +fn foo(/* comment */) {} From 83496fb3e9494ed64e4f223e1028b8edb47eb586 Mon Sep 17 00:00:00 2001 From: Alexander Glusker Date: Fri, 29 Mar 2024 10:29:39 +0300 Subject: [PATCH 3/5] Ignoring empty statements in closures. Resolve #6116 --- src/closures.rs | 16 ++++++++++++++-- tests/source/issue-6116/main.rs | 7 +++++++ tests/target/issue-6116/main.rs | 3 +++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 tests/source/issue-6116/main.rs create mode 100644 tests/target/issue-6116/main.rs diff --git a/src/closures.rs b/src/closures.rs index 5bf29441b54..0f5dbe23d1c 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -107,7 +107,10 @@ fn get_inner_expr<'a>( if !needs_block(block, prefix, context) { // block.stmts.len() == 1 except with `|| {{}}`; // https://github.com/rust-lang/rustfmt/issues/3844 - if let Some(expr) = block.stmts.first().and_then(stmt_expr) { + if let Some(expr) = iter_stmts_without_empty(&block.stmts) + .next() + .and_then(stmt_expr) + { return get_inner_expr(expr, prefix, context); } } @@ -116,6 +119,15 @@ fn get_inner_expr<'a>( expr } +fn iter_stmts_without_empty( + stmts: &thin_vec::ThinVec, +) -> impl Iterator { + stmts.iter().filter(|x| match x.kind { + crate::ast::StmtKind::Empty => false, + _ => true, + }) +} + // Figure out if a block is necessary. fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) -> bool { let has_attributes = block.stmts.first().map_or(false, |first_stmt| { @@ -123,7 +135,7 @@ fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) - }); is_unsafe_block(block) - || block.stmts.len() > 1 + || iter_stmts_without_empty(&block.stmts).count() > 1 || has_attributes || block_contains_comment(context, block) || prefix.contains('\n') diff --git a/tests/source/issue-6116/main.rs b/tests/source/issue-6116/main.rs new file mode 100644 index 00000000000..910131a4761 --- /dev/null +++ b/tests/source/issue-6116/main.rs @@ -0,0 +1,7 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + ; + a + } +} + diff --git a/tests/target/issue-6116/main.rs b/tests/target/issue-6116/main.rs new file mode 100644 index 00000000000..dc0e48f1bfe --- /dev/null +++ b/tests/target/issue-6116/main.rs @@ -0,0 +1,3 @@ +fn foo() -> fn(i32) -> i32 { + |a| a +} From 97dad9b72fed154636b3cfd9d489a358cda54a56 Mon Sep 17 00:00:00 2001 From: Alexander Glusker Date: Sat, 30 Mar 2024 02:49:01 +0300 Subject: [PATCH 4/5] review patch issue-6116 --- src/closures.rs | 10 +++------- tests/source/issue-6116/main2.rs | 8 ++++++++ tests/source/issue-6116/main3.rs | 8 ++++++++ tests/source/issue-6116/main4.rs | 7 +++++++ tests/target/issue-6116/main2.rs | 6 ++++++ tests/target/issue-6116/main3.rs | 3 +++ tests/target/issue-6116/main4.rs | 6 ++++++ 7 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 tests/source/issue-6116/main2.rs create mode 100644 tests/source/issue-6116/main3.rs create mode 100644 tests/source/issue-6116/main4.rs create mode 100644 tests/target/issue-6116/main2.rs create mode 100644 tests/target/issue-6116/main3.rs create mode 100644 tests/target/issue-6116/main4.rs diff --git a/src/closures.rs b/src/closures.rs index 0f5dbe23d1c..4af8aa95c52 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -1,3 +1,4 @@ +use rustc_ast::ast::StmtKind; use rustc_ast::{ast, ptr}; use rustc_span::Span; use thin_vec::thin_vec; @@ -119,13 +120,8 @@ fn get_inner_expr<'a>( expr } -fn iter_stmts_without_empty( - stmts: &thin_vec::ThinVec, -) -> impl Iterator { - stmts.iter().filter(|x| match x.kind { - crate::ast::StmtKind::Empty => false, - _ => true, - }) +fn iter_stmts_without_empty(stmts: &[ast::Stmt]) -> impl Iterator { + stmts.iter().filter(|x| !matches!(x.kind, StmtKind::Empty)) } // Figure out if a block is necessary. diff --git a/tests/source/issue-6116/main2.rs b/tests/source/issue-6116/main2.rs new file mode 100644 index 00000000000..1114fb1c461 --- /dev/null +++ b/tests/source/issue-6116/main2.rs @@ -0,0 +1,8 @@ +fn bar() -> fn(i32) -> i32 { + |a| { + ; + a; + b + } +} + diff --git a/tests/source/issue-6116/main3.rs b/tests/source/issue-6116/main3.rs new file mode 100644 index 00000000000..dbba9d96164 --- /dev/null +++ b/tests/source/issue-6116/main3.rs @@ -0,0 +1,8 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + ; + ; + ;;;; + a + } +} diff --git a/tests/source/issue-6116/main4.rs b/tests/source/issue-6116/main4.rs new file mode 100644 index 00000000000..754aed66e72 --- /dev/null +++ b/tests/source/issue-6116/main4.rs @@ -0,0 +1,7 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + /*comment before empty statement */; + a + } +} + diff --git a/tests/target/issue-6116/main2.rs b/tests/target/issue-6116/main2.rs new file mode 100644 index 00000000000..89679587ebc --- /dev/null +++ b/tests/target/issue-6116/main2.rs @@ -0,0 +1,6 @@ +fn bar() -> fn(i32) -> i32 { + |a| { + a; + b + } +} diff --git a/tests/target/issue-6116/main3.rs b/tests/target/issue-6116/main3.rs new file mode 100644 index 00000000000..dc0e48f1bfe --- /dev/null +++ b/tests/target/issue-6116/main3.rs @@ -0,0 +1,3 @@ +fn foo() -> fn(i32) -> i32 { + |a| a +} diff --git a/tests/target/issue-6116/main4.rs b/tests/target/issue-6116/main4.rs new file mode 100644 index 00000000000..e2eef35ef7b --- /dev/null +++ b/tests/target/issue-6116/main4.rs @@ -0,0 +1,6 @@ +fn foo() -> fn(i32) -> i32 { + |a| { + /*comment before empty statement */ + a + } +} From e9ddffba4dafce2f5614c187428a204759340e76 Mon Sep 17 00:00:00 2001 From: Alexander Glusker Date: Sat, 30 Mar 2024 06:24:44 +0300 Subject: [PATCH 5/5] 2st review on issue-6116 solvation --- src/closures.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/closures.rs b/src/closures.rs index 4af8aa95c52..44d21ccf325 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -1,4 +1,3 @@ -use rustc_ast::ast::StmtKind; use rustc_ast::{ast, ptr}; use rustc_span::Span; use thin_vec::thin_vec; @@ -108,10 +107,7 @@ fn get_inner_expr<'a>( if !needs_block(block, prefix, context) { // block.stmts.len() == 1 except with `|| {{}}`; // https://github.com/rust-lang/rustfmt/issues/3844 - if let Some(expr) = iter_stmts_without_empty(&block.stmts) - .next() - .and_then(stmt_expr) - { + if let Some(expr) = iter_stmts_without_empty(&block).next().and_then(stmt_expr) { return get_inner_expr(expr, prefix, context); } } @@ -120,8 +116,11 @@ fn get_inner_expr<'a>( expr } -fn iter_stmts_without_empty(stmts: &[ast::Stmt]) -> impl Iterator { - stmts.iter().filter(|x| !matches!(x.kind, StmtKind::Empty)) +fn iter_stmts_without_empty(block: &ast::Block) -> impl Iterator { + block + .stmts + .iter() + .filter(|stmt| !matches!(stmt.kind, ast::StmtKind::Empty)) } // Figure out if a block is necessary. @@ -131,7 +130,7 @@ fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext<'_>) - }); is_unsafe_block(block) - || iter_stmts_without_empty(&block.stmts).count() > 1 + || iter_stmts_without_empty(&block).count() > 1 || has_attributes || block_contains_comment(context, block) || prefix.contains('\n')