[clangd] Disable extract variable for RHS of assignments

[adamcz]

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

400

nit: you can fold the two if-s together.

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

if (llvm::isa<DeclRefExpr>(E) || llvm::isa<MemberExpr>(E))
     return false;
 
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   // FIXME: we could still hoist the assignment, and leave the variable there?
   ParsedBinaryOperator BinOp;
   if (BinOp.parse(*N) && BinaryOperator::isAssignmentOp(BinOp.Kind))
     return false;
 
+  const SelectionTree::Node &OuterImplicit = N->outerImplicit();
+  const auto *Parent = OuterImplicit.Parent;
+  if (!Parent)
+    return false;
   // We don't want to extract expressions used as statements, that would leave
   // a `dummy;` around that has no effect.
   // Unfortunately because the AST doesn't have ExprStmt, we have to check in
   // this roundabout way.
-  const SelectionTree::Node &OuterImplicit = N->outerImplicit();
-  if (!OuterImplicit.Parent ||
-      childExprIsStmt(OuterImplicit.Parent->ASTNode.get<Stmt>(),
+  if (childExprIsStmt(Parent->ASTNode.get<Stmt>(),
                       OuterImplicit.ASTNode.get<Expr>()))
     return false;
 
-  // FIXME: ban extracting the RHS of an assignment: `a = [[foo()]]`
+  // Disable extraction of full RHS on assignment operations, e.g:
+  // auto x = [[RHS_EXPR]];
+  // This would just result in duplicating the code.
+  if (const auto *BO = Parent->ASTNode.get<BinaryOperator>()) {
+    if (BO->isAssignmentOp() &&
+        BO->getRHS() == OuterImplicit.ASTNode.get<Expr>())
+      return false;
+  }
+
   return true;
 }
 
 // Find the Expr node that we're going to extract.
 // We don't want to trigger for assignment expressions and variable/field
 // DeclRefs. For function/member function, we want to extract the entire
 // function call.
 const SelectionTree::Node *computeExtractedExpr(const SelectionTree::Node *N) {

clang-tools-extra/clangd/unittests/TweakTests.cpp

const char *AvailableCases = R"cpp(
     }
     void f() {
       int a = [[5 +]] [[4 * [[[[xyz]]()]]]];
       // multivariable initialization
       if(1)
         int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
       // if without else
       if([[1]])
-        a = [[1]];
+        a = [[1]] + 1;
       // if with else
       if(a < [[3]])
         if(a == [[4]])
-          a = [[5]];
+          a = [[5]] + 1;
         else
-          a = [[5]];
+          a = [[5]] + 1;
       else if (a < [[4]])
-        a = [[4]];
+        a = [[4]] + 1;
       else
-        a = [[5]];
+        a = [[5]] + 1;
       // for loop
-      for(a = [[1]]; a > [[[[3]] + [[4]]]]; a++)
-        a = [[2]];
+      for(a = [[1]] + 1; a > [[[[3]] + [[4]]]]; a++)
+        a = [[2]] + 1;
       // while
       while(a < [[1]])
-        a = [[1]];
+        a = [[1]] + 1;
       // do while
       do
-        a = [[1]];
+        a = [[1]] + 1;
       while(a < [[3]]);
     }
   )cpp";
   EXPECT_AVAILABLE(AvailableCases);
 
   ExtraArgs = {"-xc"};
   const char *AvailableButC = R"cpp(
     void foo() {
void f(int b = [[1]]) {
         a = [[a + 1]];
       // lambda
       auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;};
       // assignment
       xyz([[a = 5]]);
       xyz([[a *= 5]]);
       // Variable DeclRefExpr
       a = [[b]];
+      a = [[xyz()]];
       // statement expression
       [[xyz()]];
       while (a)
         [[++a]];
       // label statement
       goto label;
       label:
         a = [[1]];
const std::vector<std::pair<std::string, std::string>>
                  })cpp",
            R"cpp(#define LOOP(x) do {x;} while(1);
                  void f(int a) {
                    auto dummy = 3; if(1)
                     LOOP(5 + dummy)
                  })cpp"},
           // attribute testing
           {R"cpp(void f(int a) {
-                    [ [gsl::suppress("type")] ] for (;;) a = [[1]];
+                    [ [gsl::suppress("type")] ] for (;;) a = [[1]] + 1;
                  })cpp",
            R"cpp(void f(int a) {
-                    auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = dummy;
+                    auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = dummy + 1;
                  })cpp"},
           // MemberExpr
           {R"cpp(class T {
                    T f() {
                      return [[T().f()]].f();
                    }
                  };)cpp",
            R"cpp(class T {