diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -108,7 +108,7 @@ /// Enable emitting diagnostics using stale preambles. bool AllowStalePreamble = false; - IncludesPolicy UnusedIncludes = IncludesPolicy::None; + IncludesPolicy UnusedIncludes = IncludesPolicy::Strict; IncludesPolicy MissingIncludes = IncludesPolicy::None; /// IncludeCleaner will not diagnose usages of these headers matched by diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -246,22 +246,19 @@ } TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) { - // Defaults to None. + // Defaults to Strict. EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::IncludesPolicy::None); + EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::Strict); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("None"); EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::IncludesPolicy::None); + EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("Strict"); EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::IncludesPolicy::Strict); + EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::Strict); Frag = {}; EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty()) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "../clang-tidy/ClangTidyOptions.h" #include "Annotations.h" #include "Config.h" #include "Diagnostics.h" @@ -18,18 +19,28 @@ #include "TestTU.h" #include "TidyProvider.h" #include "index/MemIndex.h" +#include "index/Ref.h" +#include "index/Relation.h" +#include "index/Symbol.h" #include "support/Context.h" #include "support/Path.h" +#include "clang/AST/Decl.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/LLVM.h" +#include "clang/Basic/Specifiers.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/JSON.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/TargetSelect.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include +#include #include +#include +#include +#include namespace clang { namespace clangd { @@ -42,6 +53,7 @@ using ::testing::ElementsAre; using ::testing::Field; using ::testing::IsEmpty; +using ::testing::Not; using ::testing::Pair; using ::testing::SizeIs; using ::testing::UnorderedElementsAre; @@ -889,7 +901,8 @@ int symbol; )cpp"); TU.Filename = "foo.h"; - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); + EXPECT_THAT(*TU.build().getDiagnostics(), + Not(Contains(diagName("pp_including_mainfile_in_preamble")))); EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1)); } @@ -1597,9 +1610,9 @@ TU.AdditionalFiles = {{"a.h", "#include \"b.h\""}, {"b.h", "no_type_spec; // error-ok"}}; EXPECT_THAT(*TU.build().getDiagnostics(), - UnorderedElementsAre( - Diag(Main.range(), "in included file: a type specifier is " - "required for all declarations"))); + UnorderedElementsAre(Diag(Main.range(), + "in included file: a type specifier is " + "required for all declarations"))); } TEST(DiagsInHeaders, DiagInMultipleHeaders) { @@ -1628,9 +1641,8 @@ {"a.h", "#include \"b.h\"\n"}, {"b.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}}; EXPECT_THAT(*TU.build().getDiagnostics(), - UnorderedElementsAre(Diag(Main.range(), - "in included file: a type specifier is " - "required for all declarations"))); + Contains(Diag(Main.range(), "in included file: a type specifier " + "is required for all declarations"))); } TEST(DiagsInHeaders, PreferExpansionLocationMacros) { @@ -1646,9 +1658,9 @@ {"b.h", "#include \"c.h\"\n"}, {"c.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}}; EXPECT_THAT(*TU.build().getDiagnostics(), - UnorderedElementsAre( - Diag(Main.range(), "in included file: a type specifier is " - "required for all declarations"))); + UnorderedElementsAre(Diag(Main.range(), + "in included file: a type specifier is " + "required for all declarations"))); } TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) { @@ -1675,9 +1687,9 @@ no_type_spec_10; #endif)cpp"}}; EXPECT_THAT(*TU.build().getDiagnostics(), - UnorderedElementsAre( - Diag(Main.range(), "in included file: a type specifier is " - "required for all declarations"))); + UnorderedElementsAre(Diag(Main.range(), + "in included file: a type specifier is " + "required for all declarations"))); } TEST(DiagsInHeaders, OnlyErrorOrFatal) { @@ -1897,8 +1909,6 @@ )cpp"; TU.AdditionalFiles["system/system_header.h"] = ""; TU.ExtraArgs = {"-isystem" + testPath("system")}; - // Off by default. - EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); Config Cfg; Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; // Set filtering. @@ -1908,11 +1918,11 @@ auto AST = TU.build(); EXPECT_THAT( *AST.getDiagnostics(), - UnorderedElementsAre( - AllOf(Diag(Test.range("diag"), - "included header unused.h is not used directly"), - withTag(DiagnosticTag::Unnecessary), diagSource(Diag::Clangd), - withFix(Fix(Test.range("fix"), "", "remove #include directive"))))); + Contains(AllOf( + Diag(Test.range("diag"), + "included header unused.h is not used directly"), + withTag(DiagnosticTag::Unnecessary), diagSource(Diag::Clangd), + withFix(Fix(Test.range("fix"), "", "remove #include directive"))))); auto &Diag = AST.getDiagnostics()->front(); EXPECT_EQ(getDiagnosticDocURI(Diag.Source, Diag.ID, Diag.Name), std::string("https://clangd.llvm.org/guides/include-cleaner")); diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -15,6 +15,7 @@ #include "AST.h" #include "Annotations.h" #include "Compiler.h" +#include "Config.h" #include "Diagnostics.h" #include "Headers.h" #include "ParsedAST.h" @@ -23,6 +24,7 @@ #include "TestFS.h" #include "TestTU.h" #include "TidyProvider.h" +#include "support/Context.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -33,6 +35,7 @@ #include "gmock/gmock-matchers.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include namespace clang { namespace clangd { @@ -513,6 +516,13 @@ // size is part of the lookup key for HeaderFileInfo, and we don't want to // rely on the preamble's HFI being looked up when parsing the main file. TEST(ParsedASTTest, HeaderGuardsSelfInclude) { + // Disable include cleaner diagnostics to prevent them from interfering with + // other diagnostics. + Config Cfg; + Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::None; + Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::None; + WithContextValue Ctx(Config::Key, std::move(Cfg)); + TestTU TU; TU.ImplicitHeaderGuard = false; TU.Filename = "self.h";