diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -382,17 +382,27 @@ 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(), + if (childExprIsStmt(Parent->ASTNode.get(), OuterImplicit.ASTNode.get())) 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()) { + if (BO->isAssignmentOp() && + BO->getRHS() == OuterImplicit.ASTNode.get()) + return false; + } + return true; } diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -215,26 +215,26 @@ 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"; @@ -291,6 +291,7 @@ xyz([[a *= 5]]); // Variable DeclRefExpr a = [[b]]; + a = [[xyz()]]; // statement expression [[xyz()]]; while (a) @@ -373,10 +374,10 @@ })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 {