Author: Kadir Cetinkaya Date: 2019-10-25T18:40:01+02:00 New Revision: 8e567b0730fa55d15e6c0ab20b0352d85e96b7bb
URL: https://github.com/llvm/llvm-project/commit/8e567b0730fa55d15e6c0ab20b0352d85e96b7bb DIFF: https://github.com/llvm/llvm-project/commit/8e567b0730fa55d15e6c0ab20b0352d85e96b7bb.diff LOG: [clangd] Revert define-inline action changes to un-break windows build-bots Added: Modified: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/refactor/Tweak.cpp clang-tools-extra/clangd/refactor/Tweak.h clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt clang-tools-extra/clangd/unittests/TweakTesting.cpp clang-tools-extra/clangd/unittests/TweakTesting.h clang-tools-extra/clangd/unittests/TweakTests.cpp Removed: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 64ccbe417c24..4f1fe8f5b08b 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -367,7 +367,7 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) { auto End = positionToOffset(AST.Inputs.Contents, Sel.end); if (!End) return End.takeError(); - return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End); + return Tweak::Selection(AST.AST, *Begin, *End); } void ClangdServer::enumerateTweaks(PathRef File, Range Sel, diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp index 2dc091ed762a..4f3c40d1eb13 100644 --- a/clang-tools-extra/clangd/refactor/Tweak.cpp +++ b/clang-tools-extra/clangd/refactor/Tweak.cpp @@ -9,7 +9,6 @@ #include "Logger.h" #include "Path.h" #include "SourceCode.h" -#include "index/Index.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" @@ -45,10 +44,9 @@ void validateRegistry() { } } // namespace -Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST, - unsigned RangeBegin, unsigned RangeEnd) - : Index(Index), AST(AST), SelectionBegin(RangeBegin), - SelectionEnd(RangeEnd), +Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin, + unsigned RangeEnd) + : AST(AST), SelectionBegin(RangeBegin), SelectionEnd(RangeEnd), ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) { auto &SM = AST.getSourceManager(); Code = SM.getBufferData(SM.getMainFileID()); diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h index de655abd98c7..42b7fb0684e8 100644 --- a/clang-tools-extra/clangd/refactor/Tweak.h +++ b/clang-tools-extra/clangd/refactor/Tweak.h @@ -24,7 +24,6 @@ #include "Protocol.h" #include "Selection.h" #include "SourceCode.h" -#include "index/Index.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringMap.h" @@ -47,12 +46,9 @@ class Tweak { public: /// Input to prepare and apply tweaks. struct Selection { - Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin, - unsigned RangeEnd); + Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd); /// The text of the active document. llvm::StringRef Code; - /// The Index for handling codebase related queries. - const SymbolIndex *Index = nullptr; /// Parsed AST of the active file. ParsedAST &AST; /// A location of the cursor in the editor. diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt index ddf10a2ca2ba..f43f132304c9 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -14,7 +14,6 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangDaemonTweaks OBJECT AnnotateHighlightings.cpp DumpAST.cpp - DefineInline.cpp ExpandAutoType.cpp ExpandMacro.cpp ExtractFunction.cpp diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp deleted file mode 100644 index 8785db1a292b..000000000000 --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ /dev/null @@ -1,393 +0,0 @@ -//===--- DefineInline.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 "AST.h" -#include "FindTarget.h" -#include "Logger.h" -#include "Selection.h" -#include "SourceCode.h" -#include "XRefs.h" -#include "refactor/Tweak.h" -#include "clang/AST/ASTContext.h" -#include "clang/AST/ASTTypeTraits.h" -#include "clang/AST/Decl.h" -#include "clang/AST/DeclBase.h" -#include "clang/AST/DeclCXX.h" -#include "clang/AST/DeclTemplate.h" -#include "clang/AST/Expr.h" -#include "clang/AST/ExprCXX.h" -#include "clang/AST/NestedNameSpecifier.h" -#include "clang/AST/PrettyPrinter.h" -#include "clang/AST/RecursiveASTVisitor.h" -#include "clang/AST/Stmt.h" -#include "clang/AST/TemplateBase.h" -#include "clang/AST/Type.h" -#include "clang/AST/TypeLoc.h" -#include "clang/Basic/LangOptions.h" -#include "clang/Basic/SourceLocation.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Basic/TokenKinds.h" -#include "clang/Index/IndexDataConsumer.h" -#include "clang/Index/IndexSymbol.h" -#include "clang/Index/IndexingAction.h" -#include "clang/Lex/Lexer.h" -#include "clang/Lex/Preprocessor.h" -#include "clang/Lex/Token.h" -#include "clang/Sema/Lookup.h" -#include "clang/Sema/Sema.h" -#include "clang/Tooling/Core/Replacement.h" -#include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DenseSet.h" -#include "llvm/ADT/None.h" -#include "llvm/ADT/Optional.h" -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringRef.h" -#include "llvm/Support/Casting.h" -#include "llvm/Support/Error.h" -#include "llvm/Support/FormatAdapters.h" -#include "llvm/Support/FormatVariadic.h" -#include "llvm/Support/Signals.h" -#include "llvm/Support/raw_ostream.h" -#include <cstddef> -#include <set> -#include <string> -#include <unordered_map> -#include <utility> -#include <vector> - -namespace clang { -namespace clangd { -namespace { - -// Returns semicolon location for the given FD. Since AST doesn't contain that -// information, searches for a semicolon by lexing from end of function decl -// while skipping comments. -llvm::Optional<SourceLocation> getSemicolonForDecl(const FunctionDecl *FD) { - const SourceManager &SM = FD->getASTContext().getSourceManager(); - const LangOptions &LangOpts = FD->getASTContext().getLangOpts(); - - SourceLocation CurLoc = FD->getEndLoc(); - auto NextTok = Lexer::findNextToken(CurLoc, SM, LangOpts); - if (!NextTok || !NextTok->is(tok::semi)) - return llvm::None; - return NextTok->getLocation(); -} - -// Deduces the FunctionDecl from a selection. Requires either the function body -// or the function decl to be selected. Returns null if none of the above -// criteria is met. -const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) { - const ast_type_traits::DynTypedNode &AstNode = SelNode->ASTNode; - if (const FunctionDecl *FD = AstNode.get<FunctionDecl>()) - return FD; - if (AstNode.get<CompoundStmt>() && - SelNode->Selected == SelectionTree::Complete) { - if (const SelectionTree::Node *P = SelNode->Parent) - return P->ASTNode.get<FunctionDecl>(); - } - return nullptr; -} - -// Checks the decls mentioned in Source are visible in the context of Target. -// Achives that by checking declaraions occur before target location in -// translation unit or declared in the same class. -bool checkDeclsAreVisible(const llvm::DenseSet<const Decl *> &DeclRefs, - const FunctionDecl *Target, const SourceManager &SM) { - SourceLocation TargetLoc = Target->getLocation(); - // To be used in visibility check below, decls in a class are visible - // independent of order. - const RecordDecl *Class = nullptr; - if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(Target)) - Class = MD->getParent(); - - for (const auto *DR : DeclRefs) { - // Use canonical decl, since having one decl before target is enough. - const Decl *D = DR->getCanonicalDecl(); - if (D == Target) - continue; - SourceLocation DeclLoc = D->getLocation(); - - // FIXME: Allow declarations from diff erent files with include insertion. - if (!SM.isWrittenInSameFile(DeclLoc, TargetLoc)) - return false; - - // If declaration is before target, then it is visible. - if (SM.isBeforeInTranslationUnit(DeclLoc, TargetLoc)) - continue; - - // Otherwise they need to be in same class - if (!Class) - return false; - const RecordDecl *Parent = nullptr; - if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(D)) - Parent = MD->getParent(); - else if (const auto *FD = llvm::dyn_cast<FieldDecl>(D)) - Parent = FD->getParent(); - if (Parent != Class) - return false; - } - return true; -} - -// Rewrites body of FD to fully-qualify all of the decls inside. -llvm::Expected<std::string> qualifyAllDecls(const FunctionDecl *FD) { - // There are three types of spellings that needs to be qualified in a function - // body: - // - Types: Foo -> ns::Foo - // - DeclRefExpr: ns2::foo() -> ns1::ns2::foo(); - // - UsingDecls: - // using ns2::foo -> using ns1::ns2::foo - // using namespace ns2 -> using namespace ns1::ns2 - // using ns3 = ns2 -> using ns3 = ns1::ns2 - // - // Go over all references inside a function body to generate replacements that - // will fully qualify those. So that body can be moved into an arbitrary file. - // We perform the qualification by qualyfying the first type/decl in a - // (un)qualified name. e.g: - // namespace a { namespace b { class Bar{}; void foo(); } } - // b::Bar x; -> a::b::Bar x; - // foo(); -> a::b::foo(); - // FIXME: Instead of fully qualyfying we should try deducing visible scopes at - // target location and generate minimal edits. - - const SourceManager &SM = FD->getASTContext().getSourceManager(); - tooling::Replacements Replacements; - bool HadErrors = false; - findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) { - // Since we want to qualify only the first qualifier, skip names with a - // qualifier. - if (Ref.Qualifier) - return; - // There might be no decl in dependent contexts, there's nothing much we can - // do in such cases. - if (Ref.Targets.empty()) - return; - // Do not qualify names introduced by macro expansions. - if (Ref.NameLoc.isMacroID()) - return; - - for (const NamedDecl *ND : Ref.Targets) { - if (ND->getDeclContext() != Ref.Targets.front()->getDeclContext()) { - elog("define inline: Targets from multiple contexts: {0}, {1}", - printQualifiedName(*Ref.Targets.front()), printQualifiedName(*ND)); - HadErrors = true; - return; - } - } - // All Targets are in the same scope, so we can safely chose first one. - const NamedDecl *ND = Ref.Targets.front(); - // Skip anything from a non-namespace scope, these can be: - // - Function or Method scopes, which means decl is local and doesn't need - // qualification. - // - From Class/Struct/Union scope, which again doesn't need any qualifiers, - // rather the left side of it requires qualification, like: - // namespace a { class Bar { public: static int x; } } - // void foo() { Bar::x; } - // ~~~~~ -> we need to qualify Bar not x. - if (!ND->getDeclContext()->isNamespace()) - return; - - std::string Qualifier = printNamespaceScope(*ND->getDeclContext()); - if (auto Err = Replacements.add( - tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) { - HadErrors = true; - elog("define inline: Failed to add quals: {0}", std::move(Err)); - } - }); - - if (HadErrors) { - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "define inline: Failed to compute qualifiers see logs for details."); - } - - // Get new begin and end positions for the qualified body. - auto OrigBodyRange = toHalfOpenFileRange( - SM, FD->getASTContext().getLangOpts(), FD->getBody()->getSourceRange()); - if (!OrigBodyRange) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Couldn't get range func body."); - - unsigned BodyBegin = SM.getFileOffset(OrigBodyRange->getBegin()); - unsigned BodyEnd = Replacements.getShiftedCodePosition( - SM.getFileOffset(OrigBodyRange->getEnd())); - - // Trim the result to function body. - auto QualifiedFunc = tooling::applyAllReplacements( - SM.getBufferData(SM.getFileID(OrigBodyRange->getBegin())), Replacements); - if (!QualifiedFunc) - return QualifiedFunc.takeError(); - return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1); -} - -// Returns the canonical declaration for the given FunctionDecl. This will -// usually be the first declaration in current translation unit with the -// exception of template specialization. -// For those we return first declaration diff erent than the canonical one. -// Because canonical declaration points to template decl instead of -// specialization. -const FunctionDecl *findTarget(const FunctionDecl *FD) { - auto CanonDecl = FD->getCanonicalDecl(); - if (!FD->isFunctionTemplateSpecialization()) - return CanonDecl; - // For specializations CanonicalDecl is the TemplatedDecl, which is not the - // target we want to inline into. Instead we traverse previous decls to find - // the first forward decl for this specialization. - auto PrevDecl = FD; - while (PrevDecl->getPreviousDecl() != CanonDecl) { - PrevDecl = PrevDecl->getPreviousDecl(); - assert(PrevDecl && "Found specialization without template decl"); - } - return PrevDecl; -} - -// Returns the begining location for a FunctionDecl. Returns location of -// template keyword for templated functions. -const SourceLocation getBeginLoc(const FunctionDecl *FD) { - // Include template parameter list. - if (auto *FTD = FD->getDescribedFunctionTemplate()) - return FTD->getBeginLoc(); - return FD->getBeginLoc(); -} - -/// Moves definition of a function/method to its declaration location. -/// Before: -/// a.h: -/// void foo(); -/// -/// a.cc: -/// void foo() { return; } -/// -/// ------------------------ -/// After: -/// a.h: -/// void foo() { return; } -/// -/// a.cc: -/// -class DefineInline : public Tweak { -public: - const char *id() const override final; - - Intent intent() const override { return Intent::Refactor; } - std::string title() const override { - return "Move function body to declaration"; - } - - // Returns true when selection is on a function definition that does not - // make use of any internal symbols. - bool prepare(const Selection &Sel) override { - const SelectionTree::Node *SelNode = Sel.ASTSelection.commonAncestor(); - if (!SelNode) - return false; - Source = getSelectedFunction(SelNode); - if (!Source || !Source->isThisDeclarationADefinition()) - return false; - // Only the last level of template parameter locations are not kept in AST, - // so if we are inlining a method that is in a templated class, there is no - // way to verify template parameter names. Therefore we bail out. - if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) { - if (MD->getParent()->isTemplated()) - return false; - } - // If function body starts or ends inside a macro, we refuse to move it into - // declaration location. - if (Source->getBody()->getBeginLoc().isMacroID() || - Source->getBody()->getEndLoc().isMacroID()) - return false; - - Target = findTarget(Source); - if (Target == Source) { - // The only declaration is Source. No other declaration to move function - // body. - // FIXME: If we are in an implementation file, figure out a suitable - // location to put declaration. Possibly using other declarations in the - // AST. - return false; - } - - // Check if the decls referenced in function body are visible in the - // declaration location. - if (!checkDeclsAreVisible(getNonLocalDeclRefs(Sel.AST, Source), Target, - Sel.AST.getSourceManager())) - return false; - - return true; - } - - Expected<Effect> apply(const Selection &Sel) override { - const auto &AST = Sel.AST.getASTContext(); - const auto &SM = AST.getSourceManager(); - - auto Semicolon = getSemicolonForDecl(Target); - if (!Semicolon) { - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "Couldn't find semicolon for target declaration."); - } - - auto QualifiedBody = qualifyAllDecls(Source); - if (!QualifiedBody) - return QualifiedBody.takeError(); - - const tooling::Replacement SemicolonToFuncBody(SM, *Semicolon, 1, - *QualifiedBody); - auto DefRange = toHalfOpenFileRange( - SM, AST.getLangOpts(), - SM.getExpansionRange(CharSourceRange::getCharRange(getBeginLoc(Source), - Source->getEndLoc())) - .getAsRange()); - if (!DefRange) { - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Couldn't get range for the source."); - } - unsigned int SourceLen = SM.getFileOffset(DefRange->getEnd()) - - SM.getFileOffset(DefRange->getBegin()); - const tooling::Replacement DeleteFuncBody(SM, DefRange->getBegin(), - SourceLen, ""); - - llvm::SmallVector<std::pair<std::string, Edit>, 2> Edits; - // Edit for Target. - auto FE = Effect::fileEdit(SM, SM.getFileID(*Semicolon), - tooling::Replacements(SemicolonToFuncBody)); - if (!FE) - return FE.takeError(); - Edits.push_back(std::move(*FE)); - - // Edit for Source. - if (!SM.isWrittenInSameFile(DefRange->getBegin(), - SM.getExpansionLoc(Target->getBeginLoc()))) { - // Generate a new edit if the Source and Target are in diff erent files. - auto FE = Effect::fileEdit(SM, SM.getFileID(Sel.Cursor), - tooling::Replacements(DeleteFuncBody)); - if (!FE) - return FE.takeError(); - Edits.push_back(std::move(*FE)); - } else { - // Merge with previous edit if they are in the same file. - if (auto Err = Edits.front().second.Replacements.add(DeleteFuncBody)) - return std::move(Err); - } - - Effect E; - for (auto &Pair : Edits) - E.ApplyEdits.try_emplace(std::move(Pair.first), std::move(Pair.second)); - return E; - } - -private: - const FunctionDecl *Source = nullptr; - const FunctionDecl *Target = nullptr; -}; - -REGISTER_TWEAK(DefineInline); - -} // namespace -} // namespace clangd -} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp index e5b619639a5d..63ecd7b4d4d5 100644 --- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp @@ -10,13 +10,9 @@ #include "Annotations.h" #include "SourceCode.h" -#include "TestFS.h" #include "refactor/Tweak.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/Support/Error.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" -#include <string> namespace clang { namespace clangd { @@ -63,7 +59,7 @@ std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) { cantFail(positionToOffset(A.code(), SelectionRng.end))}; } -MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index, +MATCHER_P3(TweakIsAvailable, TweakID, Ctx, Header, (TweakID + (negation ? " is unavailable" : " is available")).str()) { std::string WrappedCode = wrap(Ctx, arg); Annotations Input(WrappedCode); @@ -71,9 +67,8 @@ MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index, TestTU TU; TU.HeaderCode = Header; TU.Code = Input.code(); - TU.AdditionalFiles = std::move(ExtraFiles); ParsedAST AST = TU.build(); - Tweak::Selection S(Index, AST, Selection.first, Selection.second); + Tweak::Selection S(AST, Selection.first, Selection.second); auto PrepareResult = prepareTweak(TweakID, S); if (PrepareResult) return true; @@ -83,19 +78,17 @@ MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index, } // namespace -std::string TweakTest::apply(llvm::StringRef MarkedCode, - llvm::StringMap<std::string> *EditedFiles) const { +std::string TweakTest::apply(llvm::StringRef MarkedCode) const { std::string WrappedCode = wrap(Context, MarkedCode); Annotations Input(WrappedCode); auto Selection = rangeOrPoint(Input); TestTU TU; TU.HeaderCode = Header; - TU.AdditionalFiles = std::move(ExtraFiles); TU.Code = Input.code(); TU.ExtraArgs = ExtraArgs; ParsedAST AST = TU.build(); - Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second); + Tweak::Selection S(AST, Selection.first, Selection.second); auto T = prepareTweak(TweakID, S); if (!T) { @@ -109,29 +102,18 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode, return "message:\n" + *Result->ShowMessage; if (Result->ApplyEdits.empty()) return "no effect"; - - std::string EditedMainFile; - for (auto &It : Result->ApplyEdits) { - auto NewText = It.second.apply(); - if (!NewText) - return "bad edits: " + llvm::toString(NewText.takeError()); - llvm::StringRef Unwrapped = unwrap(Context, *NewText); - if (It.first() == testPath(TU.Filename)) - EditedMainFile = Unwrapped; - else { - if (!EditedFiles) - ADD_FAILURE() << "There were changes to additional files, but client " - "provided a nullptr for EditedFiles."; - else - EditedFiles->try_emplace(It.first(), Unwrapped.str()); - } - } - return EditedMainFile; + if (Result->ApplyEdits.size() > 1) + return "received multi-file edits"; + + auto ApplyEdit = Result->ApplyEdits.begin()->second; + if (auto NewText = ApplyEdit.apply()) + return unwrap(Context, *NewText); + else + return "bad edits: " + llvm::toString(NewText.takeError()); } ::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const { - return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraFiles, - Index.get()); + return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header); } std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) { diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.h b/clang-tools-extra/clangd/unittests/TweakTesting.h index ffcf5a0c7ea2..1e8cd58849bb 100644 --- a/clang-tools-extra/clangd/unittests/TweakTesting.h +++ b/clang-tools-extra/clangd/unittests/TweakTesting.h @@ -10,12 +10,8 @@ #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H #include "TestTU.h" -#include "index/Index.h" -#include "llvm/ADT/StringMap.h" -#include "gmock/gmock.h" #include "gtest/gtest.h" -#include <memory> -#include <string> +#include "gmock/gmock.h" namespace clang { namespace clangd { @@ -51,9 +47,6 @@ class TweakTest : public ::testing::Test { Expression, }; - // Mapping from file name to contents. - llvm::StringMap<std::string> ExtraFiles; - protected: TweakTest(const char *TweakID) : TweakID(TweakID) {} @@ -68,24 +61,15 @@ class TweakTest : public ::testing::Test { // Context in which snippets of code should be placed to run tweaks. CodeContext Context = File; - // Index to be passed into Tweak::Selection. - std::unique_ptr<const SymbolIndex> Index = nullptr; - // Apply the current tweak to the range (or point) in MarkedCode. // MarkedCode will be wrapped according to the Context. - // - if the tweak produces edits, returns the edited code (without markings) - // for the main file. - // Populates \p EditedFiles if there were changes to other files whenever - // it is non-null. It is a mapping from absolute path of the edited file to - // its new contents. Passing a nullptr to \p EditedFiles when there are - // changes, will result in a failure. + // - if the tweak produces edits, returns the edited code (without markings). // The context added to MarkedCode will be stripped away before returning, // unless the tweak edited it. // - if the tweak produces a message, returns "message:\n<message>" // - if prepare() returns false, returns "unavailable" // - if apply() returns an error, returns "fail: <message>" - std::string apply(llvm::StringRef MarkedCode, - llvm::StringMap<std::string> *EditedFiles = nullptr) const; + std::string apply(llvm::StringRef MarkedCode) const; // Accepts a code snippet with many ranges (or points) marked, and returns a // list of snippets with one range marked each. diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp index 2a6744b81d94..c5caa4b7dc5c 100644 --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -24,8 +24,6 @@ #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" -#include "llvm/ADT/StringExtras.h" -#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" @@ -35,23 +33,15 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include <cassert> -#include <string> -#include <utility> -#include <vector> using ::testing::AllOf; using ::testing::HasSubstr; using ::testing::StartsWith; -using ::testing::ElementsAre; namespace clang { namespace clangd { namespace { -MATCHER_P2(FileWithContents, FileName, Contents, "") { - return arg.first() == FileName && arg.second == Contents; -} - TEST(FileEdits, AbsolutePath) { auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"}; @@ -890,700 +880,6 @@ TEST_F(RemoveUsingNamespaceTest, All) { EXPECT_EQ(C.second, apply(C.first)) << C.first; } -TWEAK_TEST(DefineInline); -TEST_F(DefineInlineTest, TriggersOnFunctionDecl) { - // Basic check for function body and signature. - EXPECT_AVAILABLE(R"cpp( - class Bar { - void baz(); - }; - - [[void [[Bar::[[b^a^z]]]]() [[{ - return; - }]]]] - - void foo(); - [[void [[f^o^o]]() [[{ - return; - }]]]] - )cpp"); - - EXPECT_UNAVAILABLE(R"cpp( - // Not a definition - vo^i[[d^ ^f]]^oo(); - - [[vo^id ]]foo[[()]] {[[ - [[(void)(5+3); - return;]] - }]] - )cpp"); -} - -TEST_F(DefineInlineTest, NoForwardDecl) { - Header = "void bar();"; - EXPECT_UNAVAILABLE(R"cpp( - void bar() { - return; - } - // FIXME: Generate a decl in the header. - void fo^o() { - return; - })cpp"); -} - -TEST_F(DefineInlineTest, ReferencedDecls) { - EXPECT_AVAILABLE(R"cpp( - void bar(); - void foo(int test); - - void fo^o(int baz) { - int x = 10; - bar(); - })cpp"); - - // Internal symbol usage. - Header = "void foo(int test);"; - EXPECT_UNAVAILABLE(R"cpp( - void bar(); - void fo^o(int baz) { - int x = 10; - bar(); - })cpp"); - - // Becomes available after making symbol visible. - Header = "void bar();" + Header; - EXPECT_AVAILABLE(R"cpp( - void fo^o(int baz) { - int x = 10; - bar(); - })cpp"); - - // FIXME: Move declaration below bar to make it visible. - Header.clear(); - EXPECT_UNAVAILABLE(R"cpp( - void foo(); - void bar(); - - void fo^o() { - bar(); - })cpp"); - - // Order doesn't matter within a class. - EXPECT_AVAILABLE(R"cpp( - class Bar { - void foo(); - void bar(); - }; - - void Bar::fo^o() { - bar(); - })cpp"); - - // FIXME: Perform include insertion to make symbol visible. - ExtraFiles["a.h"] = "void bar();"; - Header = "void foo(int test);"; - EXPECT_UNAVAILABLE(R"cpp( - #include "a.h" - void fo^o(int baz) { - int x = 10; - bar(); - })cpp"); -} - -TEST_F(DefineInlineTest, TemplateSpec) { - EXPECT_UNAVAILABLE(R"cpp( - template <typename T> void foo(); - template<> void foo<char>(); - - template<> void f^oo<int>() { - })cpp"); - EXPECT_UNAVAILABLE(R"cpp( - template <typename T> void foo(); - - template<> void f^oo<int>() { - })cpp"); - EXPECT_UNAVAILABLE(R"cpp( - template <typename T> struct Foo { void foo(); }; - - template <typename T> void Foo<T>::f^oo() { - })cpp"); - EXPECT_AVAILABLE(R"cpp( - template <typename T> void foo(); - void bar(); - template <> void foo<int>(); - - template<> void f^oo<int>() { - bar(); - })cpp"); -} - -TEST_F(DefineInlineTest, CheckForCanonDecl) { - EXPECT_UNAVAILABLE(R"cpp( - void foo(); - - void bar() {} - void f^oo() { - // This bar normally refers to the definition just above, but it is not - // visible from the forward declaration of foo. - bar(); - })cpp"); - // Make it available with a forward decl. - EXPECT_AVAILABLE(R"cpp( - void bar(); - void foo(); - - void bar() {} - void f^oo() { - bar(); - })cpp"); -} - -TEST_F(DefineInlineTest, UsingShadowDecls) { - EXPECT_UNAVAILABLE(R"cpp( - namespace ns1 { void foo(int); } - namespace ns2 { void foo(int*); } - template <typename T> - void bar(); - - using ns1::foo; - using ns2::foo; - - template <typename T> - void b^ar() { - foo(T()); - })cpp"); -} - -TEST_F(DefineInlineTest, TransformNestedNamespaces) { - auto Test = R"cpp( - namespace a { - void bar(); - namespace b { - void baz(); - namespace c { - void aux(); - } - } - } - - void foo(); - using namespace a; - using namespace b; - using namespace c; - void f^oo() { - bar(); - a::bar(); - - baz(); - b::baz(); - a::b::baz(); - - aux(); - c::aux(); - b::c::aux(); - a::b::c::aux(); - })cpp"; - auto Expected = R"cpp( - namespace a { - void bar(); - namespace b { - void baz(); - namespace c { - void aux(); - } - } - } - - void foo(){ - a::bar(); - a::bar(); - - a::b::baz(); - a::b::baz(); - a::b::baz(); - - a::b::c::aux(); - a::b::c::aux(); - a::b::c::aux(); - a::b::c::aux(); - } - using namespace a; - using namespace b; - using namespace c; - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformUsings) { - auto Test = R"cpp( - namespace a { namespace b { namespace c { void aux(); } } } - - void foo(); - void f^oo() { - using namespace a; - using namespace b; - using namespace c; - using c::aux; - namespace d = c; - })cpp"; - auto Expected = R"cpp( - namespace a { namespace b { namespace c { void aux(); } } } - - void foo(){ - using namespace a; - using namespace a::b; - using namespace a::b::c; - using a::b::c::aux; - namespace d = a::b::c; - } - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformDecls) { - auto Test = R"cpp( - void foo(); - void f^oo() { - class Foo { - public: - void foo(); - int x; - static int y; - }; - Foo::y = 0; - - enum En { Zero, One }; - En x = Zero; - - enum class EnClass { Zero, One }; - EnClass y = EnClass::Zero; - })cpp"; - auto Expected = R"cpp( - void foo(){ - class Foo { - public: - void foo(); - int x; - static int y; - }; - Foo::y = 0; - - enum En { Zero, One }; - En x = Zero; - - enum class EnClass { Zero, One }; - EnClass y = EnClass::Zero; - } - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformTemplDecls) { - auto Test = R"cpp( - namespace a { - template <typename T> class Bar { - public: - void bar(); - }; - template <typename T> T bar; - template <typename T> void aux() {} - } - - void foo(); - - using namespace a; - void f^oo() { - bar<Bar<int>>.bar(); - aux<Bar<int>>(); - })cpp"; - auto Expected = R"cpp( - namespace a { - template <typename T> class Bar { - public: - void bar(); - }; - template <typename T> T bar; - template <typename T> void aux() {} - } - - void foo(){ - a::bar<a::Bar<int>>.bar(); - a::aux<a::Bar<int>>(); - } - - using namespace a; - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformMembers) { - auto Test = R"cpp( - class Foo { - void foo(); - }; - - void Foo::f^oo() { - return; - })cpp"; - auto Expected = R"cpp( - class Foo { - void foo(){ - return; - } - }; - - )cpp"; - EXPECT_EQ(apply(Test), Expected); - - ExtraFiles["a.h"] = R"cpp( - class Foo { - void foo(); - };)cpp"; - - llvm::StringMap<std::string> EditedFiles; - Test = R"cpp( - #include "a.h" - void Foo::f^oo() { - return; - })cpp"; - Expected = R"cpp( - #include "a.h" - )cpp"; - EXPECT_EQ(apply(Test, &EditedFiles), Expected); - - Expected = R"cpp( - class Foo { - void foo(){ - return; - } - };)cpp"; - EXPECT_THAT(EditedFiles, - ElementsAre(FileWithContents(testPath("a.h"), Expected))); -} - -TEST_F(DefineInlineTest, TransformDependentTypes) { - auto Test = R"cpp( - namespace a { - template <typename T> class Bar {}; - } - - template <typename T> - void foo(); - - using namespace a; - template <typename T> - void f^oo() { - Bar<T> B; - Bar<Bar<T>> q; - })cpp"; - auto Expected = R"cpp( - namespace a { - template <typename T> class Bar {}; - } - - template <typename T> - void foo(){ - a::Bar<T> B; - a::Bar<a::Bar<T>> q; - } - - using namespace a; - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformFunctionTempls) { - // Check we select correct specialization decl. - std::pair<llvm::StringRef, llvm::StringRef> Cases[] = { - {R"cpp( - template <typename T> - void foo(T p); - - template <> - void foo<int>(int p); - - template <> - void foo<char>(char p); - - template <> - void fo^o<int>(int p) { - return; - })cpp", - R"cpp( - template <typename T> - void foo(T p); - - template <> - void foo<int>(int p){ - return; - } - - template <> - void foo<char>(char p); - - )cpp"}, - {// Make sure we are not selecting the first specialization all the time. - R"cpp( - template <typename T> - void foo(T p); - - template <> - void foo<int>(int p); - - template <> - void foo<char>(char p); - - template <> - void fo^o<char>(char p) { - return; - })cpp", - R"cpp( - template <typename T> - void foo(T p); - - template <> - void foo<int>(int p); - - template <> - void foo<char>(char p){ - return; - } - - )cpp"}, - {R"cpp( - template <typename T> - void foo(T p); - - template <> - void foo<int>(int p); - - template <typename T> - void fo^o(T p) { - return; - })cpp", - R"cpp( - template <typename T> - void foo(T p){ - return; - } - - template <> - void foo<int>(int p); - - )cpp"}, - }; - for(const auto &Case : Cases) - EXPECT_EQ(apply(Case.first), Case.second) << Case.first; -} - -TEST_F(DefineInlineTest, TransformTypeLocs) { - auto Test = R"cpp( - namespace a { - template <typename T> class Bar { - public: - template <typename Q> class Baz {}; - }; - class Foo{}; - } - - void foo(); - - using namespace a; - void f^oo() { - Bar<int> B; - Foo foo; - a::Bar<Bar<int>>::Baz<Bar<int>> q; - })cpp"; - auto Expected = R"cpp( - namespace a { - template <typename T> class Bar { - public: - template <typename Q> class Baz {}; - }; - class Foo{}; - } - - void foo(){ - a::Bar<int> B; - a::Foo foo; - a::Bar<a::Bar<int>>::Baz<a::Bar<int>> q; - } - - using namespace a; - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformDeclRefs) { - auto Test =R"cpp( - namespace a { - template <typename T> class Bar { - public: - void foo(); - static void bar(); - int x; - static int y; - }; - void bar(); - void test(); - } - - void foo(); - using namespace a; - void f^oo() { - a::Bar<int> B; - B.foo(); - a::bar(); - Bar<Bar<int>>::bar(); - a::Bar<int>::bar(); - B.x = Bar<int>::y; - Bar<int>::y = 3; - bar(); - a::test(); - })cpp"; - auto Expected = R"cpp( - namespace a { - template <typename T> class Bar { - public: - void foo(); - static void bar(); - int x; - static int y; - }; - void bar(); - void test(); - } - - void foo(){ - a::Bar<int> B; - B.foo(); - a::bar(); - a::Bar<a::Bar<int>>::bar(); - a::Bar<int>::bar(); - B.x = a::Bar<int>::y; - a::Bar<int>::y = 3; - a::bar(); - a::test(); - } - using namespace a; - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, StaticMembers) { - auto Test = R"cpp( - namespace ns { class X { static void foo(); void bar(); }; } - void ns::X::b^ar() { - foo(); - })cpp"; - auto Expected = R"cpp( - namespace ns { class X { static void foo(); void bar(){ - foo(); - } }; } - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TransformInlineNamespaces) { - auto Test = R"cpp( - namespace a { inline namespace b { namespace { struct Foo{}; } } } - void foo(); - - using namespace a; - void ^foo() {Foo foo;})cpp"; - auto Expected = R"cpp( - namespace a { inline namespace b { namespace { struct Foo{}; } } } - void foo(){a::Foo foo;} - - using namespace a; - )cpp"; - EXPECT_EQ(apply(Test), Expected); -} - -TEST_F(DefineInlineTest, TokensBeforeSemicolon) { - std::pair<llvm::StringRef, llvm::StringRef> Cases[] = { - {R"cpp( - void foo() /*Comment -_-*/ /*Com 2*/ ; - void fo^o() { return ; })cpp", - R"cpp( - void foo() /*Comment -_-*/ /*Com 2*/ { return ; } - )cpp"}, - - {R"cpp( - void foo(); - void fo^o() { return ; })cpp", - R"cpp( - void foo(){ return ; } - )cpp"}, - - {R"cpp( - #define SEMI ; - void foo() SEMI - void fo^o() { return ; })cpp", - "fail: Couldn't find semicolon for target declaration."}, - }; - for(const auto& Case: Cases) - EXPECT_EQ(apply(Case.first), Case.second) << Case.first; -} - -TEST_F(DefineInlineTest, HandleMacros) { - EXPECT_UNAVAILABLE(R"cpp( - #define BODY { return; } - void foo(); - void f^oo()BODY)cpp"); - - EXPECT_UNAVAILABLE(R"cpp( - #define BODY void foo(){ return; } - void foo(); - [[BODY]])cpp"); - - std::pair<llvm::StringRef, llvm::StringRef> Cases[] = { - // We don't qualify declarations coming from macros. - {R"cpp( - #define BODY Foo - namespace a { class Foo{}; } - void foo(); - using namespace a; - void f^oo(){BODY})cpp", - R"cpp( - #define BODY Foo - namespace a { class Foo{}; } - void foo(){BODY} - using namespace a; - )cpp"}, - - // Macro is not visible at declaration location, but we proceed. - {R"cpp( - void foo(); - #define BODY return; - void f^oo(){BODY})cpp", - R"cpp( - void foo(){BODY} - #define BODY return; - )cpp"}, - - {R"cpp( - #define TARGET void foo() - TARGET; - void f^oo(){ return; })cpp", - R"cpp( - #define TARGET void foo() - TARGET{ return; } - )cpp"}, - - {R"cpp( - #define TARGET foo - void TARGET(); - void f^oo(){ return; })cpp", - R"cpp( - #define TARGET foo - void TARGET(){ return; } - )cpp"}, - }; - for(const auto& Case: Cases) - EXPECT_EQ(apply(Case.first), Case.second) << Case.first; -} - } // namespace } // namespace clangd } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
