Skip to content

Commit

Permalink
Don't remove inner attributes from loop, for and while expressions
Browse files Browse the repository at this point in the history
Fixes 5973

The [attribute] docs in the rust reference explain that inner
attributes are allowed in block expressions.

[attribute]: https://doc.rust-lang.org/reference/attributes.html
  • Loading branch information
ytmimi committed Jan 6, 2024
1 parent 75e3172 commit c947bbc
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 8 deletions.
52 changes: 44 additions & 8 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ pub(crate) fn rewrite_cond(
// Abstraction over control flow expressions
#[derive(Debug)]
struct ControlFlow<'a> {
inner_attributes: Option<Vec<ast::Attribute>>,
cond: Option<&'a ast::Expr>,
block: &'a ast::Block,
else_block: Option<&'a ast::Expr>,
Expand All @@ -669,6 +670,7 @@ fn extract_pats_and_cond(expr: &ast::Expr) -> (Option<&ast::Pat>, &ast::Expr) {

// FIXME: Refactor this.
fn to_control_flow(expr: &ast::Expr, expr_type: ExprType) -> Option<ControlFlow<'_>> {
let inner_attributes = inner_attributes(&expr.attrs);
match expr.kind {
ast::ExprKind::If(ref cond, ref if_block, ref else_block) => {
let (pat, cond) = extract_pats_and_cond(cond);
Expand All @@ -689,14 +691,30 @@ fn to_control_flow(expr: &ast::Expr, expr_type: ExprType) -> Option<ControlFlow<
label,
kind,
} => Some(ControlFlow::new_for(
pat, iter, body, label, expr.span, kind,
inner_attributes,
pat,
iter,
body,
label,
expr.span,
kind,
)),
ast::ExprKind::Loop(ref block, label, _) => Some(ControlFlow::new_loop(
inner_attributes,
block,
label,
expr.span,
)),
ast::ExprKind::Loop(ref block, label, _) => {
Some(ControlFlow::new_loop(block, label, expr.span))
}
ast::ExprKind::While(ref cond, ref block, label) => {
let (pat, cond) = extract_pats_and_cond(cond);
Some(ControlFlow::new_while(pat, cond, block, label, expr.span))
Some(ControlFlow::new_while(
inner_attributes,
pat,
cond,
block,
label,
expr.span,
))
}
_ => None,
}
Expand All @@ -718,6 +736,7 @@ impl<'a> ControlFlow<'a> {
) -> ControlFlow<'a> {
let matcher = choose_matcher(pat);
ControlFlow {
inner_attributes: None,
cond: Some(cond),
block,
else_block,
Expand All @@ -732,8 +751,14 @@ impl<'a> ControlFlow<'a> {
}
}

fn new_loop(block: &'a ast::Block, label: Option<ast::Label>, span: Span) -> ControlFlow<'a> {
fn new_loop(
inner_attributes: Vec<ast::Attribute>,
block: &'a ast::Block,
label: Option<ast::Label>,
span: Span,
) -> ControlFlow<'a> {
ControlFlow {
inner_attributes: Some(inner_attributes),
cond: None,
block,
else_block: None,
Expand All @@ -749,6 +774,7 @@ impl<'a> ControlFlow<'a> {
}

fn new_while(
inner_attributes: Vec<ast::Attribute>,
pat: Option<&'a ast::Pat>,
cond: &'a ast::Expr,
block: &'a ast::Block,
Expand All @@ -757,6 +783,7 @@ impl<'a> ControlFlow<'a> {
) -> ControlFlow<'a> {
let matcher = choose_matcher(pat);
ControlFlow {
inner_attributes: Some(inner_attributes),
cond: Some(cond),
block,
else_block: None,
Expand All @@ -772,6 +799,7 @@ impl<'a> ControlFlow<'a> {
}

fn new_for(
inner_attributes: Vec<ast::Attribute>,
pat: &'a ast::Pat,
cond: &'a ast::Expr,
block: &'a ast::Block,
Expand All @@ -780,6 +808,7 @@ impl<'a> ControlFlow<'a> {
kind: ForLoopKind,
) -> ControlFlow<'a> {
ControlFlow {
inner_attributes: Some(inner_attributes),
cond: Some(cond),
block,
else_block: None,
Expand Down Expand Up @@ -1102,8 +1131,15 @@ impl<'a> Rewrite for ControlFlow<'a> {
};
let block_str = {
let old_val = context.is_if_else_block.replace(self.else_block.is_some());
let result =
rewrite_block_with_visitor(context, "", self.block, None, None, block_shape, true);
let result = rewrite_block_with_visitor(
context,
"",
self.block,
self.inner_attributes.as_deref(),
None,
block_shape,
true,
);
context.is_if_else_block.replace(old_val);
result?
};
Expand Down
19 changes: 19 additions & 0 deletions tests/target/issue_5973.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
fn main() {
while i < days.len() {
#![allow(clippy::indexing_slicing)]
w |= days[i].bit_value();
i += 1;
}

for i in 0..days.len() {
#![allow(clippy::indexing_slicing)]
w |= days[i].bit_value();
i += 1;
}

loop {
#![allow(clippy::indexing_slicing)]
w |= days[i].bit_value();
i += 1;
}
}

0 comments on commit c947bbc

Please sign in to comment.