Skip to content

Commit 16fd5c2

Browse files
committed
[clangd] Support configuration of inlay hints.
The idea is that the feature will always be advertised at the LSP level, but depending on config we'll return partial or no responses. We try to avoid doing the analysis for hints we're not going to return. Examples of syntax: ``` InlayHints: Enabled: No --- InlayHints: ParameterNames: No --- InlayHints: ParameterNames: Yes DeducedTypes: Yes ``` Differential Revision: https://reviews.llvm.org/D116713
1 parent 27ea0c4 commit 16fd5c2

File tree

7 files changed

+132
-18
lines changed

7 files changed

+132
-18
lines changed

clang-tools-extra/clangd/Config.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ struct Config {
122122
/// Whether hover show a.k.a type.
123123
bool ShowAKA = false;
124124
} Hover;
125+
126+
struct {
127+
/// If false, inlay hints are completely disabled.
128+
bool Enabled = true;
129+
130+
// Whether specific categories of hints are enabled.
131+
bool Parameters = true;
132+
bool DeducedTypes = true;
133+
} InlayHints;
125134
};
126135

127136
} // namespace clangd

clang-tools-extra/clangd/ConfigCompile.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ struct FragmentCompiler {
197197
compile(std::move(F.Diagnostics));
198198
compile(std::move(F.Completion));
199199
compile(std::move(F.Hover));
200+
compile(std::move(F.InlayHints));
200201
}
201202

202203
void compile(Fragment::IfBlock &&F) {
@@ -526,6 +527,22 @@ struct FragmentCompiler {
526527
}
527528
}
528529

530+
void compile(Fragment::InlayHintsBlock &&F) {
531+
if (F.Enabled)
532+
Out.Apply.push_back([Value(**F.Enabled)](const Params &, Config &C) {
533+
C.InlayHints.Enabled = Value;
534+
});
535+
if (F.ParameterNames)
536+
Out.Apply.push_back(
537+
[Value(**F.ParameterNames)](const Params &, Config &C) {
538+
C.InlayHints.Parameters = Value;
539+
});
540+
if (F.DeducedTypes)
541+
Out.Apply.push_back([Value(**F.DeducedTypes)](const Params &, Config &C) {
542+
C.InlayHints.DeducedTypes = Value;
543+
});
544+
}
545+
529546
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
530547
constexpr static llvm::SourceMgr::DiagKind Warning =
531548
llvm::SourceMgr::DK_Warning;

clang-tools-extra/clangd/ConfigFragment.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,18 @@ struct Fragment {
283283
llvm::Optional<Located<bool>> ShowAKA;
284284
};
285285
HoverBlock Hover;
286+
287+
/// Configures labels shown inline with the code.
288+
struct InlayHintsBlock {
289+
/// Enables/disables the inlay-hints feature.
290+
llvm::Optional<Located<bool>> Enabled;
291+
292+
/// Show parameter names before function arguments.
293+
llvm::Optional<Located<bool>> ParameterNames;
294+
/// Show deduced types for `auto`.
295+
llvm::Optional<Located<bool>> DeducedTypes;
296+
};
297+
InlayHintsBlock InlayHints;
286298
};
287299

288300
} // namespace config

clang-tools-extra/clangd/ConfigYAML.cpp

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class Parser {
6666
Dict.handle("Diagnostics", [&](Node &N) { parse(F.Diagnostics, N); });
6767
Dict.handle("Completion", [&](Node &N) { parse(F.Completion, N); });
6868
Dict.handle("Hover", [&](Node &N) { parse(F.Hover, N); });
69+
Dict.handle("InlayHints", [&](Node &N) { parse(F.InlayHints, N); });
6970
Dict.parse(N);
7071
return !(N.failed() || HadError);
7172
}
@@ -199,25 +200,34 @@ class Parser {
199200
void parse(Fragment::CompletionBlock &F, Node &N) {
200201
DictParser Dict("Completion", this);
201202
Dict.handle("AllScopes", [&](Node &N) {
202-
if (auto Value = scalarValue(N, "AllScopes")) {
203-
if (auto AllScopes = llvm::yaml::parseBool(**Value))
204-
F.AllScopes = *AllScopes;
205-
else
206-
warning("AllScopes should be a boolean", N);
207-
}
203+
if (auto AllScopes = boolValue(N, "AllScopes"))
204+
F.AllScopes = *AllScopes;
208205
});
209206
Dict.parse(N);
210207
}
211208

212209
void parse(Fragment::HoverBlock &F, Node &N) {
213210
DictParser Dict("Hover", this);
214211
Dict.handle("ShowAKA", [&](Node &N) {
215-
if (auto Value = scalarValue(N, "ShowAKA")) {
216-
if (auto ShowAKA = llvm::yaml::parseBool(**Value))
217-
F.ShowAKA = *ShowAKA;
218-
else
219-
warning("ShowAKA should be a boolean", N);
220-
}
212+
if (auto ShowAKA = boolValue(N, "ShowAKA"))
213+
F.ShowAKA = *ShowAKA;
214+
});
215+
Dict.parse(N);
216+
}
217+
218+
void parse(Fragment::InlayHintsBlock &F, Node &N) {
219+
DictParser Dict("InlayHints", this);
220+
Dict.handle("Enabled", [&](Node &N) {
221+
if (auto Value = boolValue(N, "Enabled"))
222+
F.Enabled = *Value;
223+
});
224+
Dict.handle("ParameterNames", [&](Node &N) {
225+
if (auto Value = boolValue(N, "ParameterNames"))
226+
F.ParameterNames = *Value;
227+
});
228+
Dict.handle("DeducedTypes", [&](Node &N) {
229+
if (auto Value = boolValue(N, "DeducedTypes"))
230+
F.DeducedTypes = *Value;
221231
});
222232
Dict.parse(N);
223233
}
@@ -331,6 +341,16 @@ class Parser {
331341
return llvm::None;
332342
}
333343

344+
llvm::Optional<Located<bool>> boolValue(Node &N, llvm::StringRef Desc) {
345+
if (auto Scalar = scalarValue(N, Desc)) {
346+
if (auto Bool = llvm::yaml::parseBool(**Scalar))
347+
return Located<bool>(*Bool, Scalar->Range);
348+
else
349+
warning(Desc + " should be a boolean", N);
350+
}
351+
return llvm::None;
352+
}
353+
334354
// Try to parse a list of single scalar values, or just a single value.
335355
llvm::Optional<std::vector<Located<std::string>>> scalarValues(Node &N) {
336356
std::vector<Located<std::string>> Result;

clang-tools-extra/clangd/InlayHints.cpp

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//
77
//===----------------------------------------------------------------------===//
88
#include "InlayHints.h"
9+
#include "Config.h"
910
#include "HeuristicResolver.h"
1011
#include "ParsedAST.h"
1112
#include "clang/AST/DeclarationName.h"
@@ -23,8 +24,8 @@ enum class HintSide { Left, Right };
2324
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
2425
public:
2526
InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
26-
llvm::Optional<Range> RestrictRange)
27-
: Results(Results), AST(AST.getASTContext()),
27+
const Config &Cfg, llvm::Optional<Range> RestrictRange)
28+
: Results(Results), AST(AST.getASTContext()), Cfg(Cfg),
2829
RestrictRange(std::move(RestrictRange)),
2930
MainFileID(AST.getSourceManager().getMainFileID()),
3031
Resolver(AST.getHeuristicResolver()),
@@ -65,6 +66,9 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
6566
}
6667

6768
bool VisitCallExpr(CallExpr *E) {
69+
if (!Cfg.InlayHints.Parameters)
70+
return true;
71+
6872
// Do not show parameter hints for operator calls written using operator
6973
// syntax or user-defined literals. (Among other reasons, the resulting
7074
// hints can look awkard, e.g. the expression can itself be a function
@@ -135,7 +139,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
135139
// the entire argument list likely appears in the main file and can be hinted.
136140
void processCall(SourceLocation Anchor, const FunctionDecl *Callee,
137141
llvm::ArrayRef<const Expr *const> Args) {
138-
if (Args.size() == 0 || !Callee)
142+
if (!Cfg.InlayHints.Parameters || Args.size() == 0 || !Callee)
139143
return;
140144

141145
// If the anchor location comes from a macro defintion, there's nowhere to
@@ -323,6 +327,23 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
323327
void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
324328
llvm::StringRef Prefix, llvm::StringRef Label,
325329
llvm::StringRef Suffix) {
330+
// We shouldn't get as far as adding a hint if the category is disabled.
331+
// We'd like to disable as much of the analysis as possible above instead.
332+
// Assert in debug mode but add a dynamic check in production.
333+
assert(Cfg.InlayHints.Enabled && "Shouldn't get here if disabled!");
334+
switch (Kind) {
335+
#define CHECK_KIND(Enumerator, ConfigProperty) \
336+
case InlayHintKind::Enumerator: \
337+
assert(Cfg.InlayHints.ConfigProperty && \
338+
"Shouldn't get here if kind is disabled!"); \
339+
if (!Cfg.InlayHints.ConfigProperty) \
340+
return; \
341+
break
342+
CHECK_KIND(ParameterHint, Parameters);
343+
CHECK_KIND(TypeHint, DeducedTypes);
344+
#undef CHECK_KIND
345+
}
346+
326347
auto FileRange =
327348
toHalfOpenFileRange(AST.getSourceManager(), AST.getLangOpts(), R);
328349
if (!FileRange)
@@ -348,8 +369,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
348369

349370
void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix,
350371
const PrintingPolicy &Policy) {
351-
// Do not print useless "NULL TYPE" hint.
352-
if (!T.getTypePtrOrNull())
372+
if (!Cfg.InlayHints.DeducedTypes || T.isNull())
353373
return;
354374

355375
std::string TypeName = T.getAsString(Policy);
@@ -360,6 +380,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
360380

361381
std::vector<InlayHint> &Results;
362382
ASTContext &AST;
383+
const Config &Cfg;
363384
llvm::Optional<Range> RestrictRange;
364385
FileID MainFileID;
365386
StringRef MainFileBuf;
@@ -381,7 +402,10 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
381402
std::vector<InlayHint> inlayHints(ParsedAST &AST,
382403
llvm::Optional<Range> RestrictRange) {
383404
std::vector<InlayHint> Results;
384-
InlayHintVisitor Visitor(Results, AST, std::move(RestrictRange));
405+
const auto &Cfg = Config::current();
406+
if (!Cfg.InlayHints.Enabled)
407+
return Results;
408+
InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange));
385409
Visitor.TraverseAST(AST.getASTContext());
386410

387411
// De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit

clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,23 @@ TEST(ParseYAML, ShowAKA) {
228228
ASSERT_EQ(Results.size(), 1u);
229229
EXPECT_THAT(Results[0].Hover.ShowAKA, llvm::ValueIs(Val(true)));
230230
}
231+
232+
TEST(ParseYAML, InlayHints) {
233+
CapturedDiags Diags;
234+
Annotations YAML(R"yaml(
235+
InlayHints:
236+
Enabled: No
237+
ParameterNames: Yes
238+
)yaml");
239+
auto Results =
240+
Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
241+
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
242+
ASSERT_EQ(Results.size(), 1u);
243+
EXPECT_THAT(Results[0].InlayHints.Enabled, llvm::ValueIs(Val(false)));
244+
EXPECT_THAT(Results[0].InlayHints.ParameterNames, llvm::ValueIs(Val(true)));
245+
EXPECT_EQ(Results[0].InlayHints.DeducedTypes, llvm::None);
246+
}
247+
231248
} // namespace
232249
} // namespace config
233250
} // namespace clangd

clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
//
77
//===----------------------------------------------------------------------===//
88
#include "Annotations.h"
9+
#include "Config.h"
910
#include "InlayHints.h"
1011
#include "Protocol.h"
1112
#include "TestTU.h"
1213
#include "TestWorkspace.h"
1314
#include "XRefs.h"
15+
#include "support/Context.h"
1416
#include "gmock/gmock.h"
1517
#include "gtest/gtest.h"
1618

@@ -24,6 +26,8 @@ std::ostream &operator<<(std::ostream &Stream, const InlayHint &Hint) {
2426
namespace {
2527

2628
using ::testing::ElementsAre;
29+
using ::testing::IsEmpty;
30+
using ::testing::UnorderedElementsAre;
2731

2832
std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
2933
std::vector<InlayHint> Result;
@@ -56,6 +60,13 @@ MATCHER_P2(HintMatcher, Expected, Code, "") {
5660

5761
MATCHER_P(labelIs, Label, "") { return arg.label == Label; }
5862

63+
Config noHintsConfig() {
64+
Config C;
65+
C.InlayHints.Parameters = false;
66+
C.InlayHints.DeducedTypes = false;
67+
return C;
68+
}
69+
5970
template <typename... ExpectedHints>
6071
void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
6172
ExpectedHints... Expected) {
@@ -66,6 +77,10 @@ void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
6677

6778
EXPECT_THAT(hintsOfKind(AST, Kind),
6879
ElementsAre(HintMatcher(Expected, Source)...));
80+
// Sneak in a cross-cutting check that hints are disabled by config.
81+
// We'll hit an assertion failure if addInlayHint still gets called.
82+
WithContextValue WithCfg(Config::Key, noHintsConfig());
83+
EXPECT_THAT(inlayHints(AST, llvm::None), IsEmpty());
6984
}
7085

7186
// Hack to allow expression-statements operating on parameter packs in C++14.

0 commit comments

Comments
 (0)