5 files changed, 235 insertions, 59 deletions

diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs
index 4435c4b5d..e13f54c4f 100644
--- a/crates/ra_assists/src/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/add_missing_impl_members.rs
@@ -9,7 +9,35 @@ use ra_fmt::{leading_indent, reindent};
 
 use itertools::Itertools;
 
-pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
+enum AddMissingImplMembersMode {
+    DefaultMethodsOnly,
+    NoDefaultMethods,
+}
+
+pub(crate) fn add_missing_impl_members(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
+    add_missing_impl_members_inner(
+        ctx,
+        AddMissingImplMembersMode::NoDefaultMethods,
+        "add_impl_missing_members",
+        "add missing impl members",
+    )
+}
+
+pub(crate) fn add_missing_default_members(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
+    add_missing_impl_members_inner(
+        ctx,
+        AddMissingImplMembersMode::DefaultMethodsOnly,
+        "add_impl_default_members",
+        "add impl default members",
+    )
+}
+
+fn add_missing_impl_members_inner(
+    mut ctx: AssistCtx<impl HirDatabase>,
+    mode: AddMissingImplMembersMode,
+    assist_id: &'static str,
+    label: &'static str,
+) -> Option<Assist> {
     let impl_node = ctx.node_at_offset::<ast::ImplBlock>()?;
     let impl_item_list = impl_node.item_list()?;
 
@@ -35,6 +63,10 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx<impl HirDatabase>) ->
         trait_fns
             .into_iter()
             .filter(|t| def_name(t).is_some())
+            .filter(|t| match mode {
+                AddMissingImplMembersMode::DefaultMethodsOnly => t.body().is_some(),
+                AddMissingImplMembersMode::NoDefaultMethods => t.body().is_none(),
+            })
             .filter(|t| impl_fns.iter().all(|i| def_name(i) != def_name(t)))
             .collect()
     };
@@ -42,7 +74,7 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx<impl HirDatabase>) ->
         return None;
     }
 
-    ctx.add_action(AssistId("add_impl_missing_members"), "add missing impl members", |edit| {
+    ctx.add_action(AssistId(assist_id), label, |edit| {
         let (parent_indent, indent) = {
             // FIXME: Find a way to get the indent already used in the file.
             // Now, we copy the indent of first item or indent with 4 spaces relative to impl block
@@ -103,11 +135,13 @@ fn build_func_body(def: &ast::FnDef) -> String {
     let mut buf = String::new();
 
     for child in def.syntax().children() {
-        if child.kind() == SyntaxKind::SEMI {
-            buf.push_str(" { unimplemented!() }")
-        } else {
-            child.text().push_to(&mut buf);
-        }
+        match (child.prev_sibling().map(|c| c.kind()), child.kind()) {
+            (_, SyntaxKind::SEMI) => buf.push_str(" { unimplemented!() }"),
+            (_, SyntaxKind::ATTR) | (_, SyntaxKind::COMMENT) => {}
+            (Some(SyntaxKind::ATTR), SyntaxKind::WHITESPACE)
+            | (Some(SyntaxKind::COMMENT), SyntaxKind::WHITESPACE) => {}
+            _ => child.text().push_to(&mut buf),
+        };
     }
 
     buf.trim_end().to_string()
@@ -180,8 +214,7 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    fn foo(&self) { unimplemented!() }
-    fn baz(&self) -> u32 { 42 }<|>
+    fn foo(&self) { unimplemented!() }<|>
 }",
         );
     }
@@ -193,7 +226,7 @@ impl Foo for S {
             "
 trait Foo { fn foo(&self); }
 struct S;
-impl Foo for S {<|>}",
+impl Foo for S { <|> }",
             "
 trait Foo { fn foo(&self); }
 struct S;
@@ -232,8 +265,8 @@ impl Foo for S { <|> }",
     }
 
     #[test]
-    fn test_ignore_unnamed_trait_members() {
-        check_assist(
+    fn test_ignore_unnamed_trait_members_and_default_methods() {
+        check_assist_not_applicable(
             add_missing_impl_members,
             "
 trait Foo {
@@ -242,15 +275,6 @@ trait Foo {
 }
 struct S;
 impl Foo for S { <|> }",
-            "
-trait Foo {
-    fn (arg: u32);
-    fn valid(some: u32) -> bool { false }
-}
-struct S;
-impl Foo for S {
-    fn valid(some: u32) -> bool { false }<|>
-}",
         )
     }
 
@@ -260,7 +284,7 @@ impl Foo for S {
             add_missing_impl_members,
             "
 trait Foo {
-    fn valid(some: u32) -> bool { false }
+    fn valid(some: u32) -> bool;
 }
 struct S;
 
@@ -269,15 +293,66 @@ mod my_mod {
 }",
             "
 trait Foo {
-    fn valid(some: u32) -> bool { false }
+    fn valid(some: u32) -> bool;
 }
 struct S;
 
 mod my_mod {
     impl crate::Foo for S {
-        fn valid(some: u32) -> bool { false }<|>
+        fn valid(some: u32) -> bool { unimplemented!() }<|>
+    }
+}",
+        )
+    }
+
+    #[test]
+    fn test_with_docstring_and_attrs() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+#[doc(alias = "test alias")]
+trait Foo {
+    /// doc string
+    #[must_use]
+    fn foo(&self);
+}
+struct S;
+impl Foo for S {}<|>"#,
+            r#"
+#[doc(alias = "test alias")]
+trait Foo {
+    /// doc string
+    #[must_use]
+    fn foo(&self);
+}
+struct S;
+impl Foo for S {
+    fn foo(&self) { unimplemented!() }<|>
+}"#,
+        )
     }
+
+    #[test]
+    fn test_default_methods() {
+        check_assist(
+            add_missing_default_members,
+            "
+trait Foo {
+    fn valid(some: u32) -> bool { false }
+    fn foo(some: u32) -> bool;
+}
+struct S;
+impl Foo for S { <|> }",
+            "
+trait Foo {
+    fn valid(some: u32) -> bool { false }
+    fn foo(some: u32) -> bool;
+}
+struct S;
+impl Foo for S {
+    fn valid(some: u32) -> bool { false }<|>
 }",
         )
     }
+
 }
diff --git a/crates/ra_assists/src/flip_eq_operands.rs b/crates/ra_assists/src/flip_eq_operands.rs
new file mode 100644
index 000000000..df0bb689d
--- /dev/null
+++ b/crates/ra_assists/src/flip_eq_operands.rs
@@ -0,0 +1,86 @@
+use hir::db::HirDatabase;
+use ra_syntax::ast::{AstNode, BinExpr, BinOp};
+
+use crate::{AssistCtx, Assist, AssistId};
+
+pub(crate) fn flip_eq_operands(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
+    let expr = ctx.node_at_offset::<BinExpr>()?;
+    let lhs = expr.lhs()?.syntax();
+    let rhs = expr.rhs()?.syntax();
+    let op_range = expr.op()?.range();
+    let cursor_in_range = ctx.frange.range.is_subrange(&op_range);
+    let allowed_ops = [BinOp::EqualityTest, BinOp::NegatedEqualityTest];
+    let expr_op = expr.op_kind()?;
+    if !cursor_in_range || !allowed_ops.iter().any(|o| *o == expr_op) {
+        return None;
+    }
+    ctx.add_action(AssistId("flip_eq_operands"), "flip equality operands", |edit| {
+        edit.target(op_range);
+        edit.replace(lhs.range(), rhs.text());
+        edit.replace(rhs.range(), lhs.text());
+    });
+
+    ctx.build()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::helpers::{check_assist, check_assist_target};
+
+    #[test]
+    fn flip_eq_operands_for_simple_stmt() {
+        check_assist(
+            flip_eq_operands,
+            "fn f() { let res = 1 ==<|> 2; }",
+            "fn f() { let res = 2 ==<|> 1; }",
+        )
+    }
+
+    #[test]
+    fn flip_neq_operands_for_simple_stmt() {
+        check_assist(
+            flip_eq_operands,
+            "fn f() { let res = 1 !=<|> 2; }",
+            "fn f() { let res = 2 !=<|> 1; }",
+        )
+    }
+
+    #[test]
+    fn flip_eq_operands_for_complex_stmt() {
+        check_assist(
+            flip_eq_operands,
+            "fn f() { let res = (1 + 1) ==<|> (2 + 2); }",
+            "fn f() { let res = (2 + 2) ==<|> (1 + 1); }",
+        )
+    }
+
+    #[test]
+    fn flip_eq_operands_in_match_expr() {
+        check_assist(
+            flip_eq_operands,
+            r#"
+            fn dyn_eq(&self, other: &dyn Diagnostic) -> bool {
+                match other.downcast_ref::<Self>() {
+                    None => false,
+                    Some(it) => it ==<|> self,
+                }
+            }
+            "#,
+            r#"
+            fn dyn_eq(&self, other: &dyn Diagnostic) -> bool {
+                match other.downcast_ref::<Self>() {
+                    None => false,
+                    Some(it) => self ==<|> it,
+                }
+            }
+            "#,
+        )
+    }
+
+    #[test]
+    fn flip_eq_operands_target() {
+        check_assist_target(flip_eq_operands, "fn f() { let res = 1 ==<|> 2; }", "==")
+    }
+}
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 871b37f58..8900fbc4b 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -88,6 +88,7 @@ where
 mod add_derive;
 mod add_impl;
 mod flip_comma;
+mod flip_eq_operands;
 mod change_visibility;
 mod fill_match_arms;
 mod fill_struct_fields;
@@ -106,12 +107,14 @@ fn all_assists<DB: HirDatabase>() -> &'static [fn(AssistCtx<DB>) -> Option<Assis
         fill_match_arms::fill_match_arms,
         fill_struct_fields::fill_struct_fields,
         flip_comma::flip_comma,
+        flip_eq_operands::flip_eq_operands,
         introduce_variable::introduce_variable,
         replace_if_let_with_match::replace_if_let_with_match,
         split_import::split_import,
         remove_dbg::remove_dbg,
         auto_import::auto_import,
         add_missing_impl_members::add_missing_impl_members,
+        add_missing_impl_members::add_missing_default_members,
     ]
 }
 
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index 703d99d9b..c37fd0454 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -680,7 +680,7 @@ impl ExprCollector {
             }
             ast::ExprKind::PrefixExpr(e) => {
                 let expr = self.collect_expr_opt(e.expr());
-                if let Some(op) = e.op() {
+                if let Some(op) = e.op_kind() {
                     self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr)
                 } else {
                     self.alloc_expr(Expr::Missing, syntax_ptr)
@@ -703,7 +703,7 @@ impl ExprCollector {
             ast::ExprKind::BinExpr(e) => {
                 let lhs = self.collect_expr_opt(e.lhs());
                 let rhs = self.collect_expr_opt(e.rhs());
-                let op = e.op();
+                let op = e.op_kind();
                 self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr)
             }
             ast::ExprKind::TupleExpr(e) => {
diff --git a/crates/ra_syntax/src/ast.rs b/crates/ra_syntax/src/ast.rs
index d8c2cb063..226208700 100644
--- a/crates/ra_syntax/src/ast.rs
+++ b/crates/ra_syntax/src/ast.rs
@@ -521,7 +521,7 @@ pub enum PrefixOp {
 }
 
 impl PrefixExpr {
-    pub fn op(&self) -> Option<PrefixOp> {
+    pub fn op_kind(&self) -> Option<PrefixOp> {
         match self.syntax().first_child()?.kind() {
             STAR => Some(PrefixOp::Deref),
             EXCL => Some(PrefixOp::Not),
@@ -529,6 +529,10 @@ impl PrefixExpr {
             _ => None,
         }
     }
+
+    pub fn op(&self) -> Option<&SyntaxNode> {
+        self.syntax().first_child()
+    }
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
@@ -598,46 +602,54 @@ pub enum BinOp {
 }
 
 impl BinExpr {
-    pub fn op(&self) -> Option<BinOp> {
+    fn op_details(&self) -> Option<(&SyntaxNode, BinOp)> {
         self.syntax()
             .children()
             .filter_map(|c| match c.kind() {
-                PIPEPIPE => Some(BinOp::BooleanOr),
-                AMPAMP => Some(BinOp::BooleanAnd),
-                EQEQ => Some(BinOp::EqualityTest),
-                NEQ => Some(BinOp::NegatedEqualityTest),
-                LTEQ => Some(BinOp::LesserEqualTest),
-                GTEQ => Some(BinOp::GreaterEqualTest),
-                L_ANGLE => Some(BinOp::LesserTest),
-                R_ANGLE => Some(BinOp::GreaterTest),
-                PLUS => Some(BinOp::Addition),
-                STAR => Some(BinOp::Multiplication),
-                MINUS => Some(BinOp::Subtraction),
-                SLASH => Some(BinOp::Division),
-                PERCENT => Some(BinOp::Remainder),
-                SHL => Some(BinOp::LeftShift),
-                SHR => Some(BinOp::RightShift),
-                CARET => Some(BinOp::BitwiseXor),
-                PIPE => Some(BinOp::BitwiseOr),
-                AMP => Some(BinOp::BitwiseAnd),
-                DOTDOT => Some(BinOp::RangeRightOpen),
-                DOTDOTEQ => Some(BinOp::RangeRightClosed),
-                EQ => Some(BinOp::Assignment),
-                PLUSEQ => Some(BinOp::AddAssign),
-                SLASHEQ => Some(BinOp::DivAssign),
-                STAREQ => Some(BinOp::MulAssign),
-                PERCENTEQ => Some(BinOp::RemAssign),
-                SHREQ => Some(BinOp::ShrAssign),
-                SHLEQ => Some(BinOp::ShlAssign),
-                MINUSEQ => Some(BinOp::SubAssign),
-                PIPEEQ => Some(BinOp::BitOrAssign),
-                AMPEQ => Some(BinOp::BitAndAssign),
-                CARETEQ => Some(BinOp::BitXorAssign),
+                PIPEPIPE => Some((c, BinOp::BooleanOr)),
+                AMPAMP => Some((c, BinOp::BooleanAnd)),
+                EQEQ => Some((c, BinOp::EqualityTest)),
+                NEQ => Some((c, BinOp::NegatedEqualityTest)),
+                LTEQ => Some((c, BinOp::LesserEqualTest)),
+                GTEQ => Some((c, BinOp::GreaterEqualTest)),
+                L_ANGLE => Some((c, BinOp::LesserTest)),
+                R_ANGLE => Some((c, BinOp::GreaterTest)),
+                PLUS => Some((c, BinOp::Addition)),
+                STAR => Some((c, BinOp::Multiplication)),
+                MINUS => Some((c, BinOp::Subtraction)),
+                SLASH => Some((c, BinOp::Division)),
+                PERCENT => Some((c, BinOp::Remainder)),
+                SHL => Some((c, BinOp::LeftShift)),
+                SHR => Some((c, BinOp::RightShift)),
+                CARET => Some((c, BinOp::BitwiseXor)),
+                PIPE => Some((c, BinOp::BitwiseOr)),
+                AMP => Some((c, BinOp::BitwiseAnd)),
+                DOTDOT => Some((c, BinOp::RangeRightOpen)),
+                DOTDOTEQ => Some((c, BinOp::RangeRightClosed)),
+                EQ => Some((c, BinOp::Assignment)),
+                PLUSEQ => Some((c, BinOp::AddAssign)),
+                SLASHEQ => Some((c, BinOp::DivAssign)),
+                STAREQ => Some((c, BinOp::MulAssign)),
+                PERCENTEQ => Some((c, BinOp::RemAssign)),
+                SHREQ => Some((c, BinOp::ShrAssign)),
+                SHLEQ => Some((c, BinOp::ShlAssign)),
+                MINUSEQ => Some((c, BinOp::SubAssign)),
+                PIPEEQ => Some((c, BinOp::BitOrAssign)),
+                AMPEQ => Some((c, BinOp::BitAndAssign)),
+                CARETEQ => Some((c, BinOp::BitXorAssign)),
                 _ => None,
             })
             .next()
     }
 
+    pub fn op_kind(&self) -> Option<BinOp> {
+        self.op_details().map(|t| t.1)
+    }
+
+    pub fn op(&self) -> Option<&SyntaxNode> {
+        self.op_details().map(|t| t.0)
+    }
+
     pub fn lhs(&self) -> Option<&Expr> {
         children(self).nth(0)
     }