diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -374,7 +374,8 @@ WorkScheduler.runWithAST("Rename", File, std::move(Action)); } -static llvm::Expected +// May generate several candidate selections, due to SelectionTree ambiguity. +static llvm::Expected> tweakSelection(const Range &Sel, const InputsAndAST &AST) { auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start); if (!Begin) @@ -382,7 +383,15 @@ auto End = positionToOffset(AST.Inputs.Contents, Sel.end); if (!End) return End.takeError(); - return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End); + std::vector Result; + SelectionTree::createEach(AST.AST.getASTContext(), AST.AST.getTokens(), + *Begin, *End, [&](SelectionTree T) { + Result.emplace_back(AST.Inputs.Index, AST.AST, + *Begin, *End, std::move(T)); + return false; + }); + assert(!Result.empty() && "Expected at least one SelectionTree"); + return Result; } void ClangdServer::enumerateTweaks(PathRef File, Range Sel, @@ -391,12 +400,21 @@ this](Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - auto Selection = tweakSelection(Sel, *InpAST); - if (!Selection) - return CB(Selection.takeError()); + auto Selections = tweakSelection(Sel, *InpAST); + if (!Selections) + return CB(Selections.takeError()); std::vector Res; - for (auto &T : prepareTweaks(*Selection, TweakFilter)) - Res.push_back({T->id(), T->title(), T->intent()}); + // Don't allow a tweak to fire more than once across ambiguous selections. + llvm::DenseSet PreparedTweaks; + auto Filter = [&](const Tweak &T) { + return TweakFilter(T) && !PreparedTweaks.count(T.id()); + }; + for (const auto &Sel : *Selections) { + for (auto &T : prepareTweaks(Sel, Filter)) { + Res.push_back({T->id(), T->title(), T->intent()}); + PreparedTweaks.insert(T->id()); + } + } CB(std::move(Res)); }; @@ -411,21 +429,31 @@ FS = FSProvider.getFileSystem()](Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - auto Selection = tweakSelection(Sel, *InpAST); - if (!Selection) - return CB(Selection.takeError()); - auto A = prepareTweak(TweakID, *Selection); - if (!A) - return CB(A.takeError()); - auto Effect = (*A)->apply(*Selection); - if (!Effect) - return CB(Effect.takeError()); - for (auto &It : Effect->ApplyEdits) { - Edit &E = It.second; - format::FormatStyle Style = - getFormatStyleForFile(File, E.InitialCode, FS.get()); - if (llvm::Error Err = reformatEdit(E, Style)) - elog("Failed to format {0}: {1}", It.first(), std::move(Err)); + auto Selections = tweakSelection(Sel, *InpAST); + if (!Selections) + return CB(Selections.takeError()); + llvm::Optional> Effect; + // Try each selection, take the first one that prepare()s. + // If they all fail, Effect will hold get the last error. + for (const auto &Selection : *Selections) { + auto T = prepareTweak(TweakID, Selection); + if (T) { + Effect = (*T)->apply(Selection); + break; + } else { + Effect = T.takeError(); + } + } + assert(Effect.hasValue() && "Expected at least one selection"); + if (*Effect) { + // Tweaks don't apply clang-format, do that centrally here. + for (auto &It : (*Effect)->ApplyEdits) { + Edit &E = It.second; + format::FormatStyle Style = + getFormatStyleForFile(File, E.InitialCode, FS.get()); + if (llvm::Error Err = reformatEdit(E, Style)) + elog("Failed to format {0}: {1}", It.first(), std::move(Err)); + } } return CB(std::move(*Effect)); }; diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -408,22 +408,27 @@ llvm::consumeError(Offset.takeError()); return llvm::None; } - SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset); - std::vector Result; - if (const SelectionTree::Node *N = Selection.commonAncestor()) { - DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias; - auto Decls = targetDecl(N->ASTNode, Rel); - if (!Decls.empty()) { - HI = getHoverContents(Decls.front(), Index); - // Look for a close enclosing expression to show the value of. - if (!HI->Value) - HI->Value = printExprValue(N, AST.getASTContext()); + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), *Offset, + *Offset, [&](SelectionTree ST) { + std::vector Result; + if (const SelectionTree::Node *N = ST.commonAncestor()) { + DeclRelationSet Rel = + DeclRelation::TemplatePattern | DeclRelation::Alias; + auto Decls = targetDecl(N->ASTNode, Rel); + if (!Decls.empty()) { + HI = getHoverContents(Decls.front(), Index); + // Look for a close enclosing expression to show the value of. + if (!HI->Value) + HI->Value = printExprValue(N, AST.getASTContext()); + return true; + } + // FIXME: support hovers for other nodes? + // - certain expressions (sizeof etc) + // - built-in types + // - literals (esp user-defined) } - // FIXME: support hovers for other nodes? - // - certain expressions (sizeof etc) - // - built-in types - // - literals (esp user-defined) - } + return false; + }); } if (!HI) diff --git a/clang-tools-extra/clangd/Selection.h b/clang-tools-extra/clangd/Selection.h --- a/clang-tools-extra/clangd/Selection.h +++ b/clang-tools-extra/clangd/Selection.h @@ -29,6 +29,14 @@ // - we determine which low-level nodes are partly or completely covered // by the selection. // - we expose a tree of the selected nodes and their lexical parents. +// +// Sadly LSP specifies locations as being between characters, and this causes +// some ambiguities we cannot cleanly resolve: +// lhs+rhs // targeting '+' or 'lhs'? +// ^ // in GUI editors, double-clicking 'lhs' yields this position! +// +// The best we can do in these cases is try both, which leads to the awkward +// SelectionTree::createEach() API. //===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SELECTION_H @@ -64,16 +72,32 @@ // point back into the AST it was constructed with. class SelectionTree { public: - // Creates a selection tree at the given byte offset in the main file. - // This is approximately equivalent to a range of one character. - // (Usually, the character to the right of Offset, sometimes to the left). - SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens, - unsigned Offset); - // Creates a selection tree for the given range in the main file. - // The range includes bytes [Start, End). - // If Start == End, uses the same heuristics as SelectionTree(AST, Start). - SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens, - unsigned Start, unsigned End); + // Create selection trees for the given range, and pass them to Func. + // + // There may be multiple possible selection trees: + // - if the range is empty and borders two tokens, a tree for the right token + // and a tree for the left token will be yielded. + // - Func should return true on success (stop) and false on failure (continue) + // + // Always yields at least one tree. If no tokens are touched, it is empty. + static bool createEach(ASTContext &AST, const syntax::TokenBuffer &Tokens, + unsigned Begin, unsigned End, + llvm::function_ref Func); + + // Create a selection tree for the given range. + // + // Where ambiguous (range is empty and borders two tokens), prefer the token + // on the right. + static SelectionTree createRight(ASTContext &AST, + const syntax::TokenBuffer &Tokens, + unsigned Begin, unsigned End); + + // Copies are no good - contain pointers to other nodes. + SelectionTree(const SelectionTree &) = delete; + SelectionTree &operator=(const SelectionTree &) = delete; + // Moves are OK though - internal storage is pointer-stable when moved. + SelectionTree(SelectionTree &&) = default; + SelectionTree &operator=(SelectionTree &&) = default; // Describes to what extent an AST node is covered by the selection. enum Selection : unsigned char { @@ -121,6 +145,11 @@ const Node &root() const { return *Root; } private: + // Creates a selection tree for the given range in the main file. + // The range includes bytes [Start, End). + SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens, + unsigned Start, unsigned End); + std::deque Nodes; // Stable-pointer storage. const Node *Root; clang::PrintingPolicy PrintPolicy; diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -142,6 +142,11 @@ Result = SelectionTree::Partial; } +// As well as comments, don't count semicolons as real tokens. +// They're not properly claimed as expr-statement is missing from the AST. +bool shouldIgnore(const syntax::Token &Tok) { + return Tok.kind() == tok::comment || Tok.kind() == tok::semi; +} // SelectionTester can determine whether a range of tokens from the PP-expanded // stream (corresponding to an AST node) is considered selected. @@ -172,9 +177,7 @@ }); // Precompute selectedness and offset for selected spelled tokens. for (const syntax::Token *T = SelFirst; T < SelLimit; ++T) { - // As well as comments, don't count semicolons as real tokens. - // They're not properly claimed as expr-statement is missing from the AST. - if (T->kind() == tok::comment || T->kind() == tok::semi) + if (shouldIgnore(*T)) continue; SpelledTokens.emplace_back(); Tok &S = SpelledTokens.back(); @@ -650,24 +653,49 @@ return std::move(OS.str()); } -// Decide which selection emulates a "point" query in between characters. -static std::pair pointBounds(unsigned Offset, FileID FID, - ASTContext &AST) { - StringRef Buf = AST.getSourceManager().getBufferData(FID); - // Edge-cases where the choice is forced. - if (Buf.size() == 0) - return {0, 0}; - if (Offset == 0) - return {0, 1}; - if (Offset == Buf.size()) - return {Offset - 1, Offset}; - // We could choose either this byte or the previous. Usually we prefer the - // character on the right of the cursor (or under a block cursor). - // But if that's whitespace/semicolon, we likely want the token on the left. - auto IsIgnoredChar = [](char C) { return isWhitespace(C) || C == ';'; }; - if (IsIgnoredChar(Buf[Offset]) && !IsIgnoredChar(Buf[Offset - 1])) - return {Offset - 1, Offset}; - return {Offset, Offset + 1}; +// Decide which selections emulate a "point" query in between characters. +// If it's ambiguous (the neighboring characters are selectable tokens), returns +// both possibilities in preference order. +// Always returns at least one range - if no tokens touched, and empty range. +static llvm::SmallVector, 2> +pointBounds(unsigned Offset, const syntax::TokenBuffer &Tokens) { + const auto& SM = Tokens.sourceManager(); + SourceLocation Loc = SM.getComposedLoc(SM.getMainFileID(), Offset); + llvm::SmallVector, 2> Result; + // Prefer right token over left. + for (const syntax::Token &Tok : + llvm::reverse(spelledTokensTouching(Loc, Tokens))) { + if (shouldIgnore(Tok)) + continue; + unsigned Offset = Tokens.sourceManager().getFileOffset(Tok.location()); + Result.emplace_back(Offset, Offset + Tok.length()); + } + if (Result.empty()) + Result.emplace_back(Offset, Offset); + return Result; +} + +bool SelectionTree::createEach(ASTContext &AST, + const syntax::TokenBuffer &Tokens, + unsigned Begin, unsigned End, + llvm::function_ref Func) { + if (Begin != End) + return Func(SelectionTree(AST, Tokens, Begin, End)); + for (std::pair Bounds : pointBounds(Begin, Tokens)) + if (Func(SelectionTree(AST, Tokens, Bounds.first, Bounds.second))) + return true; + return false; +} + +SelectionTree SelectionTree::createRight(ASTContext &AST, + const syntax::TokenBuffer &Tokens, + unsigned int Begin, unsigned int End) { + llvm::Optional Result; + createEach(AST, Tokens, Begin, End, [&](SelectionTree T) { + Result = std::move(T); + return true; + }); + return std::move(*Result); } SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens, @@ -677,8 +705,6 @@ // but that's all clangd has needed so far. const SourceManager &SM = AST.getSourceManager(); FileID FID = SM.getMainFileID(); - if (Begin == End) - std::tie(Begin, End) = pointBounds(Begin, FID, AST); PrintPolicy.TerseOutput = true; PrintPolicy.IncludeNewlines = false; @@ -690,10 +716,6 @@ dlog("Built selection tree\n{0}", *this); } -SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens, - unsigned Offset) - : SelectionTree(AST, Tokens, Offset, Offset) {} - const Node *SelectionTree::commonAncestor() const { const Node *Ancestor = Root; while (Ancestor->Children.size() == 1 && !Ancestor->Selected) diff --git a/clang-tools-extra/clangd/SemanticSelection.cpp b/clang-tools-extra/clangd/SemanticSelection.cpp --- a/clang-tools-extra/clangd/SemanticSelection.cpp +++ b/clang-tools-extra/clangd/SemanticSelection.cpp @@ -39,7 +39,8 @@ } // Get node under the cursor. - SelectionTree ST(AST.getASTContext(), AST.getTokens(), *Offset); + SelectionTree ST = SelectionTree::createRight( + AST.getASTContext(), AST.getTokens(), *Offset, *Offset); for (const auto *Node = ST.commonAncestor(); Node != nullptr; Node = Node->Parent) { if (const Decl *D = Node->ASTNode.get()) { diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -131,15 +131,16 @@ std::vector getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, DeclRelationSet Relations) { - FileID FID; - unsigned Offset; - std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos); - SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset); - std::vector Result; - if (const SelectionTree::Node *N = Selection.commonAncestor()) { - auto Decls = targetDecl(N->ASTNode, Relations); - Result.assign(Decls.begin(), Decls.end()); - } + unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(Pos).second; + std::vector Result; + SelectionTree::createEach( + AST.getASTContext(), AST.getTokens(), Offset, Offset, + [&](SelectionTree ST) { + if (const SelectionTree::Node *N = ST.commonAncestor()) + llvm::copy(targetDecl(N->ASTNode, Relations), + std::back_inserter(Result)); + return !Result.empty(); + }); return Result; } diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -78,7 +78,8 @@ unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second; - SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset); + SelectionTree Selection = SelectionTree::createRight( + AST.getASTContext(), AST.getTokens(), Offset, Offset); const SelectionTree::Node *SelectedNode = Selection.commonAncestor(); if (!SelectedNode) return {}; diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h --- a/clang-tools-extra/clangd/refactor/Tweak.h +++ b/clang-tools-extra/clangd/refactor/Tweak.h @@ -48,7 +48,7 @@ /// Input to prepare and apply tweaks. struct Selection { Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin, - unsigned RangeEnd); + unsigned RangeEnd, SelectionTree ASTSelection); /// The text of the active document. llvm::StringRef Code; /// The Index for handling codebase related queries. diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp --- a/clang-tools-extra/clangd/refactor/Tweak.cpp +++ b/clang-tools-extra/clangd/refactor/Tweak.cpp @@ -46,10 +46,10 @@ } // namespace Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST, - unsigned RangeBegin, unsigned RangeEnd) + unsigned RangeBegin, unsigned RangeEnd, + SelectionTree ASTSelection) : Index(Index), AST(AST), SelectionBegin(RangeBegin), - SelectionEnd(RangeEnd), - ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) { + SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) { auto &SM = AST.getSourceManager(); Code = SM.getBufferData(SM.getMainFileID()); Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin); diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -77,8 +77,8 @@ auto AST = TU.build(); EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code; llvm::Annotations::Range R = A.range(); - SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin, - R.End); + auto Selection = SelectionTree::createRight( + AST.getASTContext(), AST.getTokens(), R.Begin, R.End); const SelectionTree::Node *N = Selection.commonAncestor(); if (!N) { ADD_FAILURE() << "No node selected!\n" << Code; diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -549,7 +549,7 @@ } )cpp", R"cpp(// Template auto parameter. Nothing (Not useful). - template<^auto T> + template void func() { } void foo() { diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -19,20 +19,26 @@ namespace { using ::testing::UnorderedElementsAreArray; +// Create a selection tree corresponding to a point or pair of points. +// This uses the precisely-defined createRight semantics. The fuzzier +// createEach is tested separately. SelectionTree makeSelectionTree(const StringRef MarkedCode, ParsedAST &AST) { Annotations Test(MarkedCode); switch (Test.points().size()) { - case 1: // Point selection. - return SelectionTree(AST.getASTContext(), AST.getTokens(), - cantFail(positionToOffset(Test.code(), Test.point()))); + case 1: { // Point selection. + unsigned Offset = cantFail(positionToOffset(Test.code(), Test.point())); + return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), + Offset, Offset); + } case 2: // Range selection. - return SelectionTree( + return SelectionTree::createRight( AST.getASTContext(), AST.getTokens(), cantFail(positionToOffset(Test.code(), Test.points()[0])), cantFail(positionToOffset(Test.code(), Test.points()[1]))); default: ADD_FAILURE() << "Expected 1-2 points for selection.\n" << MarkedCode; - return SelectionTree(AST.getASTContext(), AST.getTokens(), 0u, 0u); + return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0u, + 0u); } } @@ -507,6 +513,63 @@ EXPECT_EQ("CXXConstructExpr", nodeKind(&Str->outerImplicit())); } +TEST(SelectionTest, CreateAll) { + llvm::Annotations Test("int$unique^ a=1$ambiguous^+1; $empty^"); + auto AST = TestTU::withCode(Test.code()).build(); + unsigned Seen = 0; + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), + Test.point("ambiguous"), Test.point("ambiguous"), + [&](SelectionTree T) { + // Expect to see the right-biased tree first. + if (Seen == 0) + EXPECT_EQ("BinaryOperator", + nodeKind(T.commonAncestor())); + else if (Seen == 1) + EXPECT_EQ("IntegerLiteral", + nodeKind(T.commonAncestor())); + ++Seen; + return false; + }); + EXPECT_EQ(2u, Seen); + + Seen = 0; + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), + Test.point("ambiguous"), Test.point("ambiguous"), + [&](SelectionTree T) { + ++Seen; + return true; + }); + EXPECT_EQ(1u, Seen) << "Return true --> stop iterating"; + + Seen = 0; + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), + Test.point("unique"), Test.point("unique"), + [&](SelectionTree T) { + ++Seen; + return false; + }); + EXPECT_EQ(1u, Seen) << "no ambiguity --> only one tree"; + + Seen = 0; + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), + Test.point("empty"), Test.point("empty"), + [&](SelectionTree T) { + EXPECT_FALSE(T.commonAncestor()); + ++Seen; + return false; + }); + EXPECT_EQ(1u, Seen) << "empty tree still created"; + + Seen = 0; + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), + Test.point("unique"), Test.point("ambiguous"), + [&](SelectionTree T) { + ++Seen; + return false; + }); + EXPECT_EQ(1u, Seen) << "one tree for nontrivial selection"; +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp @@ -63,12 +63,33 @@ cantFail(positionToOffset(A.code(), SelectionRng.end))}; } +// Prepare and apply the specified tweak based on the selection in Input. +// Returns None if and only if prepare() failed. +llvm::Optional> +applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID, + const SymbolIndex *Index) { + auto Range = rangeOrPoint(Input); + llvm::Optional> Result; + SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first, + Range.second, [&](SelectionTree ST) { + Tweak::Selection S(Index, AST, Range.first, + Range.second, std::move(ST)); + if (auto T = prepareTweak(TweakID, S)) { + Result = (*T)->apply(S); + return true; + } else { + llvm::consumeError(T.takeError()); + return false; + } + }); + return Result; +} + MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index, FileName, (TweakID + (negation ? " is unavailable" : " is available")).str()) { std::string WrappedCode = wrap(Ctx, arg); Annotations Input(WrappedCode); - auto Selection = rangeOrPoint(Input); TestTU TU; TU.Filename = FileName; TU.HeaderCode = Header; @@ -76,12 +97,11 @@ TU.ExtraArgs = ExtraArgs; TU.AdditionalFiles = std::move(ExtraFiles); ParsedAST AST = TU.build(); - Tweak::Selection S(Index, AST, Selection.first, Selection.second); - auto PrepareResult = prepareTweak(TweakID, S); - if (PrepareResult) - return true; - llvm::consumeError(PrepareResult.takeError()); - return false; + auto Result = applyTweak(AST, Input, TweakID, Index); + // We only care if prepare() succeeded, but must handle Errors. + if (Result && !*Result) + consumeError(Result->takeError()); + return Result.hasValue(); } } // namespace @@ -90,8 +110,6 @@ llvm::StringMap *EditedFiles) const { std::string WrappedCode = wrap(Context, MarkedCode); Annotations Input(WrappedCode); - auto Selection = rangeOrPoint(Input); - TestTU TU; TU.Filename = FileName; TU.HeaderCode = Header; @@ -99,23 +117,20 @@ TU.Code = Input.code(); TU.ExtraArgs = ExtraArgs; ParsedAST AST = TU.build(); - Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second); - auto T = prepareTweak(TweakID, S); - if (!T) { - llvm::consumeError(T.takeError()); - return "unavailable"; - } - llvm::Expected Result = (*T)->apply(S); + auto Result = applyTweak(AST, Input, TweakID, Index.get()); if (!Result) - return "fail: " + llvm::toString(Result.takeError()); - if (Result->ShowMessage) - return "message:\n" + *Result->ShowMessage; - if (Result->ApplyEdits.empty()) + return "unavailable"; + if (!*Result) + return "fail: " + llvm::toString(Result->takeError()); + const auto &Effect = **Result; + if ((*Result)->ShowMessage) + return "message:\n" + *Effect.ShowMessage; + if (Effect.ApplyEdits.empty()) return "no effect"; std::string EditedMainFile; - for (auto &It : Result->ApplyEdits) { + for (auto &It : Effect.ApplyEdits) { auto NewText = It.second.apply(); if (!NewText) return "bad edits: " + llvm::toString(NewText.takeError()); 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 @@ -1903,7 +1903,7 @@ // Basic check for function body and signature. EXPECT_AVAILABLE(R"cpp( class Bar { - [[void [[f^o^o]]() [[{ return; }]]]] + [[void [[f^o^o^]]() [[{ return; }]]]] }; void foo();