Repository: rust-analyzer/rust-analyzer
Pull request #2343 (2019-11-21 19:01:00)

implement assist invert_if
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 


Patch:
diff --git a/crates/ra_assists/src/assists/apply_demorgan.rs b/crates/ra_assists/src/assists/apply_demorgan.rs
index 068da1774d..7c57c0560d 100644
--- a/crates/ra_assists/src/assists/apply_demorgan.rs
+++ b/crates/ra_assists/src/assists/apply_demorgan.rs
@@ -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::*;
diff --git a/crates/ra_assists/src/assists/invert_if.rs b/crates/ra_assists/src/assists/invert_if.rs
new file mode 100644
index 0000000000..bababa3e25
--- /dev/null
+++ b/crates/ra_assists/src/assists/invert_if.rs
@@ -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 } }")
+    }
+}
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index f2f0dacbf7..a372bd8b9d 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -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,
diff --git a/crates/ra_mbe/src/syntax_bridge.rs b/crates/ra_mbe/src/syntax_bridge.rs
index 1de399fee4..d1c49c0b3f 100644
--- a/crates/ra_mbe/src/syntax_bridge.rs
+++ b/crates/ra_mbe/src/syntax_bridge.rs
@@ -5,7 +5,6 @@ use ra_syntax::{
     ast, AstNode, AstToken, NodeOrToken, Parse, SmolStr, SyntaxKind, SyntaxKind::*, SyntaxNode,
     SyntaxTreeBuilder, TextRange, TextUnit, T,
 };
-use std::iter::successors;
 use tt::buffer::{Cursor, TokenBuffer};
 
 use crate::subtree_source::SubtreeTokenSource;
@@ -161,31 +160,6 @@ impl Convertor {
 
         let first_child = tt.first_child_or_token()?;
         let last_child = tt.last_child_or_token()?;
-
-        // ignore trivial first_child and last_child
-        let first_child = successors(Some(first_child), |it| {
-            if it.kind().is_trivia() {
-                it.next_sibling_or_token()
-            } else {
-                None
-            }
-        })
-        .last()
-        .unwrap();
-        if first_child.kind().is_trivia() {
-            return Some(tt::Subtree { token_trees: vec![], delimiter: tt::Delimiter::None });
-        }
-
-        let last_child = successors(Some(last_child), |it| {
-            if it.kind().is_trivia() {
-                it.prev_sibling_or_token()
-            } else {
-                None
-            }
-        })
-        .last()
-        .unwrap();
-
         let (delimiter, skip_first) = match (first_child.kind(), last_child.kind()) {
             (T!['('], T![')']) => (tt::Delimiter::Parenthesis, true),
             (T!['{'], T!['}']) => (tt::Delimiter::Brace, true),
@@ -389,7 +363,6 @@ mod tests {
     use super::*;
     use crate::tests::{create_rules, expand};
     use ra_parser::TokenSource;
-    use ra_syntax::algo::{insert_children, InsertPosition};
 
     #[test]
     fn convert_tt_token_source() {
@@ -450,45 +423,4 @@ mod tests {
         let expansion = expand(&rules, "stmts!();");
         assert!(token_tree_to_syntax_node(&expansion, FragmentKind::Expr).is_err());
     }
-
-    #[test]
-    fn test_token_tree_last_child_is_white_space() {
-        let source_file = ast::SourceFile::parse("f!({} );").ok().unwrap();
-        let macro_call = source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
-        let token_tree = macro_call.token_tree().unwrap();
-
-        // Token Tree now is :
-        // TokenTree
-        // - T!['(']
-        // - TokenTree
-        //   - T!['{']
-        //   - T!['}']
-        // - WHITE_SPACE
-        // - T![')']
-
-        let rbrace =
-            token_tree.syntax().descendants_with_tokens().find(|it| it.kind() == T!['}']).unwrap();
-        let space = token_tree
-            .syntax()
-            .descendants_with_tokens()
-            .find(|it| it.kind() == SyntaxKind::WHITESPACE)
-            .unwrap();
-
-        // reorder th white space, such that the white is inside the inner token-tree.
-        let token_tree = insert_children(
-            &rbrace.parent().unwrap(),
-            InsertPosition::Last,
-            &mut std::iter::once(space),
-        );
-
-        // Token Tree now is :
-        // TokenTree
-        // - T!['{']
-        // - T!['}']
-        // - WHITE_SPACE
-        let token_tree = ast::TokenTree::cast(token_tree).unwrap();
-        let tt = ast_to_token_tree(&token_tree).unwrap().0;
-
-        assert_eq!(tt.delimiter, tt::Delimiter::Brace);
-    }
 }
diff --git a/crates/ra_syntax/src/ast/edit.rs b/crates/ra_syntax/src/ast/edit.rs
index 6f005a2d88..95bf9db147 100644
--- a/crates/ra_syntax/src/ast/edit.rs
+++ b/crates/ra_syntax/src/ast/edit.rs
@@ -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 {
diff --git a/crates/ra_syntax/src/ast/expr_extensions.rs b/crates/ra_syntax/src/ast/expr_extensions.rs
index 7c53aa9344..2fd0398378 100644
--- a/crates/ra_syntax/src/ast/expr_extensions.rs
+++ b/crates/ra_syntax/src/ast/expr_extensions.rs
@@ -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,
diff --git a/crates/ra_syntax/src/ast/make.rs b/crates/ra_syntax/src/ast/make.rs
index 9749327fa4..40db570da0 100644
--- a/crates/ra_syntax/src/ast/make.rs
+++ b/crates/ra_syntax/src/ast/make.rs
@@ -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
diff --git a/docs/user/assists.md b/docs/user/assists.md
index 8da7578e2f..6f4c30bee3 100644
--- a/docs/user/assists.md
+++ b/docs/user/assists.md
@@ -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.


Test patch:
diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs
index 176761efb9..3c716c2d12 100644
--- a/crates/ra_assists/src/doc_tests/generated.rs
+++ b/crates/ra_assists/src/doc_tests/generated.rs
@@ -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(