Skip to content

Commit

Permalink
Auto merge of #18015 - ChayimFriedman2:flip-comma-attribute, r=Veykril
Browse files Browse the repository at this point in the history
Handle attributes correctly in "Flip comma"

Attributes often contain path followed by a token tree (e.g. `align(2)`), and the previous code handled them as two separate items, which led to results such as `#[repr(alignC, (2))]`.

An alternative is to just make the assist unavailable in attributes, like we do in macros. But contrary to macros, attributes often have a fixed form, so this seems useful.

Fixes #18013.
  • Loading branch information
bors committed Sep 1, 2024
2 parents 20ed8ce + 89a39d9 commit b987284
Showing 1 changed file with 61 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use syntax::{algo::non_trivia_sibling, Direction, SyntaxKind, T};
use ide_db::base_db::SourceDatabase;
use syntax::TextSize;
use syntax::{
algo::non_trivia_sibling, ast, AstNode, Direction, SyntaxKind, SyntaxToken, TextRange, T,
};

use crate::{AssistContext, AssistId, AssistKind, Assists};

Expand All @@ -21,6 +25,8 @@ pub(crate) fn flip_comma(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<(
let comma = ctx.find_token_syntax_at_offset(T![,])?;
let prev = non_trivia_sibling(comma.clone().into(), Direction::Prev)?;
let next = non_trivia_sibling(comma.clone().into(), Direction::Next)?;
let (mut prev_text, mut next_text) = (prev.to_string(), next.to_string());
let (mut prev_range, mut next_range) = (prev.text_range(), next.text_range());

// Don't apply a "flip" in case of a last comma
// that typically comes before punctuation
Expand All @@ -34,17 +40,55 @@ pub(crate) fn flip_comma(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<(
return None;
}

if let Some(parent) = comma.parent().and_then(ast::TokenTree::cast) {
// An attribute. It often contains a path followed by a token tree (e.g. `align(2)`), so we have
// to be smarter.
let prev_start =
match comma.siblings_with_tokens(Direction::Prev).skip(1).find(|it| it.kind() == T![,])
{
Some(it) => position_after_token(it.as_token().unwrap()),
None => position_after_token(&parent.left_delimiter_token()?),
};
let prev_end = prev.text_range().end();
let next_start = next.text_range().start();
let next_end =
match comma.siblings_with_tokens(Direction::Next).skip(1).find(|it| it.kind() == T![,])
{
Some(it) => position_before_token(it.as_token().unwrap()),
None => position_before_token(&parent.right_delimiter_token()?),
};
prev_range = TextRange::new(prev_start, prev_end);
next_range = TextRange::new(next_start, next_end);
let file_text = ctx.db().file_text(ctx.file_id().file_id());
prev_text = file_text[prev_range].to_owned();
next_text = file_text[next_range].to_owned();
}

acc.add(
AssistId("flip_comma", AssistKind::RefactorRewrite),
"Flip comma",
comma.text_range(),
|edit| {
edit.replace(prev.text_range(), next.to_string());
edit.replace(next.text_range(), prev.to_string());
edit.replace(prev_range, next_text);
edit.replace(next_range, prev_text);
},
)
}

fn position_before_token(token: &SyntaxToken) -> TextSize {
match non_trivia_sibling(token.clone().into(), Direction::Prev) {
Some(prev_token) => prev_token.text_range().end(),
None => token.text_range().start(),
}
}

fn position_after_token(token: &SyntaxToken) -> TextSize {
match non_trivia_sibling(token.clone().into(), Direction::Next) {
Some(prev_token) => prev_token.text_range().start(),
None => token.text_range().end(),
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -89,4 +133,18 @@ mod tests {
// See https://github.com/rust-lang/rust-analyzer/issues/7693
check_assist_not_applicable(flip_comma, r#"bar!(a,$0 b)"#);
}

#[test]
fn flip_comma_attribute() {
check_assist(
flip_comma,
r#"#[repr(align(2),$0 C)] struct Foo;"#,
r#"#[repr(C, align(2))] struct Foo;"#,
);
check_assist(
flip_comma,
r#"#[foo(bar, baz(1 + 1),$0 qux, other)] struct Foo;"#,
r#"#[foo(bar, qux, baz(1 + 1), other)] struct Foo;"#,
);
}
}

0 comments on commit b987284

Please sign in to comment.