diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -15,6 +15,7 @@ DumpAST.cpp RawStringLiteral.cpp SwapIfBranches.cpp + ExtractVariable.cpp LINK_LIBS clangAST diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -0,0 +1,250 @@ +//===--- ExtractVariable.cpp ------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +#include "ClangdUnit.h" +#include "Logger.h" +#include "Protocol.h" +#include "Selection.h" +#include "SourceCode.h" +#include "refactor/Tweak.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "clang/AST/OperationKinds.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Stmt.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/None.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" + +namespace clang { +namespace clangd { +namespace { +// information regarding the Expr that is being extracted +class ExtractionContext { +public: + ExtractionContext(const SelectionTree::Node *Node, const SourceManager &SM, + const ASTContext &Ctx); + // return the expr + inline const clang::Expr *getExpr() const; + // return the SelectionTree node for the Expr + inline const SelectionTree::Node *getExprNode() const; + // Generate Replacement for replacing selected expression with given VarName + tooling::Replacement replaceWithVar(std::string VarName) const; + // Generate Replacement for declaring the selected Expr as a new variable + tooling::Replacement insertDeclaration(std::string VarName) const; + // returns true if the Expr can be extracted + inline bool isExtractable() const; + +private: + bool Extractable = false; + const clang::Expr *Expr; + const SelectionTree::Node *ExprNode; + // Stmt before which we will extract + const clang::Stmt *InsertionPoint = nullptr; + + const SourceManager &SM; + + const ASTContext &Ctx; + // vector of all Decls referenced in the Expr + std::vector ReferencedDecls; + // returns true if the Expr doesn't reference any variable declared in scope + bool canInsertBefore(const clang::Stmt *Scope) const; + // computes the Stmt before which we will extract out Expr + const clang::Stmt *computeInsertionPoint() const; +}; +/// Extracts an expression to the variable dummy +/// Before: +/// int x = 5 + 4 * 3; +/// ^^^^^ +/// After: +/// auto dummy = 5 + 4; +/// int x = dummy * 3; +class ExtractVariable : public Tweak { +public: + const char *id() const override final; + bool prepare(const Selection &Inputs) override; + Expected apply(const Selection &Inputs) override; + std::string title() const override; + Intent intent() const override { return Refactor; } + +private: + // the expression to extract + std::unique_ptr Target; +}; +REGISTER_TWEAK(ExtractVariable) + +// returns true if the Expr is a reference to a function +// this is to avoid the function name being extracted in a function call. +// e.g. int a = [[f]](); +bool isAFunctionRef(const clang::Expr *Expr) { + const clang::DeclRefExpr *DeclRef = dyn_cast_or_null(Expr); + if (DeclRef && isa(DeclRef->getDecl())) + return true; + return false; +} + +// Returns all the Decls referenced inside the given Expr +static std::vector +computeReferencedDecls(const clang::Expr *Expr) { + // RAV subclass to find all DeclRefs in a given Stmt + class FindDeclRefsVisitor + : public clang::RecursiveASTVisitor { + public: + std::vector ReferencedDecls; + bool VisitDeclRefExpr(DeclRefExpr *DeclRef) { // NOLINT + ReferencedDecls.push_back(DeclRef->getDecl()); + return true; + } + }; + FindDeclRefsVisitor Visitor; + Visitor.TraverseStmt(const_cast(dyn_cast(Expr))); + return Visitor.ReferencedDecls; +} + +inline bool ExtractionContext::isExtractable() const { return Extractable; } +ExtractionContext::ExtractionContext(const SelectionTree::Node *Node, + const SourceManager &SM, + const ASTContext &Ctx) + : ExprNode(Node), SM(SM), Ctx(Ctx) { + if ((Expr = Node->ASTNode.get())) { + Expr->dump(); + ReferencedDecls = computeReferencedDecls(Expr); + Extractable = + !isAFunctionRef(Expr) && (InsertionPoint = computeInsertionPoint()); + } +} + +inline const clang::Expr *ExtractionContext::getExpr() const { return Expr; } + +inline const SelectionTree::Node *ExtractionContext::getExprNode() const { + return ExprNode; +} + +// checks whether extracting before InsertionPoint will take a +// variable out of scope +bool ExtractionContext::canInsertBefore(const clang::Stmt *Scope) const { + if (!Scope) + return true; + SourceLocation ScopeBegin = Scope->getBeginLoc(); + SourceLocation ScopeEnd = Scope->getEndLoc(); + for (const Decl *ReferencedDecl : ReferencedDecls) { + if (SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) && + SM.isPointWithin(ReferencedDecl->getEndLoc(), ScopeBegin, ScopeEnd)) + return false; + } + return true; +} + +// Return the Stmt before which we need to insert the extraction. +// To find the Stmt, we go up the AST Tree and if the Parent of the current Stmt +// is a CompoundStmt, we can extract inside this CompoundStmt just before the +// current Stmt. We ALWAYS insert before a Stmt whose parent is a CompoundStmt +// +// When extracting from a Case/Default statement, we turn on the bool +// WaitForSwitch and turn it off once the CurInsertionPoint is a switch +// statement. As long as WaitForSwitch is on, we keep going up the AST +// +const clang::Stmt *ExtractionContext::computeInsertionPoint() const { + bool WaitForSwitch = false; + for (const SelectionTree::Node *CurNode = getExprNode(); CurNode->Parent; + CurNode = CurNode->Parent) { + const clang::Stmt *CurInsertionPoint = CurNode->ASTNode.get(); + // give up if extraction will take a variable out of scope + if (!canInsertBefore(CurInsertionPoint)) + break; + if (const clang::Stmt *Ancestor = CurNode->Parent->ASTNode.get()) { + if (WaitForSwitch) { + if (isa(CurInsertionPoint)) + WaitForSwitch = false; + else + continue; + } + if (isa(Ancestor)) { + // Return if the CurInsertionPoint is not inside a macro. Else we + // continue traversing up. + if (Ancestor->getBeginLoc().isMacroID()) + continue; + return CurInsertionPoint; + } else if (isa(Ancestor)) + WaitForSwitch = true; + // don't extract from lambda captures/default arguments + else if (isa(Ancestor)) + break; + } + // don't extract from function/method default arguments. + // stop extraction that creates a class field. + else if (CurNode->Parent->ASTNode.get() || + CurNode->Parent->ASTNode.get()) + break; + } + return nullptr; +} +// returns the replacement for substituting the extraction with VarName +tooling::Replacement +ExtractionContext::replaceWithVar(std::string VarName) const { + const llvm::Optional ExtractionRng = + toHalfOpenFileRange(SM, Ctx.getLangOpts(), getExpr()->getSourceRange()); + unsigned ExtractionLength = SM.getFileOffset(ExtractionRng->getEnd()) - + SM.getFileOffset(ExtractionRng->getBegin()); + return tooling::Replacement(SM, ExtractionRng->getBegin(), ExtractionLength, + VarName); +} +// returns the Replacement for declaring a new variable storing the extraction +tooling::Replacement +ExtractionContext::insertDeclaration(std::string VarName) const { + const llvm::Optional ExtractionRng = + toHalfOpenFileRange(SM, Ctx.getLangOpts(), getExpr()->getSourceRange()); + assert(ExractionRng && "ExtractionRng should not be null"); + llvm::StringRef ExtractionCode = toSourceCode(SM, *ExtractionRng); + const SourceLocation InsertionLoc = + toHalfOpenFileRange(SM, Ctx.getLangOpts(), + InsertionPoint->getSourceRange()) + ->getBegin(); + // FIXME: Replace auto with explicit type and add &/&& as necessary + std::string ExtractedVarDecl = + std::string("auto ") + VarName + " = " + ExtractionCode.str() + "; "; + return tooling::Replacement(SM, InsertionLoc, 0, ExtractedVarDecl); +} + +// FIXME: Ignore assignment (a = 1) Expr since it is extracted as dummy = a = 1 +// FIXME: Expressions involving macro expansions +bool ExtractVariable::prepare(const Selection &Inputs) { + const ASTContext &Ctx = Inputs.AST.getASTContext(); + const SourceManager &SM = Inputs.AST.getSourceManager(); + const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); + if (!N) + return false; + Target = llvm::make_unique(N, SM, Ctx); + return Target->isExtractable(); +} + +Expected ExtractVariable::apply(const Selection &Inputs) { + tooling::Replacements Result; + // FIXME: get variable name from user or suggest based on type + std::string VarName = "dummy"; + // insert new variable declaration + if (auto Err = Result.add(Target->insertDeclaration(VarName))) + return std::move(Err); + // replace expression with variable name + if (auto Err = Result.add(Target->replaceWithVar(VarName))) + return std::move(Err); + return Effect::applyEdit(Result); +} + +std::string ExtractVariable::title() const { + return "Extract subexpression to variable"; +} + +} // namespace +} // namespace clangd +} // namespace clang 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 @@ -239,13 +239,13 @@ checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }"); const char *Input = "int x = 2 ^+ 2;"; - auto result = getMessage(ID, Input); - EXPECT_THAT(result, ::testing::HasSubstr("BinaryOperator")); - EXPECT_THAT(result, ::testing::HasSubstr("'+'")); - EXPECT_THAT(result, ::testing::HasSubstr("|-IntegerLiteral")); - EXPECT_THAT(result, + auto Result = getMessage(ID, Input); + EXPECT_THAT(Result, ::testing::HasSubstr("BinaryOperator")); + EXPECT_THAT(Result, ::testing::HasSubstr("'+'")); + EXPECT_THAT(Result, ::testing::HasSubstr("|-IntegerLiteral")); + EXPECT_THAT(Result, ::testing::HasSubstr(" 'int' 2\n`-IntegerLiteral")); - EXPECT_THAT(result, ::testing::HasSubstr(" 'int' 2")); + EXPECT_THAT(Result, ::testing::HasSubstr(" 'int' 2")); } TEST(TweakTest, ShowSelectionTree) { @@ -277,6 +277,128 @@ const char *Input = "struct ^X { int x; int y; }"; EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 | int x")); } +TEST(TweakTest, ExtractVariable) { + llvm::StringLiteral ID = "ExtractVariable"; + checkAvailable(ID, R"cpp( + int xyz() { + return ^1; + } + void f() { + int a = 5 + [[4 * xyz^()]]; + if(1) + int x = ^1, y = ^a + 1, a = ^1, z = a + 1; + switch(a) + case 1: + a = ^1; + break; + default: + a = ^3; + // if without else + if(^1) {} + // if with else + if(a < ^3) + if(a == ^4) + a = ^5; + else + a = ^6; + else if (a < ^4) + a = ^4; + else + a = ^5; + // for loop testing + for(a = ^1; a > ^3^+^4; a++) + a = ^2; + // while testing + while(a < ^1) + ^a++; + // do while testing + do + a = ^1; + while(a < ^3); + } + )cpp"); + checkNotAvailable(ID, R"cpp( + int xyz(int a = ^1) { + return 1; + class T { + T(int a = ^1) {}; + int xyz = ^1; + }; + } + void f(int b = ^1) { + // we don't want to extract just the function + int a = 5 + 4 * [[xyz]](); + // if + if(1) + int x = 1, y = a + 1, a = 1, z = ^a + 1; + if(int a = 1) + if(^a == 4) + a = ^a ^+ 1; + // for loop testing + for(int a = 1, b = 2, c = 3; ^a > ^b ^+ ^c; ^a++) + a = ^a ^+ 1; + // lambda testing + auto lamb = [&^a, &^b](int r = ^1) {return 1;} + } + )cpp"); + // vector of pairs of input and output strings + const std::vector> + InputOutputs = { + // extraction from variable declaration/assignment + {R"cpp(void varDecl() { + int a = 5 * (4 + (3 [[- 1)]]); + })cpp", + R"cpp(void varDecl() { + auto dummy = (3 - 1); int a = 5 * (4 + dummy); + })cpp"}, + // extraction from case inside nested structure + {R"cpp(void f(int a) { + if(1) + while(a < 1) + switch (1) { + case 1: + a = [[1 + 2]]; + break; + default: + break; + } + })cpp", + R"cpp(void f(int a) { + auto dummy = 1 + 2; if(1) + while(a < 1) + switch (1) { + case 1: + a = dummy; + break; + default: + break; + } + })cpp"}, + // ensure InsertionPoint isn't inside a macro + {R"cpp(#define LOOP(x) {int a = x + 1;} + void f(int a) { + if(1) + LOOP(5 + ^3) + })cpp", + R"cpp(#define LOOP(x) {int a = x + 1;} + void f(int a) { + auto dummy = 3; if(1) + LOOP(5 + dummy) + })cpp"}, + // FIXME: Doesn't work because bug in selection tree + /*{R"cpp(#define PLUS(x) x++ + void f(int a) { + PLUS(^a); + })cpp", + R"cpp(#define PLUS(x) x++ + void f(int a) { + auto dummy = a; PLUS(dummy); + })cpp"},*/ + }; + for (const auto &IO : InputOutputs) { + checkTransform(ID, IO.first, IO.second); + } +} } // namespace } // namespace clangd