diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -77,7 +77,8 @@ /// Transforms a tweak into a code action that would apply it if executed. /// EXPECTS: T.prepare() was called and returned true. CodeAction toCodeAction(const ClangdServer::TweakRef &T, const URIForFile &File, - Range Selection) { + Range Selection, + const std::vector &RequestedActionKinds) { CodeAction CA; CA.title = T.Title; CA.kind = T.Kind.str(); @@ -93,6 +94,7 @@ Args.file = File; Args.tweakID = T.ID; Args.selection = Selection; + Args.requestedActionKinds = RequestedActionKinds; CA.command->argument = std::move(Args); return CA; } @@ -648,7 +650,8 @@ ? llvm::json::Object{{"codeActionKinds", {CodeAction::QUICKFIX_KIND, CodeAction::REFACTOR_KIND, - CodeAction::INFO_KIND}}} + CodeAction::INFO_KIND, + CodeAction::SOURCE_KIND}}} : llvm::json::Value(true); std::vector Commands; @@ -804,7 +807,12 @@ // ApplyEdit will take care of calling Reply(). return applyEdit(std::move(WE), "Tweak applied.", std::move(Reply)); }; - Server->applyTweak(Args.file.file(), Args.selection, Args.tweakID, + Server->applyTweak(Args.tweakID, + {Args.file.file().str(), + Args.selection, + Args.requestedActionKinds, + {}, + {}}, std::move(Action)); } @@ -1023,13 +1031,10 @@ Inputs.File = File.file(); Inputs.Selection = Params.range; Inputs.RequestedActionKinds = Params.context.only; - Inputs.TweakFilter = [this](const Tweak &T) { - return Opts.TweakFilter(T); - }; - auto CB = [this, - Reply = std::move(Reply), - ToLSPDiags = std::move(ToLSPDiags), File, - Selection = Params.range]( + Inputs.TweakFilter = [this](const Tweak &T) { return Opts.TweakFilter(T); }; + auto CB = [this, Reply = std::move(Reply), ToLSPDiags = std::move(ToLSPDiags), + File, Selection = Params.range, + ActionKinds = Inputs.RequestedActionKinds]( llvm::Expected Fixits) mutable { if (!Fixits) return Reply(Fixits.takeError()); @@ -1038,13 +1043,12 @@ for (const auto &QF : Fixits->QuickFixes) { CAs.push_back(toCodeAction(QF.F, File, Version, SupportsDocumentChanges, SupportsChangeAnnotation)); - if (auto It = ToLSPDiags.find(QF.Diag); - It != ToLSPDiags.end()) { + if (auto It = ToLSPDiags.find(QF.Diag); It != ToLSPDiags.end()) { CAs.back().diagnostics = {It->second}; } } for (const auto &TR : Fixits->TweakRefs) - CAs.push_back(toCodeAction(TR, File, Selection)); + CAs.push_back(toCodeAction(TR, File, Selection, ActionKinds)); // If there's exactly one quick-fix, call it "preferred". // We never consider refactorings etc as preferred. diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -386,8 +386,8 @@ void codeAction(const CodeActionInputs &Inputs, Callback CB); - /// Apply the code tweak with a specified \p ID. - void applyTweak(PathRef File, Range Sel, StringRef ID, + /// Apply the code tweak with a specified \p tweakID. + void applyTweak(std::string TweakID, CodeActionInputs Inputs, Callback CB); /// Called when an event occurs for a watched file in the workspace. 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 @@ -629,7 +629,8 @@ // vector of pointers because GCC doesn't like non-copyable Selection. static llvm::Expected>> tweakSelection(const Range &Sel, const InputsAndAST &AST, - llvm::vfs::FileSystem *FS) { + llvm::vfs::FileSystem *FS, + const std::vector &RequestedActionKinds) { auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start); if (!Begin) return Begin.takeError(); @@ -637,13 +638,14 @@ if (!End) return End.takeError(); std::vector> Result; - SelectionTree::createEach( - AST.AST.getASTContext(), AST.AST.getTokens(), *Begin, *End, - [&](SelectionTree T) { - Result.push_back(std::make_unique( - AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T), FS)); - return false; - }); + SelectionTree::createEach(AST.AST.getASTContext(), AST.AST.getTokens(), + *Begin, *End, [&](SelectionTree T) { + Result.push_back( + std::make_unique( + AST.Inputs.Index, AST.AST, *Begin, *End, + std::move(T), FS, RequestedActionKinds)); + return false; + }); assert(!Result.empty() && "Expected at least one SelectionTree"); return std::move(Result); } @@ -681,7 +683,8 @@ } // Collect Tweaks - auto Selections = tweakSelection(Params.Selection, *InpAST, /*FS=*/nullptr); + auto Selections = tweakSelection(Params.Selection, *InpAST, /*FS=*/nullptr, + Params.RequestedActionKinds); if (!Selections) return CB(Selections.takeError()); // Don't allow a tweak to fire more than once across ambiguous selections. @@ -704,7 +707,7 @@ Transient); } -void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID, +void ClangdServer::applyTweak(std::string TweakID, CodeActionInputs Inputs, Callback CB) { // Tracks number of times a tweak has been attempted. static constexpr trace::Metric TweakAttempt( @@ -713,13 +716,14 @@ static constexpr trace::Metric TweakFailed( "tweak_failed", trace::Metric::Counter, "tweak_id"); TweakAttempt.record(1, TweakID); - auto Action = [File = File.str(), Sel, TweakID = TweakID.str(), - CB = std::move(CB), + auto Action = [File = Inputs.File, Sel = std::move(Inputs.Selection), + TweakID = std::move(TweakID), CB = std::move(CB), + ActionKinds = std::move(Inputs.RequestedActionKinds), this](Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); auto FS = DirtyFS->view(std::nullopt); - auto Selections = tweakSelection(Sel, *InpAST, FS.get()); + auto Selections = tweakSelection(Sel, *InpAST, FS.get(), ActionKinds); if (!Selections) return CB(Selections.takeError()); std::optional> Effect; @@ -748,7 +752,7 @@ } return CB(std::move(*Effect)); }; - WorkScheduler->runWithAST("ApplyTweak", File, std::move(Action)); + WorkScheduler->runWithAST("ApplyTweak", Inputs.File, std::move(Action)); } void ClangdServer::locateSymbolAt(PathRef File, Position Pos, diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -28,6 +28,7 @@ #include "clang-include-cleaner/Record.h" #include "support/Path.h" #include "clang/Frontend/FrontendAction.h" +#include "clang/Lex/Lexer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/ArrayRef.h" @@ -118,6 +119,11 @@ /// AST. Might be std::nullopt if no Preamble is used. std::optional preambleVersion() const; + // Describes the bounds of the preamble. + PreambleBounds getPreambleBounds() const { + return Preamble->Preamble.getBounds(); + } + const HeuristicResolver *getHeuristicResolver() const { return Resolver.get(); } diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -28,6 +28,7 @@ #include "support/MemoryTree.h" #include "clang/Index/IndexSymbol.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" #include @@ -1035,6 +1036,9 @@ Range selection; /// ID of the tweak that should be executed. Corresponds to Tweak::id(). std::string tweakID; + /// Code action kinds requested by the client via the `only` field in + /// the context. + std::vector requestedActionKinds; }; bool fromJSON(const llvm::json::Value &, TweakArgs &, llvm::json::Path); llvm::json::Value toJSON(const TweakArgs &A); @@ -1070,6 +1074,7 @@ const static llvm::StringLiteral QUICKFIX_KIND; const static llvm::StringLiteral REFACTOR_KIND; const static llvm::StringLiteral INFO_KIND; + const static llvm::StringLiteral SOURCE_KIND; /// The diagnostics that this code action resolves. std::optional> diagnostics; diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -869,6 +869,7 @@ const llvm::StringLiteral CodeAction::QUICKFIX_KIND = "quickfix"; const llvm::StringLiteral CodeAction::REFACTOR_KIND = "refactor"; const llvm::StringLiteral CodeAction::INFO_KIND = "info"; +const llvm::StringLiteral CodeAction::SOURCE_KIND = "source"; llvm::json::Value toJSON(const CodeAction &CA) { auto CodeAction = llvm::json::Object{{"title", CA.title}}; @@ -928,12 +929,15 @@ llvm::json::Path P) { llvm::json::ObjectMapper O(Params, P); return O && O.map("file", A.file) && O.map("selection", A.selection) && - O.map("tweakID", A.tweakID); + O.map("tweakID", A.tweakID) && + O.map("requestedActionKinds", A.requestedActionKinds); } llvm::json::Value toJSON(const TweakArgs &A) { - return llvm::json::Object{ - {"tweakID", A.tweakID}, {"selection", A.selection}, {"file", A.file}}; + return llvm::json::Object{{"tweakID", A.tweakID}, + {"selection", A.selection}, + {"file", A.file}, + {"requestedActionKinds", A.requestedActionKinds}}; } llvm::json::Value toJSON(const ApplyWorkspaceEditParams &Params) { 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 @@ -49,7 +49,8 @@ struct Selection { Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd, SelectionTree ASTSelection, - llvm::vfs::FileSystem *VFS); + llvm::vfs::FileSystem *VFS, + const std::vector &RequestedActionKinds); /// The text of the active document. llvm::StringRef Code; /// The Index for handling codebase related queries. @@ -68,6 +69,9 @@ /// File system used to access source code (for cross-file tweaks). /// This is only populated when applying a tweak, not during prepare. llvm::vfs::FileSystem *FS = nullptr; + /// Requested code action kinds from the `only` field of + /// code action request context. + std::vector RequestedActionKinds; // FIXME: provide a way to get sources and ASTs for other files. }; 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 @@ -57,12 +57,13 @@ } } // namespace -Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST, - unsigned RangeBegin, unsigned RangeEnd, - SelectionTree ASTSelection, - llvm::vfs::FileSystem *FS) +Tweak::Selection::Selection( + const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin, + unsigned RangeEnd, SelectionTree ASTSelection, llvm::vfs::FileSystem *FS, + const std::vector &RequestedActionKinds) : Index(Index), AST(&AST), SelectionBegin(RangeBegin), - SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)), FS(FS) { + SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)), FS(FS), + RequestedActionKinds(RequestedActionKinds) { auto &SM = AST.getSourceManager(); Code = SM.getBufferData(SM.getMainFileID()); Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin); 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 @@ -24,6 +24,7 @@ MemberwiseConstructor.cpp ObjCLocalizeStringLiteral.cpp ObjCMemberwiseInitializer.cpp + OrganizeImports.cpp PopulateSwitch.cpp RawStringLiteral.cpp RemoveUsingNamespace.cpp diff --git a/clang-tools-extra/clangd/refactor/tweaks/OrganizeImports.cpp b/clang-tools-extra/clangd/refactor/tweaks/OrganizeImports.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/refactor/tweaks/OrganizeImports.cpp @@ -0,0 +1,105 @@ +//===--- OrganizeImports.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 "IncludeCleaner.h" +#include "Protocol.h" +#include "SourceCode.h" +#include "clang-include-cleaner/IncludeSpeller.h" +#include "clang-include-cleaner/Types.h" +#include "refactor/Tweak.h" +#include "support/Logger.h" +#include "clang/Basic/LLVM.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Format/Format.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" +#include +#include +#include +#include +#include + +namespace clang::clangd { +namespace { + +// Tweak for applying IWYU-related changes (removing unused and adding missing +// includes) in batch. The tweak can be triggered via the "Organize Imports" +// source action (VS Code). The set of changes applied fully corresponds to the +// findings from the clang-include-cleaner tool. +class OrganizeImports : public Tweak { +public: + const char *id() const override; + + bool prepare(const Selection &Inputs) override; + Expected apply(const Selection &Inputs) override; + std::string title() const override { + return "Remove unused and add missing includes"; + } + llvm::StringLiteral kind() const override { return CodeAction::SOURCE_KIND; } +}; +REGISTER_TWEAK(OrganizeImports) + +bool OrganizeImports::prepare(const Tweak::Selection &Inputs) { + if (Inputs.AST->getLangOpts().ObjC) + return false; + if (std::find(Inputs.RequestedActionKinds.begin(), + Inputs.RequestedActionKinds.end(), + CodeAction::SOURCE_KIND) != Inputs.RequestedActionKinds.end()) + return true; + if (Inputs.RequestedActionKinds.empty()) + // To accommodate clients without knowledge of source actions, we trigger + // without checking code action kinds when inside the preamble region. + return offsetToPosition(Inputs.Code, Inputs.SelectionEnd) <= + offsetToPosition(Inputs.Code, Inputs.AST->getPreambleBounds().Size); + return false; +} + +Expected OrganizeImports::apply(const Selection &Inputs) { + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(*Inputs.AST); + const auto MainFilePath = Inputs.AST->tuPath(); + tooling::Replacements Replacements; + for (const auto *Inc : Findings.UnusedIncludes) + llvm::cantFail(Replacements.add( + tooling::Replacement{MainFilePath, UINT_MAX, 1, Inc->Written})); + + const auto &SM = Inputs.AST->getSourceManager(); + llvm::DenseSet Providers; + for (const auto &Missing : Findings.MissingIncludes) { + assert(!Missing.Providers.empty()); + Providers.insert(Missing.Providers[0]); + } + + for (const auto &P : Providers) { + std::string Spelling = include_cleaner::spellHeader( + {P, Inputs.AST->getPreprocessor().getHeaderSearchInfo(), + SM.getFileEntryForID(SM.getMainFileID())}); + llvm::cantFail(Replacements.add(tooling::Replacement{ + MainFilePath, UINT_MAX, 0, "#include " + Spelling})); + } + + auto FileStyle = + format::getStyle(format::DefaultFormatStyle, MainFilePath, + format::DefaultFallbackStyle, Inputs.Code, Inputs.FS); + if (!FileStyle) { + elog("Couldn't get style for {0}: {1}", MainFilePath, + FileStyle.takeError()); + FileStyle = format::getLLVMStyle(); + } + auto Final = + format::cleanupAroundReplacements(Inputs.Code, Replacements, *FileStyle); + if (!Final) + return Final.takeError(); + if (Final->empty()) + return Tweak::Effect{"No edits to apply.", {}}; + return Effect::mainFileEdit(SM, *Final); +} + +} // namespace +} // namespace clang::clangd diff --git a/clang-tools-extra/clangd/test/code-action-request.test b/clang-tools-extra/clangd/test/code-action-request.test --- a/clang-tools-extra/clangd/test/code-action-request.test +++ b/clang-tools-extra/clangd/test/code-action-request.test @@ -33,6 +33,7 @@ # CHECK-NEXT: "arguments": [ # CHECK-NEXT: { # CHECK-NEXT: "file": "file://{{.*}}/clangd-test/main.cpp", +# CHECK-NEXT: "requestedActionKinds": [], # CHECK-NEXT: "selection": { # CHECK-NEXT: "end": { # CHECK-NEXT: "character": 4, @@ -92,7 +93,7 @@ # CHECK-NEXT: "result": [ # CHECK-NEXT: { --- -{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","requestedActionKinds": [],"selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} # CHECK: "newText": "int", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test --- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test +++ b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test @@ -486,6 +486,27 @@ # CHECK-NEXT: ], # CHECK-NEXT: "command": "clangd.applyFix", # CHECK-NEXT: "title": "Apply fix: fix all includes" +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "arguments": [ +# CHECK-NEXT: { +# CHECK-NEXT: "file": "file:///clangd-test/simple.cpp", +# CHECK-NEXT: "requestedActionKinds": [], +# CHECK-NEXT: "selection": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 17, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 0, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "tweakID": "OrganizeImports" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "command": "clangd.applyTweak", +# CHECK-NEXT: "title": "Remove unused and add missing includes" # CHECK-NEXT: } # CHECK-NEXT: ] --- diff --git a/clang-tools-extra/clangd/test/request-reply.test b/clang-tools-extra/clangd/test/request-reply.test --- a/clang-tools-extra/clangd/test/request-reply.test +++ b/clang-tools-extra/clangd/test/request-reply.test @@ -3,7 +3,7 @@ --- {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto i = 0;"}}} --- -{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} +{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","requestedActionKinds":[],"selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} # CHECK: "id": 0, # CHECK: "method": "workspace/applyEdit", # CHECK: "newText": "int", @@ -25,7 +25,7 @@ # CHECK-NEXT: }, # CHECK-NEXT: "id": 4, --- -{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} +{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","requestedActionKinds":[],"selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandDeducedType"}]}} # CHECK: "id": 1, # CHECK: "method": "workspace/applyEdit", --- diff --git a/clang-tools-extra/clangd/test/tweaks-format.test b/clang-tools-extra/clangd/test/tweaks-format.test --- a/clang-tools-extra/clangd/test/tweaks-format.test +++ b/clang-tools-extra/clangd/test/tweaks-format.test @@ -5,7 +5,7 @@ --- {"jsonrpc":"2.0","id":5,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///main.cc"},"range":{"start":{"line":0,"character":11},"end":{"line":0,"character":11}},"context":{"diagnostics":[]}}} --- -{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}} +{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","requestedActionKinds":[],"selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}} # CHECK: "newText": "\n ", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -402,7 +402,7 @@ auto Tree = SelectionTree::createRight(AST->getASTContext(), AST->getTokens(), Start, End); Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree), - nullptr); + nullptr, {}); // FS is only populated when applying a tweak, not during prepare as // prepare should not do any I/O to be fast. auto Tweaks = diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -128,6 +128,7 @@ tweaks/MemberwiseConstructorTests.cpp tweaks/ObjCLocalizeStringLiteralTests.cpp tweaks/ObjCMemberwiseInitializerTests.cpp + tweaks/OrganizeImportsTests.cpp tweaks/PopulateSwitchTests.cpp tweaks/RawStringLiteralTests.cpp tweaks/RemoveUsingNamespaceTests.cpp diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -1303,12 +1303,13 @@ // Ensure that disabled formatting is respected. Notification N; - Server.applyTweak(FooCpp, {}, TweakID, [&](llvm::Expected E) { - ASSERT_TRUE(static_cast(E)); - EXPECT_THAT(llvm::cantFail(E->ApplyEdits.lookup(FooCpp).apply()), - NewContents); - N.notify(); - }); + Server.applyTweak( + TweakID, {FooCpp, {}, {}, {}, {}}, [&](llvm::Expected E) { + ASSERT_TRUE(static_cast(E)); + EXPECT_THAT(llvm::cantFail(E->ApplyEdits.lookup(FooCpp).apply()), + NewContents); + N.notify(); + }); N.wait(); } diff --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp --- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp +++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp @@ -49,8 +49,8 @@ auto Tree = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0, 0); auto Actual = prepareTweak( - TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree), nullptr), - &Set); + TweakID, + Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree), nullptr, {}), &Set); ASSERT_TRUE(bool(Actual)); EXPECT_EQ(Actual->get()->id(), TweakID); } diff --git a/clang-tools-extra/clangd/unittests/tweaks/OrganizeImportsTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/OrganizeImportsTests.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/tweaks/OrganizeImportsTests.cpp @@ -0,0 +1,120 @@ +//===--OrganizeImportsTest.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 "Protocol.h" +#include "TweakTesting.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Testing/Annotations/Annotations.h" +#include "gtest/gtest.h" +#include +#include + +namespace clang { +namespace clangd { +namespace { + +TWEAK_TEST(OrganizeImports); + +TEST_F(OrganizeImportsTest, Prepare) { + struct { + llvm::StringRef Code; + bool IsAvailable; + const std::vector RequestedActionKinds; + } Cases[] = {{ + R"cpp( +#include "Te^stTU.h" +)cpp", + true, + {CodeAction::SOURCE_KIND.str()}}, + {"void foo(^) {}", true, {CodeAction::SOURCE_KIND.str()}}, + { + R"cpp( +#include "Te^stTU.h" +)cpp", + true, + {}}, + {"void foo(^) {}", false, {}}}; + + for (auto Case : Cases) { + llvm::Annotations A{Case.Code}; + auto AST = build(A.code()); + for (const auto &P : A.points()) + EXPECT_EQ(Case.IsAvailable, + isAvailable(AST, {P, P}, Case.RequestedActionKinds)) + << decorate(A.code(), P); + } +} + +TEST_F(OrganizeImportsTest, Apply) { + Header = "void foo();"; + struct { + llvm::StringRef Code; + llvm::StringRef Header; + llvm::StringRef Expected; + } Cases[] = {{// Remove unused include. + R"cpp( +#include "TestTU.h" +void foo() {} +void b^ar() { + foo(); +} +)cpp", + R"cpp( +#pragma once +)cpp", + R"cpp( +void foo() {} +void bar() { + foo(); +} +)cpp"}, + {// Add missing include. + R"cpp( +void b^ar() { + foo(); +} +)cpp", + R"cpp( +#pragma once +void foo(); +)cpp", + R"cpp( +#include "TestTU.h" +void bar() { + foo(); +} +)cpp"}, + {// Replace unused include with missing. + R"cpp( +#include "foo.h" +void b^ar() { + foo(); +} +)cpp", + R"cpp( +#pragma once +void foo(); +)cpp", + R"cpp( +#include "TestTU.h" +void bar() { + foo(); +} +)cpp"}}; + for (auto C : Cases) { + Header = C.Header; + ExtraFiles["foo.h"] = "#pragma once"; + EXPECT_EQ(C.Expected, + apply(C.Code, nullptr, {CodeAction::SOURCE_KIND.str()})) + << C.Expected; + } +} + +} // namespace +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h --- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h +++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h @@ -18,6 +18,7 @@ #include "gtest/gtest.h" #include #include +#include namespace clang { namespace clangd { @@ -88,13 +89,16 @@ // - if the tweak produces a message, returns "message:\n" // - if prepare() returns false, returns "unavailable" // - if apply() returns an error, returns "fail: " - std::string apply(llvm::StringRef MarkedCode, - llvm::StringMap *EditedFiles = nullptr) const; + std::string + apply(llvm::StringRef MarkedCode, + llvm::StringMap *EditedFiles = nullptr, + const std::vector &RequestedActionKinds = {}) const; // Helpers for EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macros. using WrappedAST = std::pair; WrappedAST build(llvm::StringRef) const; - bool isAvailable(WrappedAST &, llvm::Annotations::Range) const; + bool isAvailable(WrappedAST &, llvm::Annotations::Range, + const std::vector & = {}) const; // Return code re-decorated with a single point/range. static std::string decorate(llvm::StringRef, unsigned); static std::string decorate(llvm::StringRef, llvm::Annotations::Range); @@ -116,9 +120,10 @@ auto AST = build(A.code()); \ assert(!A.points().empty() || !A.ranges().empty()); \ for (const auto &P : A.points()) \ - EXPECT_EQ(Available, isAvailable(AST, {P, P})) << decorate(A.code(), P); \ + EXPECT_EQ(Available, isAvailable(AST, {P, P}, {})) \ + << decorate(A.code(), P); \ for (const auto &R : A.ranges()) \ - EXPECT_EQ(Available, isAvailable(AST, R)) << decorate(A.code(), R); \ + EXPECT_EQ(Available, isAvailable(AST, R, {})) << decorate(A.code(), R); \ } while (0) #define EXPECT_AVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, true) #define EXPECT_UNAVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, false) diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp @@ -8,6 +8,7 @@ #include "TweakTesting.h" +#include "Protocol.h" #include "SourceCode.h" #include "TestTU.h" #include "refactor/Tweak.h" @@ -16,6 +17,7 @@ #include "gtest/gtest.h" #include #include +#include namespace clang { namespace clangd { @@ -63,12 +65,14 @@ // Returns std::nullopt if and only if prepare() failed. std::optional> applyTweak(ParsedAST &AST, llvm::Annotations::Range Range, StringRef TweakID, - const SymbolIndex *Index, llvm::vfs::FileSystem *FS) { + const SymbolIndex *Index, llvm::vfs::FileSystem *FS, + const std::vector &RequestedActionKinds) { std::optional> Result; SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.Begin, Range.End, [&](SelectionTree ST) { Tweak::Selection S(Index, AST, Range.Begin, - Range.End, std::move(ST), FS); + Range.End, std::move(ST), FS, + RequestedActionKinds); if (auto T = prepareTweak(TweakID, S, nullptr)) { Result = (*T)->apply(S); return true; @@ -82,8 +86,10 @@ } // namespace -std::string TweakTest::apply(llvm::StringRef MarkedCode, - llvm::StringMap *EditedFiles) const { +std::string +TweakTest::apply(llvm::StringRef MarkedCode, + llvm::StringMap *EditedFiles, + const std::vector &RequestedActionKinds) const { std::string WrappedCode = wrap(Context, MarkedCode); llvm::Annotations Input(WrappedCode); TestTU TU; @@ -96,7 +102,8 @@ auto Result = applyTweak( AST, rangeOrPoint(Input), TweakID, Index.get(), - &AST.getSourceManager().getFileManager().getVirtualFileSystem()); + &AST.getSourceManager().getFileManager().getVirtualFileSystem(), + RequestedActionKinds); if (!Result) return "unavailable"; if (!*Result) @@ -126,14 +133,16 @@ return EditedMainFile; } -bool TweakTest::isAvailable(WrappedAST &AST, - llvm::Annotations::Range Range) const { +bool TweakTest::isAvailable( + WrappedAST &AST, llvm::Annotations::Range Range, + const std::vector &RequestedActionKinds) const { // Adjust range for wrapping offset. Range.Begin += AST.second; Range.End += AST.second; auto Result = applyTweak( AST.first, Range, TweakID, Index.get(), - &AST.first.getSourceManager().getFileManager().getVirtualFileSystem()); + &AST.first.getSourceManager().getFileManager().getVirtualFileSystem(), + RequestedActionKinds); // We only care if prepare() succeeded, but must handle Errors. if (Result && !*Result) consumeError(Result->takeError());