[clang-tools-extra] bb6d96c - [clangd] Enable modules to contribute tweaks.
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 13 08:50:29 PDT 2021
Author: Kadir Cetinkaya
Date: 2021-04-13T17:45:08+02:00
New Revision: bb6d96ced80f288475cd374c3c7a25ee8cad2bb2
URL: https://github.com/llvm/llvm-project/commit/bb6d96ced80f288475cd374c3c7a25ee8cad2bb2
DIFF: https://github.com/llvm/llvm-project/commit/bb6d96ced80f288475cd374c3c7a25ee8cad2bb2.diff
LOG: [clangd] Enable modules to contribute tweaks.
First patch to enable diagnostic fix generation through modules. The
workflow will look like:
- ASTWorker letting modules know about diagnostics while building AST,
modules can read clang::Diagnostic and mutate clangd::Diagnostic through
that hook.
- Modules can implement and expose tweaks to fix diagnostics or act as
general refactorings.
- Tweak::Selection will contain information about the diagnostic
associated with the codeAction request to enable modules to fail their
diagnostic fixing tweakson prepare if need be.
Differential Revision: https://reviews.llvm.org/D98498
Added:
clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/FeatureModule.h
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 557689774b148..ec69c6a71a346 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -573,8 +573,9 @@ void ClangdServer::enumerateTweaks(
static constexpr trace::Metric TweakAvailable(
"tweak_available", trace::Metric::Counter, "tweak_id");
auto Action = [File = File.str(), Sel, CB = std::move(CB),
- Filter =
- std::move(Filter)](Expected<InputsAndAST> InpAST) mutable {
+ Filter = std::move(Filter),
+ FeatureModules(this->FeatureModules)](
+ Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
auto Selections = tweakSelection(Sel, *InpAST);
@@ -587,7 +588,7 @@ void ClangdServer::enumerateTweaks(
return Filter(T) && !PreparedTweaks.count(T.id());
};
for (const auto &Sel : *Selections) {
- for (auto &T : prepareTweaks(*Sel, DeduplicatingFilter)) {
+ for (auto &T : prepareTweaks(*Sel, DeduplicatingFilter, FeatureModules)) {
Res.push_back({T->id(), T->title(), T->kind()});
PreparedTweaks.insert(T->id());
TweakAvailable.record(1, T->id());
@@ -622,7 +623,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
// 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);
+ auto T = prepareTweak(TweakID, *Selection, FeatureModules);
if (T) {
Effect = (*T)->apply(*Selection);
break;
diff --git a/clang-tools-extra/clangd/FeatureModule.h b/clang-tools-extra/clangd/FeatureModule.h
index 337fa24e94543..13d7b7711829e 100644
--- a/clang-tools-extra/clangd/FeatureModule.h
+++ b/clang-tools-extra/clangd/FeatureModule.h
@@ -25,6 +25,7 @@ class LSPBinder;
class SymbolIndex;
class ThreadsafeFS;
class TUScheduler;
+class Tweak;
/// A FeatureModule contributes a vertical feature to clangd.
///
@@ -91,6 +92,10 @@ class FeatureModule {
/// Called by the server when shutting down, and also by tests.
virtual bool blockUntilIdle(Deadline) { return true; }
+ /// Tweaks implemented by this module. Can be called asynchronously when
+ /// enumerating or applying code actions.
+ virtual void contributeTweaks(std::vector<std::unique_ptr<Tweak>> &Out) {}
+
protected:
/// Accessors for modules to access shared server facilities they depend on.
Facilities &facilities();
diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp
index 34b5b2b544dff..8ccb28f236bd1 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
#include "Tweak.h"
+#include "FeatureModule.h"
#include "SourceCode.h"
#include "index/Index.h"
#include "support/Logger.h"
@@ -20,6 +21,7 @@
#include <functional>
#include <memory>
#include <utility>
+#include <vector>
LLVM_INSTANTIATE_REGISTRY(llvm::Registry<clang::clangd::Tweak>)
@@ -43,6 +45,18 @@ void validateRegistry() {
}
#endif
}
+
+std::vector<std::unique_ptr<Tweak>>
+getAllTweaks(const FeatureModuleSet *Modules) {
+ std::vector<std::unique_ptr<Tweak>> All;
+ for (const auto &E : TweakRegistry::entries())
+ All.emplace_back(E.instantiate());
+ if (Modules) {
+ for (auto &M : *Modules)
+ M.contributeTweaks(All);
+ }
+ return All;
+}
} // namespace
Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
@@ -57,12 +71,12 @@ Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
std::vector<std::unique_ptr<Tweak>>
prepareTweaks(const Tweak::Selection &S,
- llvm::function_ref<bool(const Tweak &)> Filter) {
+ llvm::function_ref<bool(const Tweak &)> Filter,
+ const FeatureModuleSet *Modules) {
validateRegistry();
std::vector<std::unique_ptr<Tweak>> Available;
- for (const auto &E : TweakRegistry::entries()) {
- std::unique_ptr<Tweak> T = E.instantiate();
+ for (auto &T : getAllTweaks(Modules)) {
if (!Filter(*T) || !T->prepare(S))
continue;
Available.push_back(std::move(T));
@@ -74,17 +88,17 @@ prepareTweaks(const Tweak::Selection &S,
return Available;
}
-llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef ID,
- const Tweak::Selection &S) {
- auto It = llvm::find_if(
- TweakRegistry::entries(),
- [ID](const TweakRegistry::entry &E) { return E.getName() == ID; });
- if (It == TweakRegistry::end())
- return error("tweak ID {0} is invalid", ID);
- std::unique_ptr<Tweak> T = It->instantiate();
- if (!T->prepare(S))
- return error("failed to prepare() tweak {0}", ID);
- return std::move(T);
+llvm::Expected<std::unique_ptr<Tweak>>
+prepareTweak(StringRef ID, const Tweak::Selection &S,
+ const FeatureModuleSet *Modules) {
+ for (auto &T : getAllTweaks(Modules)) {
+ if (T->id() != ID)
+ continue;
+ if (!T->prepare(S))
+ return error("failed to prepare() tweak {0}", ID);
+ return std::move(T);
+ }
+ return error("tweak ID {0} is invalid", ID);
}
llvm::Expected<std::pair<Path, Edit>>
diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h
index f991b78d89603..cc8d12ba2cb85 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -35,6 +35,8 @@
namespace clang {
namespace clangd {
+class FeatureModuleSet;
+
/// An interface base for small context-sensitive refactoring actions.
/// To implement a new tweak use the following pattern in a .cpp file:
/// class MyTweak : public Tweak {
@@ -128,13 +130,15 @@ class Tweak {
/// can run on the selection.
std::vector<std::unique_ptr<Tweak>>
prepareTweaks(const Tweak::Selection &S,
- llvm::function_ref<bool(const Tweak &)> Filter);
+ llvm::function_ref<bool(const Tweak &)> Filter,
+ const FeatureModuleSet *Modules);
// Calls prepare() on the tweak with a given ID.
// If prepare() returns false, returns an error.
// If prepare() returns true, returns the corresponding tweak.
-llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef TweakID,
- const Tweak::Selection &S);
+llvm::Expected<std::unique_ptr<Tweak>>
+prepareTweak(StringRef ID, const Tweak::Selection &S,
+ const FeatureModuleSet *Modules);
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index 0138ae3eb13bc..dc17530a2f8f5 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -211,7 +211,8 @@ class Checker {
auto Tree = SelectionTree::createRight(AST->getASTContext(),
AST->getTokens(), Start, End);
Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree));
- for (const auto &T : prepareTweaks(Selection, Opts.TweakFilter)) {
+ for (const auto &T :
+ prepareTweaks(Selection, Opts.TweakFilter, Opts.FeatureModules)) {
auto Result = T->apply(Selection);
if (!Result) {
elog(" tweak: {0} ==> FAIL: {1}", T->id(), Result.takeError());
diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index b16a443b3da3f..4fc0fa7f7a903 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -55,6 +55,7 @@ add_unittest(ClangdUnitTests ClangdTests
DraftStoreTests.cpp
DumpASTTests.cpp
ExpectedTypeTest.cpp
+ FeatureModulesTests.cpp
FileDistanceTests.cpp
FileIndexTests.cpp
FindSymbolsTests.cpp
diff --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
new file mode 100644
index 0000000000000..d73b805499f7f
--- /dev/null
+++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -0,0 +1,55 @@
+//===--- FeatureModulesTests.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 "FeatureModule.h"
+#include "Selection.h"
+#include "TestTU.h"
+#include "refactor/Tweak.h"
+#include "support/Logger.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <memory>
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(FeatureModulesTest, ContributesTweak) {
+ static constexpr const char *TweakID = "ModuleTweak";
+ struct TweakContributingModule final : public FeatureModule {
+ struct ModuleTweak final : public Tweak {
+ const char *id() const override { return TweakID; }
+ bool prepare(const Selection &Sel) override { return true; }
+ Expected<Effect> apply(const Selection &Sel) override {
+ return error("not implemented");
+ }
+ std::string title() const override { return id(); }
+ llvm::StringLiteral kind() const override { return ""; };
+ };
+
+ void contributeTweaks(std::vector<std::unique_ptr<Tweak>> &Out) override {
+ Out.emplace_back(new ModuleTweak);
+ }
+ };
+
+ FeatureModuleSet Set;
+ Set.add(std::make_unique<TweakContributingModule>());
+
+ auto AST = TestTU::withCode("").build();
+ auto Tree =
+ SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0, 0);
+ auto Actual = prepareTweak(
+ TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree)), &Set);
+ ASSERT_TRUE(bool(Actual));
+ EXPECT_EQ(Actual->get()->id(), TweakID);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
index 680280f831494..71bd09e341772 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -75,7 +75,7 @@ applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
Range.second, [&](SelectionTree ST) {
Tweak::Selection S(Index, AST, Range.first,
Range.second, std::move(ST));
- if (auto T = prepareTweak(TweakID, S)) {
+ if (auto T = prepareTweak(TweakID, S, nullptr)) {
Result = (*T)->apply(S);
return true;
} else {
More information about the cfe-commits
mailing list