2343: implement assist invert_if r=matklad a=bravomikekilo

fix [issue 2219 invert if condition](https://github.com/rust-analyzer/rust-analyzer/issues/2219)
I put the assist cursor range to `if` of the if expression, because both condition and body will be replaced. Is there any way to replace them without cover the cursor position?
@matklad 

Co-authored-by: bravomikekilo <bmk1221@126.com>
This commit is contained in:
bors[bot] 2019-11-24 08:32:07 +00:00 committed by GitHub
commit 7b6aa7c34e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 174 additions and 35 deletions

View file

@ -1,6 +1,6 @@
use super::invert_if::invert_boolean_expression;
use hir::db::HirDatabase;
use ra_syntax::ast::{self, AstNode};
use ra_syntax::SyntaxNode;
use crate::{Assist, AssistCtx, AssistId};
@ -32,18 +32,18 @@ pub(crate) fn apply_demorgan(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist>
if !cursor_in_range {
return None;
}
let lhs = expr.lhs()?.syntax().clone();
let lhs_range = lhs.text_range();
let rhs = expr.rhs()?.syntax().clone();
let rhs_range = rhs.text_range();
let not_lhs = undo_negation(lhs)?;
let not_rhs = undo_negation(rhs)?;
let lhs = expr.lhs()?;
let lhs_range = lhs.syntax().text_range();
let rhs = expr.rhs()?;
let rhs_range = rhs.syntax().text_range();
let not_lhs = invert_boolean_expression(&lhs)?;
let not_rhs = invert_boolean_expression(&rhs)?;
ctx.add_assist(AssistId("apply_demorgan"), "apply demorgan's law", |edit| {
edit.target(op_range);
edit.replace(op_range, opposite_op);
edit.replace(lhs_range, format!("!({}", not_lhs));
edit.replace(rhs_range, format!("{})", not_rhs));
edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text()));
edit.replace(rhs_range, format!("{})", not_rhs.syntax().text()));
})
}
@ -56,28 +56,6 @@ fn opposite_logic_op(kind: ast::BinOp) -> Option<&'static str> {
}
}
// This function tries to undo unary negation, or inequality
fn undo_negation(node: SyntaxNode) -> Option<String> {
match ast::Expr::cast(node)? {
ast::Expr::BinExpr(bin) => match bin.op_kind()? {
ast::BinOp::NegatedEqualityTest => {
let lhs = bin.lhs()?.syntax().text();
let rhs = bin.rhs()?.syntax().text();
Some(format!("{} == {}", lhs, rhs))
}
_ => None,
},
ast::Expr::PrefixExpr(pe) => match pe.op_kind()? {
ast::PrefixOp::Not => {
let child = pe.expr()?.syntax().text();
Some(String::from(child))
}
_ => None,
},
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;

View file

@ -0,0 +1,102 @@
use hir::db::HirDatabase;
use ra_syntax::ast::{self, AstNode};
use ra_syntax::T;
use crate::{Assist, AssistCtx, AssistId};
// Assist: invert_if
//
// Apply invert_if
// This transforms if expressions of the form `if !x {A} else {B}` into `if x {B} else {A}`
// This also works with `!=`. This assist can only be applied with the cursor
// on `if`.
//
// ```
// fn main() {
// if<|> !y { A } else { B }
// }
// ```
// ->
// ```
// fn main() {
// if y { B } else { A }
// }
// ```
pub(crate) fn invert_if(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
let if_keyword = ctx.find_token_at_offset(T![if])?;
let expr = ast::IfExpr::cast(if_keyword.parent())?;
let if_range = if_keyword.text_range();
let cursor_in_range = ctx.frange.range.is_subrange(&if_range);
if !cursor_in_range {
return None;
}
let cond = expr.condition()?.expr()?;
let then_node = expr.then_branch()?.syntax().clone();
if let ast::ElseBranch::Block(else_block) = expr.else_branch()? {
let flip_cond = invert_boolean_expression(&cond)?;
let cond_range = cond.syntax().text_range();
let else_node = else_block.syntax();
let else_range = else_node.text_range();
let then_range = then_node.text_range();
return ctx.add_assist(AssistId("invert_if"), "invert if branches", |edit| {
edit.target(if_range);
edit.replace(cond_range, flip_cond.syntax().text());
edit.replace(else_range, then_node.text());
edit.replace(then_range, else_node.text());
});
}
None
}
pub(crate) fn invert_boolean_expression(expr: &ast::Expr) -> Option<ast::Expr> {
match expr {
ast::Expr::BinExpr(bin) => match bin.op_kind()? {
ast::BinOp::NegatedEqualityTest => bin.replace_op(T![==]).map(|it| it.into()),
_ => None,
},
ast::Expr::PrefixExpr(pe) => match pe.op_kind()? {
ast::PrefixOp::Not => pe.expr(),
_ => None,
},
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::helpers::{check_assist, check_assist_not_applicable};
#[test]
fn invert_if_remove_inequality() {
check_assist(
invert_if,
"fn f() { i<|>f x != 3 { 1 } else { 3 + 2 } }",
"fn f() { i<|>f x == 3 { 3 + 2 } else { 1 } }",
)
}
#[test]
fn invert_if_remove_not() {
check_assist(
invert_if,
"fn f() { <|>if !cond { 3 * 2 } else { 1 } }",
"fn f() { <|>if cond { 1 } else { 3 * 2 } }",
)
}
#[test]
fn invert_if_doesnt_apply_with_cursor_not_on_if() {
check_assist_not_applicable(invert_if, "fn f() { if !<|>cond { 3 * 2 } else { 1 } }")
}
#[test]
fn invert_if_doesnt_apply_without_negated() {
check_assist_not_applicable(invert_if, "fn f() { i<|>f cond { 3 * 2 } else { 1 } }")
}
}

View file

@ -341,6 +341,23 @@ fn main() {
)
}
#[test]
fn doctest_invert_if() {
check(
"invert_if",
r#####"
fn main() {
if<|> !y { A } else { B }
}
"#####,
r#####"
fn main() {
if y { B } else { A }
}
"#####,
)
}
#[test]
fn doctest_make_raw_string() {
check(

View file

@ -97,6 +97,7 @@ mod assists {
mod add_impl;
mod add_new;
mod apply_demorgan;
mod invert_if;
mod flip_comma;
mod flip_binexpr;
mod flip_trait_bound;
@ -122,6 +123,7 @@ mod assists {
add_impl::add_impl,
add_new::add_new,
apply_demorgan::apply_demorgan,
invert_if::invert_if,
change_visibility::change_visibility,
fill_match_arms::fill_match_arms,
merge_match_arms::merge_match_arms,

View file

@ -13,11 +13,21 @@ use crate::{
make::{self, tokens},
AstNode, TypeBoundsOwner,
},
AstToken, Direction, InsertPosition, SmolStr, SyntaxElement,
AstToken, Direction, InsertPosition, SmolStr, SyntaxElement, SyntaxKind,
SyntaxKind::{ATTR, COMMENT, WHITESPACE},
SyntaxNode, SyntaxToken, T,
};
impl ast::BinExpr {
#[must_use]
pub fn replace_op(&self, op: SyntaxKind) -> Option<ast::BinExpr> {
let op_node: SyntaxElement = self.op_details()?.0.into();
let to_insert: Option<SyntaxElement> = Some(tokens::op(op).into());
let replace_range = RangeInclusive::new(op_node.clone(), op_node);
Some(replace_children(self, replace_range, to_insert.into_iter()))
}
}
impl ast::FnDef {
#[must_use]
pub fn with_body(&self, body: ast::Block) -> ast::FnDef {

View file

@ -127,7 +127,7 @@ pub enum BinOp {
}
impl ast::BinExpr {
fn op_details(&self) -> Option<(SyntaxToken, BinOp)> {
pub fn op_details(&self) -> Option<(SyntaxToken, BinOp)> {
self.syntax().children_with_tokens().filter_map(|it| it.into_token()).find_map(|c| {
let bin_op = match c.kind() {
T![||] => BinOp::BooleanOr,

View file

@ -173,10 +173,21 @@ fn ast_from_text<N: AstNode>(text: &str) -> N {
}
pub mod tokens {
use crate::{AstNode, Parse, SourceFile, SyntaxKind::*, SyntaxToken, T};
use crate::{AstNode, Parse, SourceFile, SyntaxKind, SyntaxKind::*, SyntaxToken, T};
use once_cell::sync::Lazy;
static SOURCE_FILE: Lazy<Parse<SourceFile>> = Lazy::new(|| SourceFile::parse(",\n; ;"));
static SOURCE_FILE: Lazy<Parse<SourceFile>> =
Lazy::new(|| SourceFile::parse("const C: () = (1 != 1, 2 == 2)\n;"));
pub fn op(op: SyntaxKind) -> SyntaxToken {
SOURCE_FILE
.tree()
.syntax()
.descendants_with_tokens()
.filter_map(|it| it.into_token())
.find(|it| it.kind() == op)
.unwrap()
}
pub fn comma() -> SyntaxToken {
SOURCE_FILE

View file

@ -329,6 +329,25 @@ fn main() {
}
```
## `invert_if`
Apply invert_if
This transforms if expressions of the form `if !x {A} else {B}` into `if x {B} else {A}`
This also works with `!=`. This assist can only be applied with the cursor
on `if`.
```rust
// BEFORE
fn main() {
if┃ !y { A } else { B }
}
// AFTER
fn main() {
if y { B } else { A }
}
```
## `make_raw_string`
Adds `r#` to a plain string literal.