forked from OSchip/llvm-project
[clangd] Add fixes for clang "include <foo.h>" diagnostics
Clang doesn't offer these fixes I guess for a couple of reasons: - where to insert includes is a formatting concern, and clang shouldn't depend on clang-format - the way clang prints diagnostics, we'd show a bunch of basically irrelevant context of "this is where we'd want to insert the include" Maybe it's possible to hack around 1, but 2 is still a concern. Meanwhile, bolting this onto include-fixer gets the job done. Fixes https://github.com/clangd/clangd/issues/355 Fixes https://github.com/clangd/clangd/issues/937 Differential Revision: https://reviews.llvm.org/D114667
This commit is contained in:
Sam McCall
parent
b73cf6207e
commit
2676759bf2
|
|
@ -823,6 +823,18 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
|
|||
if (LastDiag->Severity == DiagnosticsEngine::Ignored)
|
||||
return;
|
||||
|
||||
// Give include-fixer a chance to replace a note with a fix.
|
||||
if (Fixer) {
|
||||
auto ReplacementFixes = Fixer(LastDiag->Severity, Info);
|
||||
if (!ReplacementFixes.empty()) {
|
||||
assert(Info.getNumFixItHints() == 0 &&
|
||||
"Include-fixer replaced a note with clang fix-its attached!");
|
||||
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ReplacementFixes.begin(),
|
||||
ReplacementFixes.end());
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (!Info.getFixItHints().empty()) {
|
||||
// A clang note with fix-it is not a separate diagnostic in clangd. We
|
||||
// attach it as a Fix to the main diagnostic instead.
|
||||
|
|
|
|||
|
|
@ -144,6 +144,8 @@ public:
|
|||
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
|
||||
const clang::Diagnostic &Info) override;
|
||||
|
||||
/// When passed a main diagnostic, returns fixes to add to it.
|
||||
/// When passed a note diagnostic, returns fixes to replace it with.
|
||||
using DiagFixer = std::function<std::vector<Fix>(DiagnosticsEngine::Level,
|
||||
const clang::Diagnostic &)>;
|
||||
using LevelAdjuster = std::function<DiagnosticsEngine::Level(
|
||||
|
|
|
|||
|
|
@ -31,7 +31,6 @@
|
|||
#include "clang/Sema/Scope.h"
|
||||
#include "clang/Sema/Sema.h"
|
||||
#include "clang/Sema/TypoCorrection.h"
|
||||
#include "llvm/ADT/ArrayRef.h"
|
||||
#include "llvm/ADT/DenseMap.h"
|
||||
#include "llvm/ADT/None.h"
|
||||
#include "llvm/ADT/Optional.h"
|
||||
|
|
@ -47,6 +46,27 @@
|
|||
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
namespace {
|
||||
|
||||
llvm::Optional<llvm::StringRef> getArgStr(const clang::Diagnostic &Info,
|
||||
unsigned Index) {
|
||||
switch (Info.getArgKind(Index)) {
|
||||
case DiagnosticsEngine::ak_c_string:
|
||||
return llvm::StringRef(Info.getArgCStr(Index));
|
||||
case DiagnosticsEngine::ak_std_string:
|
||||
return llvm::StringRef(Info.getArgStdStr(Index));
|
||||
default:
|
||||
return llvm::None;
|
||||
}
|
||||
}
|
||||
|
||||
std::vector<Fix> only(llvm::Optional<Fix> F) {
|
||||
if (F)
|
||||
return {std::move(*F)};
|
||||
return {};
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
|
||||
const clang::Diagnostic &Info) const {
|
||||
|
|
@ -102,10 +122,53 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
|
|||
LastUnresolvedName->Loc == Info.getLocation())
|
||||
return fixUnresolvedName();
|
||||
}
|
||||
break;
|
||||
// Cases where clang explicitly knows which header to include.
|
||||
// (There's no fix provided for boring formatting reasons).
|
||||
case diag::err_implied_std_initializer_list_not_found:
|
||||
return only(insertHeader("<initializer_list>"));
|
||||
case diag::err_need_header_before_typeid:
|
||||
return only(insertHeader("<typeid>"));
|
||||
case diag::err_need_header_before_ms_uuidof:
|
||||
return only(insertHeader("<guiddef.h>"));
|
||||
case diag::err_need_header_before_placement_new:
|
||||
case diag::err_implicit_coroutine_std_nothrow_type_not_found:
|
||||
return only(insertHeader("<new>"));
|
||||
case diag::err_omp_implied_type_not_found:
|
||||
case diag::err_omp_interop_type_not_found:
|
||||
return only(insertHeader("<omp.h>"));
|
||||
case diag::err_implied_coroutine_type_not_found:
|
||||
return only(insertHeader("<coroutine>"));
|
||||
case diag::err_implied_comparison_category_type_not_found:
|
||||
return only(insertHeader("<compare>"));
|
||||
case diag::note_include_header_or_declare:
|
||||
if (Info.getNumArgs() > 0)
|
||||
if (auto Header = getArgStr(Info, 0))
|
||||
return only(insertHeader(("<" + *Header + ">").str(),
|
||||
getArgStr(Info, 1).getValueOr("")));
|
||||
break;
|
||||
}
|
||||
|
||||
return {};
|
||||
}
|
||||
|
||||
llvm::Optional<Fix> IncludeFixer::insertHeader(llvm::StringRef Spelled,
|
||||
llvm::StringRef Symbol) const {
|
||||
Fix F;
|
||||
|
||||
if (auto Edit = Inserter->insert(Spelled))
|
||||
F.Edits.push_back(std::move(*Edit));
|
||||
else
|
||||
return llvm::None;
|
||||
|
||||
if (Symbol.empty())
|
||||
F.Message = llvm::formatv("Include {0}", Spelled);
|
||||
else
|
||||
F.Message = llvm::formatv("Include {0} for symbol {1}", Spelled, Symbol);
|
||||
|
||||
return F;
|
||||
}
|
||||
|
||||
std::vector<Fix> IncludeFixer::fixIncompleteType(const Type &T) const {
|
||||
// Only handle incomplete TagDecl type.
|
||||
const TagDecl *TD = T.getAsTagDecl();
|
||||
|
|
@ -160,14 +223,11 @@ std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
|
|||
for (const auto &Inc : getRankedIncludes(Sym)) {
|
||||
if (auto ToInclude = Inserted(Sym, Inc)) {
|
||||
if (ToInclude->second) {
|
||||
auto I = InsertedHeaders.try_emplace(ToInclude->first);
|
||||
if (!I.second)
|
||||
if (!InsertedHeaders.try_emplace(ToInclude->first).second)
|
||||
continue;
|
||||
if (auto Edit = Inserter->insert(ToInclude->first))
|
||||
Fixes.push_back(Fix{std::string(llvm::formatv(
|
||||
"Add include {0} for symbol {1}{2}",
|
||||
ToInclude->first, Sym.Scope, Sym.Name)),
|
||||
{std::move(*Edit)}});
|
||||
if (auto Fix =
|
||||
insertHeader(ToInclude->first, (Sym.Scope + Sym.Name).str()))
|
||||
Fixes.push_back(std::move(*Fix));
|
||||
}
|
||||
} else {
|
||||
vlog("Failed to calculate include insertion for {0} into {1}: {2}", Inc,
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ public:
|
|||
IndexRequestLimit(IndexRequestLimit) {}
|
||||
|
||||
/// Returns include insertions that can potentially recover the diagnostic.
|
||||
/// If Info is a note and fixes are returned, they should *replace* the note.
|
||||
std::vector<Fix> fix(DiagnosticsEngine::Level DiagLevel,
|
||||
const clang::Diagnostic &Info) const;
|
||||
|
||||
|
|
@ -55,6 +56,9 @@ private:
|
|||
/// Generates header insertion fixes for all symbols. Fixes are deduplicated.
|
||||
std::vector<Fix> fixesForSymbols(const SymbolSlab &Syms) const;
|
||||
|
||||
llvm::Optional<Fix> insertHeader(llvm::StringRef Name,
|
||||
llvm::StringRef Symbol = "") const;
|
||||
|
||||
struct UnresolvedName {
|
||||
std::string Name; // E.g. "X" in foo::X.
|
||||
SourceLocation Loc; // Start location of the unresolved name.
|
||||
|
|
|
|||
|
|
@ -867,58 +867,58 @@ class T {
|
|||
"incomplete type 'ns::X' named in nested name specifier"),
|
||||
DiagName("incomplete_nested_name_spec"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("base"), "base class has incomplete type"),
|
||||
DiagName("incomplete_base_class"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("access"),
|
||||
"member access into incomplete type 'ns::X'"),
|
||||
DiagName("incomplete_member_access"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(
|
||||
Diag(
|
||||
Test.range("type"),
|
||||
"incomplete type 'ns::X' where a complete type is required"),
|
||||
DiagName("incomplete_type"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("incomplete"),
|
||||
"variable has incomplete type 'ns::X'"),
|
||||
DiagName("typecheck_decl_incomplete_type"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(
|
||||
Diag(Test.range("tag"), "incomplete definition of type 'ns::X'"),
|
||||
DiagName("typecheck_incomplete_tag"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("use"),
|
||||
"invalid use of incomplete type 'ns::X'"),
|
||||
DiagName("invalid_incomplete_type_use"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("sizeof"),
|
||||
"invalid application of 'sizeof' to "
|
||||
"an incomplete type 'ns::X'"),
|
||||
DiagName("sizeof_alignof_incomplete_or_sizeless_type"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("for"),
|
||||
"cannot use incomplete type 'ns::X' as a range"),
|
||||
DiagName("for_range_incomplete_type"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("return"),
|
||||
"incomplete result type 'ns::X' in function definition"),
|
||||
DiagName("func_def_incomplete_result"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("field"), "field has incomplete type 'ns::X'"),
|
||||
DiagName("field_incomplete_or_sizeless"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X")))}))
|
||||
"Include \"x.h\" for symbol ns::X")))}))
|
||||
<< Test.code();
|
||||
}
|
||||
|
||||
|
|
@ -984,28 +984,28 @@ using Type = ns::$template[[Foo]]<int>;
|
|||
AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
|
||||
DiagName("unknown_typename"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"),
|
||||
AllOf(Diag(Test.range("qualified1"),
|
||||
"no type named 'X' in namespace 'ns'"),
|
||||
DiagName("typename_nested_not_found"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("qualified2"),
|
||||
"no member named 'X' in namespace 'ns'"),
|
||||
DiagName("no_member"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::X"))),
|
||||
"Include \"x.h\" for symbol ns::X"))),
|
||||
AllOf(Diag(Test.range("global"),
|
||||
"no type named 'Global' in the global namespace"),
|
||||
DiagName("typename_nested_not_found"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",
|
||||
"Add include \"global.h\" for symbol Global"))),
|
||||
"Include \"global.h\" for symbol Global"))),
|
||||
AllOf(Diag(Test.range("template"),
|
||||
"no template named 'Foo' in namespace 'ns'"),
|
||||
DiagName("no_member_template"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"foo.h\"\n",
|
||||
"Add include \"foo.h\" for symbol ns::Foo")))));
|
||||
"Include \"foo.h\" for symbol ns::Foo")))));
|
||||
}
|
||||
|
||||
TEST(IncludeFixerTest, MultipleMatchedSymbols) {
|
||||
|
|
@ -1029,12 +1029,12 @@ void foo() {
|
|||
Diag(Test.range("unqualified"), "unknown type name 'X'"),
|
||||
DiagName("unknown_typename"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"a.h\"\n",
|
||||
"Add include \"a.h\" for symbol na::X"),
|
||||
"Include \"a.h\" for symbol na::X"),
|
||||
Fix(Test.range("insert"), "#include \"b.h\"\n",
|
||||
"Add include \"b.h\" for symbol na::nb::X")))));
|
||||
"Include \"b.h\" for symbol na::nb::X")))));
|
||||
}
|
||||
|
||||
TEST(IncludeFixerTest, NoCrashMemebrAccess) {
|
||||
TEST(IncludeFixerTest, NoCrashMemberAccess) {
|
||||
Annotations Test(R"cpp(// error-ok
|
||||
struct X { int xyz; };
|
||||
void g() { X x; x.$[[xy]]; }
|
||||
|
|
@ -1085,8 +1085,7 @@ void bar(X *x) {
|
|||
ADD_FAILURE() << "D.Fixes.size() != 1";
|
||||
continue;
|
||||
}
|
||||
EXPECT_EQ(D.Fixes[0].Message,
|
||||
std::string("Add include \"a.h\" for symbol X"));
|
||||
EXPECT_EQ(D.Fixes[0].Message, std::string("Include \"a.h\" for symbol X"));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1106,11 +1105,11 @@ void g() { ns::$[[scope]]::X_Y(); }
|
|||
|
||||
EXPECT_THAT(
|
||||
*TU.build().getDiagnostics(),
|
||||
UnorderedElementsAre(AllOf(
|
||||
Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
|
||||
DiagName("no_member"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol ns::scope::X_Y")))));
|
||||
UnorderedElementsAre(
|
||||
AllOf(Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
|
||||
DiagName("no_member"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Include \"x.h\" for symbol ns::scope::X_Y")))));
|
||||
}
|
||||
|
||||
TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) {
|
||||
|
|
@ -1135,32 +1134,29 @@ void f() {
|
|||
EXPECT_THAT(
|
||||
*TU.build().getDiagnostics(),
|
||||
UnorderedElementsAre(
|
||||
AllOf(
|
||||
Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
|
||||
"did you mean 'clang'?"),
|
||||
DiagName("undeclared_var_use_suggest"),
|
||||
WithFix(_, // change clangd to clang
|
||||
Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol clang::clangd::X"))),
|
||||
AllOf(
|
||||
Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
|
||||
DiagName("typename_nested_not_found"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol clang::clangd::X"))),
|
||||
AllOf(Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
|
||||
"did you mean 'clang'?"),
|
||||
DiagName("undeclared_var_use_suggest"),
|
||||
WithFix(_, // change clangd to clang
|
||||
Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Include \"x.h\" for symbol clang::clangd::X"))),
|
||||
AllOf(Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
|
||||
DiagName("typename_nested_not_found"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Include \"x.h\" for symbol clang::clangd::X"))),
|
||||
AllOf(
|
||||
Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; "
|
||||
"did you mean 'clang'?"),
|
||||
DiagName("undeclared_var_use_suggest"),
|
||||
WithFix(
|
||||
_, // change clangd to clang
|
||||
Fix(Test.range("insert"), "#include \"y.h\"\n",
|
||||
"Add include \"y.h\" for symbol clang::clangd::ns::Y"))),
|
||||
WithFix(_, // change clangd to clang
|
||||
Fix(Test.range("insert"), "#include \"y.h\"\n",
|
||||
"Include \"y.h\" for symbol clang::clangd::ns::Y"))),
|
||||
AllOf(Diag(Test.range("ns"),
|
||||
"no member named 'ns' in namespace 'clang'"),
|
||||
DiagName("no_member"),
|
||||
WithFix(Fix(
|
||||
Test.range("insert"), "#include \"y.h\"\n",
|
||||
"Add include \"y.h\" for symbol clang::clangd::ns::Y")))));
|
||||
WithFix(
|
||||
Fix(Test.range("insert"), "#include \"y.h\"\n",
|
||||
"Include \"y.h\" for symbol clang::clangd::ns::Y")))));
|
||||
}
|
||||
|
||||
TEST(IncludeFixerTest, SpecifiedScopeIsNamespaceAlias) {
|
||||
|
|
@ -1181,7 +1177,7 @@ namespace c {
|
|||
Diag(Test.range(), "no type named 'X' in namespace 'a'"),
|
||||
DiagName("typename_nested_not_found"),
|
||||
WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
|
||||
"Add include \"x.h\" for symbol a::X")))));
|
||||
"Include \"x.h\" for symbol a::X")))));
|
||||
}
|
||||
|
||||
TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
|
||||
|
|
@ -1206,6 +1202,26 @@ TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
|
|||
ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
|
||||
}
|
||||
|
||||
TEST(IncludeFixerTest, HeaderNamedInDiag) {
|
||||
Annotations Test(R"cpp(
|
||||
$insert[[]]int main() {
|
||||
[[printf]]("");
|
||||
}
|
||||
)cpp");
|
||||
auto TU = TestTU::withCode(Test.code());
|
||||
TU.ExtraArgs = {"-xc"};
|
||||
auto Index = buildIndexWithSymbol({});
|
||||
TU.ExternalIndex = Index.get();
|
||||
|
||||
EXPECT_THAT(
|
||||
*TU.build().getDiagnostics(),
|
||||
ElementsAre(AllOf(
|
||||
Diag(Test.range(), "implicitly declaring library function 'printf' "
|
||||
"with type 'int (const char *, ...)'"),
|
||||
WithFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
|
||||
"Include <stdio.h> for symbol printf")))));
|
||||
}
|
||||
|
||||
TEST(DiagsInHeaders, DiagInsideHeader) {
|
||||
Annotations Main(R"cpp(
|
||||
#include [["a.h"]]
|
||||
|
|
|
|||
Loading…
Reference in New Issue