qchateau updated this revision to Diff 464492.
qchateau added a comment.
- clangd: rename HeaderInsertion and IncludeInsertion, read value from Config
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134384/new/
https://reviews.llvm.org/D134384
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -218,6 +218,20 @@
EXPECT_THAT(Results[0].Completion.AllScopes, testing::Eq(llvm::None));
}
+TEST(ParseYAML, InsertIncludes) {
+ CapturedDiags Diags;
+ Annotations YAML(R"yaml(
+Completion:
+ InsertIncludes: Never
+ )yaml");
+ auto Results =
+ Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+ ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+ ASSERT_EQ(Results.size(), 1u);
+ EXPECT_THAT(Results[0].Completion.InsertIncludes,
+ llvm::ValueIs(val("Never")));
+}
+
TEST(ParseYAML, ShowAKA) {
CapturedDiags Diags;
Annotations YAML(R"yaml(
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "CodeComplete.h"
#include "Config.h"
#include "ConfigFragment.h"
#include "ConfigTesting.h"
@@ -537,6 +538,23 @@
EXPECT_TRUE(Conf.Completion.AllScopes);
}
+TEST_F(ConfigCompileTests, InsertIncludes) {
+ // Defaults to IWYU.
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Completion.InsertIncludes, Config::InsertIncludesPolicy::IWYU);
+
+ Frag = {};
+ Frag.Completion.InsertIncludes = std::string("IWYU");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Completion.InsertIncludes, Config::InsertIncludesPolicy::IWYU);
+
+ Frag = {};
+ Frag.Completion.InsertIncludes = std::string("Never");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Completion.InsertIncludes,
+ Config::InsertIncludesPolicy::NeverInsert);
+}
+
TEST_F(ConfigCompileTests, Style) {
Frag = {};
Frag.Style.FullyQualifiedNamespaces.push_back(std::string("foo"));
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
#include "ClangdServer.h"
#include "CodeComplete.h"
#include "Compiler.h"
+#include "Config.h"
#include "Matchers.h"
#include "Protocol.h"
#include "Quality.h"
@@ -742,7 +743,7 @@
EXPECT_TRUE(Results.HasMore);
}
-TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
+TEST(CompletionTest, InsertIncludesPolicyPreprocessorIntegrationTests) {
TestTU TU;
TU.ExtraArgs.push_back("-I" + testPath("sub"));
TU.AdditionalFiles["sub/bar.h"] = "";
@@ -757,12 +758,15 @@
auto Results = completions(TU, Test.point(), {Sym});
EXPECT_THAT(Results.Completions,
ElementsAre(AllOf(named("X"), insertInclude("\"bar.h\""))));
- // Can be disabled via option.
- CodeCompleteOptions NoInsertion;
- NoInsertion.InsertIncludes = CodeCompleteOptions::NeverInsert;
- Results = completions(TU, Test.point(), {Sym}, NoInsertion);
- EXPECT_THAT(Results.Completions,
- ElementsAre(AllOf(named("X"), Not(insertInclude()))));
+ // Can be disabled via config.
+ {
+ Config Cfg;
+ Cfg.Completion.InsertIncludes = Config::InsertIncludesPolicy::NeverInsert;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+ Results = completions(TU, Test.point(), {Sym});
+ EXPECT_THAT(Results.Completions,
+ ElementsAre(AllOf(named("X"), Not(insertInclude()))));
+ }
// Duplicate based on inclusions in preamble.
Test = Annotations(R"cpp(
#include "sub/bar.h" // not shortest, so should only match resolved.
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -249,19 +249,19 @@
init(CodeCompleteOptions().EnableFunctionArgSnippets),
};
-opt<CodeCompleteOptions::IncludeInsertion> HeaderInsertion{
+opt<Config::InsertIncludesPolicy> HeaderInsertion{
"header-insertion",
cat(Features),
desc("Add #include directives when accepting code completions"),
- init(CodeCompleteOptions().InsertIncludes),
+ init(Config().Completion.InsertIncludes),
values(
- clEnumValN(CodeCompleteOptions::IWYU, "iwyu",
+ clEnumValN(Config::InsertIncludesPolicy::IWYU, "iwyu",
"Include what you use. "
"Insert the owning header for top-level symbols, unless the "
"header is already directly included or the symbol is "
"forward-declared"),
clEnumValN(
- CodeCompleteOptions::NeverInsert, "never",
+ Config::InsertIncludesPolicy::NeverInsert, "never",
"Never insert #include directives as part of code completion")),
};
@@ -697,6 +697,8 @@
C.Index.Background = *BGPolicy;
if (AllScopesCompletion.getNumOccurrences())
C.Completion.AllScopes = AllScopesCompletion;
+ if (HeaderInsertion.getNumOccurrences())
+ C.Completion.InsertIncludes = HeaderInsertion;
return true;
};
}
@@ -904,7 +906,6 @@
if (CompletionStyle.getNumOccurrences())
Opts.CodeComplete.BundleOverloads = CompletionStyle != Detailed;
Opts.CodeComplete.ShowOrigins = ShowOrigins;
- Opts.CodeComplete.InsertIncludes = HeaderInsertion;
if (!HeaderInsertionDecorators) {
Opts.CodeComplete.IncludeIndicator.Insert.clear();
Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -218,6 +218,10 @@
if (auto AllScopes = boolValue(N, "AllScopes"))
F.AllScopes = *AllScopes;
});
+ Dict.handle("InsertIncludes", [&](Node &N) {
+ if (auto InsertIncludes = scalarValue(N, "InsertIncludes"))
+ F.InsertIncludes = *InsertIncludes;
+ });
Dict.parse(N);
}
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -284,6 +284,12 @@
/// Whether code completion should include suggestions from scopes that are
/// not visible. The required scope prefix will be inserted.
llvm::Optional<Located<bool>> AllScopes;
+ /// Add #include directives when accepting code completions.
+ ///
+ /// Valid values are:
+ /// - Never
+ /// - IWYU
+ llvm::Optional<Located<std::string>> InsertIncludes;
};
CompletionBlock Completion;
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -558,6 +558,17 @@
C.Completion.AllScopes = AllScopes;
});
}
+ if (F.InsertIncludes) {
+ if (auto Val =
+ compileEnum<Config::InsertIncludesPolicy>("InsertIncludes",
+ **F.InsertIncludes)
+ .map("Never", Config::InsertIncludesPolicy::NeverInsert)
+ .map("IWYU", Config::InsertIncludesPolicy::IWYU)
+ .value())
+ Out.Apply.push_back([Val](const Params &, Config &C) {
+ C.Completion.InsertIncludes = *Val;
+ });
+ }
}
void compile(Fragment::HoverBlock &&F) {
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -118,11 +118,18 @@
std::vector<std::string> FullyQualifiedNamespaces;
} Style;
+ enum class InsertIncludesPolicy {
+ IWYU,
+ NeverInsert,
+ };
/// Configures code completion feature.
struct {
/// Whether code completion includes results that are not visible in current
/// scopes.
bool AllScopes = true;
+
+ /// Add #include directives when accepting code completions.
+ InsertIncludesPolicy InsertIncludes = InsertIncludesPolicy::IWYU;
} Completion;
/// Configures hover feature.
Index: clang-tools-extra/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -65,11 +65,6 @@
/// Whether to present doc comments as plain-text or markdown.
MarkupKind DocumentationFormat = MarkupKind::PlainText;
- enum IncludeInsertion {
- IWYU,
- NeverInsert,
- } InsertIncludes = IncludeInsertion::IWYU;
-
/// A visual indicator to prepend to the completion label to indicate whether
/// completion result would trigger an #include insertion or not.
struct IncludeInsertionIndicator {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "FileDistance.h"
#include "FuzzyMatch.h"
@@ -256,7 +257,8 @@
// The best header to include if include insertion is allowed.
llvm::Optional<llvm::StringRef>
headerToInsertIfAllowed(const CodeCompleteOptions &Opts) const {
- if (Opts.InsertIncludes == CodeCompleteOptions::NeverInsert ||
+ if (Config::current().Completion.InsertIncludes ==
+ Config::InsertIncludesPolicy::NeverInsert ||
RankedIncludeHeaders.empty())
return None;
if (SemaResult && SemaResult->Declaration) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
