Skip to content

fix additional semicolon during changing parentheses style #6024

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

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
3 changes: 2 additions & 1 deletion src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ pub(crate) fn format_expr(
| ast::ExprKind::MethodCall(..)
| ast::ExprKind::Await(_, _) => rewrite_chain(expr, context, shape),
ast::ExprKind::MacCall(ref mac) => {
rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|| {
let (rewrite, _) = rewrite_macro(mac, None, context, shape, MacroPosition::Expression);
rewrite.or_else(|| {
wrap_str(
context.snippet(expr.span).to_owned(),
context.config.max_width(),
Expand Down
3 changes: 2 additions & 1 deletion src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3374,7 +3374,8 @@ impl Rewrite for ast::ForeignItem {
rewrite_type_alias(ty_alias, context, shape.indent, kind, span)
}
ast::ForeignItemKind::MacCall(ref mac) => {
rewrite_macro(mac, None, context, shape, MacroPosition::Item)
let (rewrite, _) = rewrite_macro(mac, None, context, shape, MacroPosition::Item);
rewrite
}
}?;

Expand Down
44 changes: 24 additions & 20 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ pub(crate) fn rewrite_macro(
context: &RewriteContext<'_>,
shape: Shape,
position: MacroPosition,
) -> Option<String> {
) -> (Option<String>, Option<Delimiter>) {
let should_skip = context
.skip_context
.macros
.skip(context.snippet(mac.path.span));
if should_skip {
None
(None, None)
} else {
let guard = context.enter_macro();
let result = catch_unwind(AssertUnwindSafe(|| {
Expand All @@ -175,27 +175,29 @@ pub(crate) fn rewrite_macro(
)
}));
match result {
Err(..) | Ok(None) => {
Err(..) | Ok((None, ..)) => {
context.macro_rewrite_failure.replace(true);
None
(None, None)
}
Ok(rw) => rw,
}
}
}

//We return not only string but also new delimiter if it changes
//It needs to remove semicolon if delimiter changes in some situations
fn rewrite_macro_inner(
mac: &ast::MacCall,
extra_ident: Option<symbol::Ident>,
context: &RewriteContext<'_>,
shape: Shape,
position: MacroPosition,
is_nested_macro: bool,
) -> Option<String> {
) -> (Option<String>, Option<Delimiter>) {
if context.config.use_try_shorthand() {
if let Some(expr) = convert_try_mac(mac, context) {
context.leave_macro();
return expr.rewrite(context, shape);
return (expr.rewrite(context, shape), None);
}
}

Expand All @@ -213,7 +215,7 @@ fn rewrite_macro_inner(
let ts = mac.args.tokens.clone();
let has_comment = contains_comment(context.snippet(mac.span()));
if ts.is_empty() && !has_comment {
return match style {
let rewrite = match style {
Delimiter::Parenthesis if position == MacroPosition::Item => {
Some(format!("{macro_name}();"))
}
Expand All @@ -225,11 +227,12 @@ fn rewrite_macro_inner(
Delimiter::Brace => Some(format!("{macro_name} {{}}")),
_ => unreachable!(),
};
return (rewrite, Some(style));
}
// Format well-known macros which cannot be parsed as a valid AST.
if macro_name == "lazy_static!" && !has_comment {
if let success @ Some(..) = format_lazy_static(context, shape, ts.clone()) {
return success;
return (success, Some(Delimiter::Brace));
}
}

Expand All @@ -240,17 +243,14 @@ fn rewrite_macro_inner(
} = match parse_macro_args(context, ts, style, is_forced_bracket) {
Some(args) => args,
None => {
return return_macro_parse_failure_fallback(
context,
shape.indent,
position,
mac.span(),
);
let rewrite =
return_macro_parse_failure_fallback(context, shape.indent, position, mac.span());
return (rewrite, None);
}
};

if !arg_vec.is_empty() && arg_vec.iter().all(MacroArg::is_item) {
return rewrite_macro_with_items(
let rewrite = rewrite_macro_with_items(
context,
&arg_vec,
&macro_name,
Expand All @@ -260,9 +260,10 @@ fn rewrite_macro_inner(
position,
mac.span(),
);
return (rewrite, Some(style));
}

match style {
let rewrite = match style {
Delimiter::Parenthesis => {
// Handle special case: `vec!(expr; expr)`
if vec_with_semi {
Expand Down Expand Up @@ -308,20 +309,22 @@ fn rewrite_macro_inner(
force_trailing_comma = Some(SeparatorTactic::Vertical);
};
}
let rewrite = rewrite_array(
let Some(rewrite) = rewrite_array(
&macro_name,
arg_vec.iter(),
mac.span(),
context,
shape,
force_trailing_comma,
Some(original_style),
)?;
) else {
return (None, None);
};

let comma = match position {
MacroPosition::Item => ";",
_ => "",
};

Some(format!("{rewrite}{comma}"))
}
}
Expand All @@ -336,7 +339,8 @@ fn rewrite_macro_inner(
}
}
_ => unreachable!(),
}
};
return (rewrite, Some(style));
}

fn handle_vec_semi(
Expand Down
3 changes: 2 additions & 1 deletion src/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ impl Rewrite for Pat {
shape,
),
PatKind::MacCall(ref mac) => {
rewrite_macro(mac, None, context, shape, MacroPosition::Pat)
let (rewrite, _) = rewrite_macro(mac, None, context, shape, MacroPosition::Pat);
rewrite
}
PatKind::Paren(ref pat) => pat
.rewrite(context, shape.offset_left(1)?.sub_width(1)?)
Expand Down
4 changes: 3 additions & 1 deletion src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,9 @@ impl Rewrite for ast::Ty {
ast::TyKind::BareFn(ref bare_fn) => rewrite_bare_fn(bare_fn, self.span, context, shape),
ast::TyKind::Never => Some(String::from("!")),
ast::TyKind::MacCall(ref mac) => {
rewrite_macro(mac, None, context, shape, MacroPosition::Expression)
let (rewrite, _) =
rewrite_macro(mac, None, context, shape, MacroPosition::Expression);
rewrite
}
ast::TyKind::ImplicitSelf => Some(String::from("")),
ast::TyKind::ImplTrait(_, ref it) => {
Expand Down
37 changes: 23 additions & 14 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,28 +683,38 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {

// 1 = ;
let shape = self.shape().saturating_sub_width(1);
let rewrite = self.with_context(|ctx| rewrite_macro(mac, ident, ctx, shape, pos));
let original_style = macro_style(mac, &self.get_context());
let (rewrite, rewrite_style) =
self.with_context(|ctx| rewrite_macro(mac, ident, ctx, shape, pos));
// As of v638 of the rustc-ap-* crates, the associated span no longer includes
// the trailing semicolon. This determines the correct span to ensure scenarios
// with whitespace between the delimiters and trailing semi (i.e. `foo!(abc) ;`)
// are formatted correctly.
let (span, rewrite) = match macro_style(mac, &self.get_context()) {
Delimiter::Bracket | Delimiter::Parenthesis if MacroPosition::Item == pos => {
let search_span = mk_sp(mac.span().hi(), self.snippet_provider.end_pos());
let hi = self.snippet_provider.span_before(search_span, ";");
let target_span = mk_sp(mac.span().lo(), hi + BytePos(1));
let rewrite = rewrite.map(|rw| {
let rewrite = match rewrite_style.unwrap_or(original_style) {
Delimiter::Bracket | Delimiter::Parenthesis if MacroPosition::Item == pos => rewrite
.map(|rw| {
if !rw.ends_with(';') {
format!("{};", rw)
} else {
rw
}
});
(target_span, rewrite)
}
_ => (mac.span(), rewrite),
}),
_ => rewrite,
};

let span = match original_style {
Delimiter::Bracket | Delimiter::Parenthesis
if (MacroPosition::Item == pos)
|| (MacroPosition::Statement == pos)
&& rewrite_style
.is_some_and(|x| x != original_style && x == Delimiter::Brace) =>
{
let search_span = mk_sp(mac.span().hi(), self.snippet_provider.end_pos());
let hi = self.snippet_provider.span_before(search_span, ";");
mk_sp(mac.span().lo(), hi + BytePos(1))
}
_ => mac.span(),
};
self.push_rewrite(span, rewrite);
}

Expand Down Expand Up @@ -989,13 +999,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}
}

pub(crate) fn with_context<F>(&mut self, f: F) -> Option<String>
pub(crate) fn with_context<F, RESULT>(&mut self, f: F) -> RESULT
where
F: Fn(&RewriteContext<'_>) -> Option<String>,
F: Fn(&RewriteContext<'_>) -> RESULT,
{
let context = self.get_context();
let result = f(&context);

self.macro_rewrite_failure |= context.macro_rewrite_failure.get();
result
}
Expand Down
7 changes: 7 additions & 0 deletions tests/source/issue-6013/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
lazy_static!(
static ref DYNAMODB_CLIENT: Option<aws_sdk_dynamodb::Client> = None;
static ref CASCADE_IP: String = std::env::var("CASCADE_IP").unwrap_or("127.0.0.1".to_string());
static ref CASCADE_PORT: String = std::env::var("CASCADE_PORT").unwrap_or("4000".to_string());
) ;
}
9 changes: 9 additions & 0 deletions tests/target/issue-6013/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {
lazy_static! {
static ref DYNAMODB_CLIENT: Option<aws_sdk_dynamodb::Client> = None;
static ref CASCADE_IP: String =
std::env::var("CASCADE_IP").unwrap_or("127.0.0.1".to_string());
static ref CASCADE_PORT: String =
std::env::var("CASCADE_PORT").unwrap_or("4000".to_string());
}
}