[clang-tools-extra] ea20f33 - [clangd] Enforce strict unused includes by default
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 12 02:48:32 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-06-12T11:47:03+02:00
New Revision: ea20f339d91f57694899c29198f2dfb41bf9a03a
URL: https://github.com/llvm/llvm-project/commit/ea20f339d91f57694899c29198f2dfb41bf9a03a
DIFF: https://github.com/llvm/llvm-project/commit/ea20f339d91f57694899c29198f2dfb41bf9a03a.diff
LOG: [clangd] Enforce strict unused includes by default
Depends on D152685
Differential Revision: https://reviews.llvm.org/D152686
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 4c6fad25384a8..78fcec1582f00 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -108,7 +108,7 @@ struct Config {
/// 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
index 45dacba5822b7..f0ba582187fd5 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -246,22 +246,19 @@ TEST_F(ConfigCompileTests, PathSpecMatch) {
}
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
index 552f4f4ecf886..62b5c466ee77c 100644
--- 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 <algorithm>
+#include <cstddef>
#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
namespace clang {
namespace clangd {
@@ -42,6 +53,7 @@ using ::testing::Each;
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 @@ TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) {
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 @@ TEST(DiagsInHeaders, DiagInTransitiveInclude) {
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 @@ TEST(DiagsInHeaders, PreferExpansionLocation) {
{"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 @@ TEST(DiagsInHeaders, PreferExpansionLocationMacros) {
{"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 @@ TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
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 @@ TEST(DiagnosticsTest, IncludeCleaner) {
)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 @@ TEST(DiagnosticsTest, IncludeCleaner) {
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
index 8d5d0630de165..452d33087776a 100644
--- 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 <utility>
namespace clang {
namespace clangd {
@@ -513,6 +516,13 @@ TEST(ParsedASTTest, HeaderGuards) {
// 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";
More information about the cfe-commits
mailing list