[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363680: [clangd] Add hidden tweaks to dump AST/selection.
(authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D62538?vs=201765=205322#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62538/new/
https://reviews.llvm.org/D62538
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/Selection.cpp
clang-tools-extra/trunk/clangd/refactor/Tweak.h
clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/trunk/clangd/refactor/tweaks/DumpAST.cpp
clang-tools-extra/trunk/clangd/refactor/tweaks/RawStringLiteral.cpp
clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -483,6 +483,28 @@
};
bool fromJSON(const llvm::json::Value &, InitializeParams &);
+enum class MessageType {
+ /// An error message.
+ Error = 1,
+ /// A warning message.
+ Warning = 1,
+ /// An information message.
+ Info = 1,
+ /// A log message.
+ Log = 1,
+};
+llvm::json::Value toJSON(const MessageType &);
+
+/// The show message notification is sent from a server to a client to ask the
+/// client to display a particular message in the user interface.
+struct ShowMessageParams {
+ /// The message type.
+ MessageType type = MessageType::Info;
+ /// The actual message.
+ std::string message;
+};
+llvm::json::Value toJSON(const ShowMessageParams &);
+
struct DidOpenTextDocumentParams {
/// The document that was opened.
TextDocumentItem textDocument;
@@ -740,6 +762,7 @@
llvm::Optional kind;
const static llvm::StringLiteral QUICKFIX_KIND;
const static llvm::StringLiteral REFACTOR_KIND;
+ const static llvm::StringLiteral INFO_KIND;
/// The diagnostics that this code action resolves.
llvm::Optional> diagnostics;
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -13,6 +13,7 @@
#include "SourceCode.h"
#include "Trace.h"
#include "URI.h"
+#include "refactor/Tweak.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/ScopeExit.h"
@@ -31,7 +32,14 @@
Range Selection) {
CodeAction CA;
CA.title = T.Title;
- CA.kind = CodeAction::REFACTOR_KIND;
+ switch (T.Intent) {
+ case Tweak::Refactor:
+CA.kind = CodeAction::REFACTOR_KIND;
+break;
+ case Tweak::Info:
+CA.kind = CodeAction::INFO_KIND;
+break;
+ }
// This tweak may have an expensive second stage, we only run it if the user
// actually chooses it in the UI. We reply with a command that would run the
// corresponding tweak.
@@ -481,18 +489,25 @@
llvm::inconvertibleErrorCode(),
"trying to apply a code action for a non-added file"));
-auto Action = [ApplyEdit](decltype(Reply) Reply, URIForFile File,
- std::string Code,
- llvm::Expected> R) {
+auto Action = [this, ApplyEdit](decltype(Reply) Reply, URIForFile File,
+std::string Code,
+llvm::Expected R) {
if (!R)
return Reply(R.takeError());
- WorkspaceEdit WE;
- WE.changes.emplace();
- (*WE.changes)[File.uri()] = std::move(*R);
-
- Reply("Fix applied.");
- ApplyEdit(std::move(WE));
+ if (R->ApplyEdit) {
+WorkspaceEdit WE;
+WE.changes.emplace();
+(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
+ApplyEdit(std::move(WE));
+ }
+ if (R->ShowMessage) {
+ShowMessageParams Msg;
+Msg.message = *R->ShowMessage;
+Msg.type = MessageType::Info;
+notify("window/showMessage", Msg);
+ }
+ Reply("Tweak applied.");
};
Server->applyTweak(Params.tweakArgs->file.file(),
Params.tweakArgs->selection, Params.tweakArgs->tweakID,
Index: clang-tools-extra/trunk/clangd/Selection.cpp
===
--- clang-tools-extra/trunk/clangd/Selection.cpp
+++
[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
sammccall marked 2 inline comments as done.
sammccall added inline comments.
Comment at: clangd/refactor/Tweak.h:104
+ /// Is this a 'hidden' tweak, which are off by default.
+ virtual bool hidden() const { return false; }
};
ilya-biryukov wrote:
> I wonder whether this should be a static method. WDYT?
>
> That would allow to even prevent calling `prepare()` on those tweaks.
> OTOH, we want `prepare()` should be fast and it shouldn't matter if that's
> the case.
We'd need to add criteria to prepareTweaks() as hidden() couldn't be checked
polymorphically on the results.
I think this is a good idea but should probably happen if/when we have a more
important reason to touch that API.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62538/new/
https://reviews.llvm.org/D62538
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM with a few NITs
Comment at: clangd/refactor/Tweak.h:104
+ /// Is this a 'hidden' tweak, which are off by default.
+ virtual bool hidden() const { return false; }
};
I wonder whether this should be a static method. WDYT?
That would allow to even prevent calling `prepare()` on those tweaks.
OTOH, we want `prepare()` should be fast and it shouldn't matter if that's the
case.
Comment at: clangd/unittests/TweakTests.cpp:19
#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock-matchers.h"
#include "gmock/gmock.h"
NIT: the include is redundant. Maybe remove? (probably added by clangd)
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62538/new/
https://reviews.llvm.org/D62538
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
sammccall added a comment. i mean ping Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62538/new/ https://reviews.llvm.org/D62538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
sammccall added a comment. pind Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62538/new/ https://reviews.llvm.org/D62538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
sammccall updated this revision to Diff 201765.
sammccall added a comment.
Add DumpRecordLayout. Fix a small SelectionTree bug uncovered by these tweaks.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62538/new/
https://reviews.llvm.org/D62538
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/Selection.cpp
clangd/refactor/Tweak.h
clangd/refactor/tweaks/CMakeLists.txt
clangd/refactor/tweaks/DumpAST.cpp
clangd/refactor/tweaks/RawStringLiteral.cpp
clangd/refactor/tweaks/SwapIfBranches.cpp
clangd/tool/ClangdMain.cpp
clangd/unittests/SelectionTests.cpp
clangd/unittests/TweakTests.cpp
Index: clangd/unittests/TweakTests.cpp
===
--- clangd/unittests/TweakTests.cpp
+++ clangd/unittests/TweakTests.cpp
@@ -16,12 +16,12 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock-matchers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include
using llvm::Failed;
-using llvm::HasValue;
using llvm::Succeeded;
namespace clang {
@@ -76,7 +76,8 @@
void checkNotAvailable(StringRef ID, llvm::StringRef Input) {
return checkAvailable(ID, Input, /*Available=*/false);
}
-llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
+
+llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
Annotations Code(Input);
Range SelectionRng;
if (Code.points().size() != 0) {
@@ -98,15 +99,30 @@
auto T = prepareTweak(ID, S);
if (!T)
return T.takeError();
- auto Replacements = (*T)->apply(S);
- if (!Replacements)
-return Replacements.takeError();
- return applyAllReplacements(Code.code(), *Replacements);
+ return (*T)->apply(S);
+}
+
+llvm::Expected applyEdit(StringRef ID, llvm::StringRef Input) {
+ auto Effect = apply(ID, Input);
+ if (!Effect)
+return Effect.takeError();
+ if (!Effect->ApplyEdit)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "No replacements");
+ Annotations Code(Input);
+ return applyAllReplacements(Code.code(), *Effect->ApplyEdit);
+}
+
+std::string getMessage(StringRef ID, llvm::StringRef Input) {
+ auto Effect = apply(ID, Input);
+ if (!Effect)
+return "error: " + llvm::toString(Effect.takeError());
+ return Effect->ShowMessage.getValueOr("no message produced!");
}
void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
std::string Output) {
- auto Result = apply(ID, Input);
+ auto Result = applyEdit(ID, Input);
ASSERT_TRUE(bool(Result)) << llvm::toString(Result.takeError()) << Input;
EXPECT_EQ(Output, std::string(*Result)) << Input;
}
@@ -217,6 +233,49 @@
checkTransform(ID, Input, Output);
}
+TEST(TweakTest, DumpAST) {
+ llvm::StringLiteral ID = "DumpAST";
+
+ checkAvailable(ID, "^int f^oo() { re^turn 2 ^+ 2; }");
+ checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }");
+
+ const char *Input = "int x = 2 ^+ 2;";
+ const char *Output = R"(BinaryOperator.*'\+'.*
+.*IntegerLiteral.*'int' 2.*
+.*IntegerLiteral.*'int' 2.*)";
+ EXPECT_THAT(getMessage(ID, Input), ::testing::MatchesRegex(Output));
+}
+
+TEST(TweakTest, ShowSelectionTree) {
+ llvm::StringLiteral ID = "ShowSelectionTree";
+
+ checkAvailable(ID, "^int f^oo() { re^turn 2 ^+ 2; }");
+ checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }");
+
+ const char *Input = "int fcall(int); int x = fca[[ll(2 +]]2);";
+ const char *Output = R"(TranslationUnitDecl
+ VarDecl int x = fcall(2 + 2)
+ .CallExpr fcall(2 + 2)
+ ImplicitCastExpr fcall
+ .DeclRefExpr fcall
+ .BinaryOperator 2 + 2
+ *IntegerLiteral 2
+)";
+ EXPECT_EQ(Output, getMessage(ID, Input));
+}
+
+TEST(TweakTest, DumpRecordLayout) {
+ llvm::StringLiteral ID = "DumpRecordLayout";
+ checkAvailable(ID, "^s^truct ^X ^{ int x; ^};");
+ checkNotAvailable(ID, "struct X { int ^a; };");
+ checkNotAvailable(ID, "struct ^X;");
+ checkNotAvailable(ID, "template struct ^X { T t; };");
+ checkNotAvailable(ID, "enum ^X {};");
+
+ const char *Input = "struct ^X { int x; int y; }";
+ EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 | int x"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/unittests/SelectionTests.cpp
===
--- clangd/unittests/SelectionTests.cpp
+++ clangd/unittests/SelectionTests.cpp
@@ -216,6 +216,16 @@
}
}
+// Regression test: this used to match the injected X, not the outer X.
+TEST(SelectionTest, InjectedClassName) {
+ const char* Code = "struct ^X { int x; };";
+ auto AST = TestTU::withCode(Annotations(Code).code()).build();
+ auto T = makeSelectionTree(Code, AST);
+ ASSERT_EQ("CXXRecordDecl", nodeKind(T.commonAncestor())) << T;
+ auto *D =
[PATCH] D62538: [clangd] Add hidden tweaks to dump AST/selection.
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
mgorny.
Herald added a project: clang.
This introduces a few new concepts:
- tweaks have an Intent (they don't all advertise as refactorings)
- tweaks may produce messages (for ShowMessage notification). Generalized
Replacements -> Effect.
- tweaks (and other features) may be hidden (clangd -hidden-features flag). We
may choose to promote these one day. I'm not sure they're worth their own
feature flags though.
Verified it in vim-clangd (not yet open source), curious if the UI is ok in
VSCode.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D62538
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/refactor/Tweak.h
clangd/refactor/tweaks/CMakeLists.txt
clangd/refactor/tweaks/DumpAST.cpp
clangd/refactor/tweaks/RawStringLiteral.cpp
clangd/refactor/tweaks/SwapIfBranches.cpp
clangd/tool/ClangdMain.cpp
clangd/unittests/TweakTests.cpp
Index: clangd/unittests/TweakTests.cpp
===
--- clangd/unittests/TweakTests.cpp
+++ clangd/unittests/TweakTests.cpp
@@ -16,12 +16,12 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock-matchers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include
using llvm::Failed;
-using llvm::HasValue;
using llvm::Succeeded;
namespace clang {
@@ -76,7 +76,8 @@
void checkNotAvailable(StringRef ID, llvm::StringRef Input) {
return checkAvailable(ID, Input, /*Available=*/false);
}
-llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
+
+llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
Annotations Code(Input);
Range SelectionRng;
if (Code.points().size() != 0) {
@@ -98,15 +99,30 @@
auto T = prepareTweak(ID, S);
if (!T)
return T.takeError();
- auto Replacements = (*T)->apply(S);
- if (!Replacements)
-return Replacements.takeError();
- return applyAllReplacements(Code.code(), *Replacements);
+ return (*T)->apply(S);
+}
+
+llvm::Expected applyEdit(StringRef ID, llvm::StringRef Input) {
+ auto Effect = apply(ID, Input);
+ if (!Effect)
+return Effect.takeError();
+ if (!Effect->ApplyEdit)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "No replacements");
+ Annotations Code(Input);
+ return applyAllReplacements(Code.code(), *Effect->ApplyEdit);
+}
+
+std::string getMessage(StringRef ID, llvm::StringRef Input) {
+ auto Effect = apply(ID, Input);
+ if (!Effect)
+return "error: " + llvm::toString(Effect.takeError());
+ return Effect->ShowMessage.getValueOr("no message produced!");
}
void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
std::string Output) {
- auto Result = apply(ID, Input);
+ auto Result = applyEdit(ID, Input);
ASSERT_TRUE(bool(Result)) << llvm::toString(Result.takeError()) << Input;
EXPECT_EQ(Output, std::string(*Result)) << Input;
}
@@ -217,6 +233,37 @@
checkTransform(ID, Input, Output);
}
+TEST(TweakTest, DumpAST) {
+ llvm::StringLiteral ID = "DumpAST";
+
+ checkAvailable(ID, "^int f^oo() { re^turn 2 ^+ 2; }");
+ checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }");
+
+ const char *Input = "int x = 2 ^+ 2;";
+ const char *Output = R"(BinaryOperator.*'\+'.*
+.*IntegerLiteral.*'int' 2.*
+.*IntegerLiteral.*'int' 2.*)";
+ EXPECT_THAT(getMessage(ID, Input), ::testing::MatchesRegex(Output));
+}
+
+TEST(TweakTest, ShowSelectionTree) {
+ llvm::StringLiteral ID = "ShowSelectionTree";
+
+ checkAvailable(ID, "^int f^oo() { re^turn 2 ^+ 2; }");
+ checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }");
+
+ const char *Input = "int fcall(int); int x = fca[[ll(2 +]]2);";
+ const char *Output = R"(TranslationUnitDecl
+ VarDecl int x = fcall(2 + 2)
+ .CallExpr fcall(2 + 2)
+ ImplicitCastExpr fcall
+ .DeclRefExpr fcall
+ .BinaryOperator 2 + 2
+ *IntegerLiteral 2
+)";
+ EXPECT_EQ(Output, getMessage(ID, Input));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -268,6 +268,11 @@
"Always used text-based completion")),
llvm::cl::init(CodeCompleteOptions().RunParser), llvm::cl::Hidden);
+static llvm::cl::opt HiddenFeatures(
+"hidden-features",
+llvm::cl::desc("Enable hidden features mostly useful to clangd developers"),
+llvm::cl::init(false), llvm::cl::Hidden);
+
namespace {
/// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -465,6 +470,7 @@
}
Opts.StaticIndex =
