diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -19,12 +19,15 @@ endif () option(CLANGD_MALLOC_TRIM "Call malloc_trim(3) periodically in Clangd. (only takes effect when using glibc)" ON) +# -DCLANG_TIDY_CHECKS=Off avoids a dependency on clang-tidy, reducing rebuilds. +option(CLANGD_TIDY_CHECKS "Link all clang-tidy checks into clangd" ON) llvm_canonicalize_cmake_booleans( CLANGD_BUILD_XPC CLANGD_ENABLE_REMOTE ENABLE_GRPC_REFLECTION CLANGD_MALLOC_TRIM + CLANGD_TIDY_CHECKS LLVM_ENABLE_ZLIB ) @@ -162,10 +165,12 @@ ${LLVM_PTHREAD_LIB} clangTidy - ${ALL_CLANG_TIDY_CHECKS} clangdSupport ) +if(CLANGD_TIDY_CHECKS) + target_link_libraries(clangDaemon PRIVATE ${ALL_CLANG_TIDY_CHECKS}) +endif() add_subdirectory(refactor/tweaks) if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux") diff --git a/clang-tools-extra/clangd/Features.cpp b/clang-tools-extra/clangd/Features.cpp --- a/clang-tools-extra/clangd/Features.cpp +++ b/clang-tools-extra/clangd/Features.cpp @@ -48,6 +48,10 @@ #if CLANGD_BUILD_XPC "+xpc" #endif + +#if !CLANGD_TIDY_CHECKS + "-tidy" +#endif ; } diff --git a/clang-tools-extra/clangd/Features.inc.in b/clang-tools-extra/clangd/Features.inc.in --- a/clang-tools-extra/clangd/Features.inc.in +++ b/clang-tools-extra/clangd/Features.inc.in @@ -3,3 +3,4 @@ #define CLANGD_ENABLE_REMOTE @CLANGD_ENABLE_REMOTE@ #define ENABLE_GRPC_REFLECTION @ENABLE_GRPC_REFLECTION@ #define CLANGD_MALLOC_TRIM @CLANGD_MALLOC_TRIM@ +#define CLANGD_TIDY_CHECKS @CLANGD_TIDY_CHECKS@ diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -15,6 +15,7 @@ #include "Config.h" #include "Diagnostics.h" #include "FeatureModule.h" +#include "Features.h" #include "Headers.h" #include "HeuristicResolver.h" #include "IncludeFixer.h" @@ -60,8 +61,10 @@ // Force the linker to link in Clang-tidy modules. // clangd doesn't support the static analyzer. +#if CLANGD_TIDY_CHECKS #define CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS #include "../clang-tidy/ClangTidyForceLinker.h" +#endif namespace clang { namespace clangd { diff --git a/clang-tools-extra/clangd/test/diagnostics.test b/clang-tools-extra/clangd/test/diagnostics-tidy.test rename from clang-tools-extra/clangd/test/diagnostics.test rename to clang-tools-extra/clangd/test/diagnostics-tidy.test --- a/clang-tools-extra/clangd/test/diagnostics.test +++ b/clang-tools-extra/clangd/test/diagnostics-tidy.test @@ -1,27 +1,12 @@ +# REQUIRES: clangd-tidy-checks # RUN: clangd -lit-test -clang-tidy-checks=bugprone-sizeof-expression < %s | FileCheck -strict-whitespace %s {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} --- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"void main() {\n(void)sizeof(42);\n}"}}} +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"int main() {\n(void)sizeof(42);\n}"}}} # CHECK: "method": "textDocument/publishDiagnostics", # CHECK-NEXT: "params": { # CHECK-NEXT: "diagnostics": [ # CHECK-NEXT: { -# CHECK-NEXT: "code": "-Wmain-return-type", -# CHECK-NEXT: "message": "Return type of 'main' is not 'int' (fix available)", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 4, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "severity": 2, -# CHECK-NEXT: "source": "clang" -# CHECK-NEXT: }, -# CHECK-NEXT: { # CHECK-NEXT: "code": "bugprone-sizeof-expression", # CHECK-NEXT: "message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'?", # CHECK-NEXT: "range": { @@ -54,3 +39,4 @@ {"jsonrpc":"2.0","id":5,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"} + diff --git a/clang-tools-extra/clangd/test/lit.cfg.py b/clang-tools-extra/clangd/test/lit.cfg.py --- a/clang-tools-extra/clangd/test/lit.cfg.py +++ b/clang-tools-extra/clangd/test/lit.cfg.py @@ -31,5 +31,8 @@ if config.clangd_enable_remote: config.available_features.add('clangd-remote-index') +if config.clangd_tidy_checks: + config.available_features.add('clangd-tidy-checks') + if config.have_zlib: config.available_features.add('zlib') diff --git a/clang-tools-extra/clangd/test/lit.site.cfg.py.in b/clang-tools-extra/clangd/test/lit.site.cfg.py.in --- a/clang-tools-extra/clangd/test/lit.site.cfg.py.in +++ b/clang-tools-extra/clangd/test/lit.site.cfg.py.in @@ -25,6 +25,7 @@ config.clangd_binary_dir = "@CMAKE_CURRENT_BINARY_DIR@/.." config.clangd_build_xpc = @CLANGD_BUILD_XPC@ config.clangd_enable_remote = @CLANGD_ENABLE_REMOTE@ +config.clangd_tidy_checks = @CLANGD_TIDY_CHECKS@ config.have_zlib = @LLVM_ENABLE_ZLIB@ # Delegate logic to lit.cfg.py. 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 @@ -277,6 +277,7 @@ EXPECT_THAT(Conf.Diagnostics.Suppress, IsEmpty()); } +#if CLANGD_TIDY_CHECKS TEST_F(ConfigCompileTests, Tidy) { auto &Tidy = Frag.Diagnostics.ClangTidy; Tidy.Add.emplace_back("bugprone-use-after-move"); @@ -299,6 +300,7 @@ "0"); EXPECT_THAT(Diags.Diagnostics, IsEmpty()); } +#endif // CLANGD_TIDY_CHECKS TEST_F(ConfigCompileTests, TidyBadChecks) { auto &Tidy = Frag.Diagnostics.ClangTidy; 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 @@ -10,6 +10,7 @@ #include "Config.h" #include "Diagnostics.h" #include "FeatureModule.h" +#include "Features.h" #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" @@ -114,6 +115,19 @@ return Res; } +// Normally returns the provided diagnostics matcher. +// If clang-tidy checks are not linked in, returns a matcher for no diagnostics! +// This is intended for tests where the diagnostics come from clang-tidy checks. +// We don't #ifdef each individual test as it's intrusive and we want to ensure +// that as much of the test is still compiled an run as possible. +::testing::Matcher> +ifTidyChecks(::testing::Matcher> M, + ::testing::Matcher> Else = IsEmpty()) { + if (!CLANGD_TIDY_CHECKS) + return Else; + return M; +} + TEST(DiagnosticsTest, DiagnosticRanges) { // Check we report correct ranges, including various edge-cases. Annotations Test(R"cpp( @@ -121,7 +135,6 @@ #define ID(X) X namespace test{}; void $decl[[foo]](); - class T{$explicit[[]]$constructor[[T]](int a);}; int main() { $typo[[go\ o]](); @@ -163,13 +176,7 @@ AllOf(Diag(Test.range("macro"), "use of undeclared identifier 'fod'; did you mean 'foo'?"), WithFix(Fix(Test.range("macroarg"), "foo", - "change 'fod' to 'foo'"))), - // We make sure here that the entire token is highlighted - AllOf(Diag(Test.range("constructor"), - "single-argument constructors must be marked explicit to " - "avoid unintentional implicit conversions"), - WithFix(Fix(Test.range("explicit"), "explicit ", - "insert 'explicit '"))))); + "change 'fod' to 'foo'"))))); } TEST(DiagnosticsTest, FlagsMatter) { @@ -212,10 +219,10 @@ // Verify that we filter out the duplicated diagnostic message. EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre(::testing::AllOf( + ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Test.range(), "floating point literal has suffix 'f', which is not uppercase"), - DiagSource(Diag::ClangTidy)))); + DiagSource(Diag::ClangTidy))))); Test = Annotations(R"cpp( template @@ -232,10 +239,10 @@ // duplicated messages, verify that we deduplicate them. EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre(::testing::AllOf( + ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Test.range(), "floating point literal has suffix 'f', which is not uppercase"), - DiagSource(Diag::ClangTidy)))); + DiagSource(Diag::ClangTidy))))); } TEST(DiagnosticsTest, ClangTidy) { @@ -249,6 +256,8 @@ return $doubled[[sizeof]](sizeof(int)); } + class T{$explicit[[]]$constructor[[T]](int a);}; + // misc-no-recursion uses a custom traversal from the TUDecl void foo(); void $bar[[bar]]() { @@ -262,43 +271,57 @@ TU.HeaderFilename = "assert.h"; // Suppress "not found" error. TU.ClangTidyProvider = addTidyChecks("bugprone-sizeof-expression," "bugprone-macro-repeated-side-effects," + "google-explicit-constructor," "modernize-deprecated-headers," "modernize-use-trailing-return-type," "misc-no-recursion"); EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre( - AllOf(Diag(Test.range("deprecated"), - "inclusion of deprecated C++ header 'assert.h'; consider " - "using 'cassert' instead"), - DiagSource(Diag::ClangTidy), - DiagName("modernize-deprecated-headers"), - WithFix(Fix(Test.range("deprecated"), "", - "change '\"assert.h\"' to ''"))), - Diag(Test.range("doubled"), - "suspicious usage of 'sizeof(sizeof(...))'"), - AllOf( + ifTidyChecks( + UnorderedElementsAre( + AllOf( + Diag( + Test.range("deprecated"), + "inclusion of deprecated C++ header 'assert.h'; consider " + "using 'cassert' instead"), + DiagSource(Diag::ClangTidy), + DiagName("modernize-deprecated-headers"), + WithFix(Fix(Test.range("deprecated"), "", + "change '\"assert.h\"' to ''"))), + Diag(Test.range("doubled"), + "suspicious usage of 'sizeof(sizeof(...))'"), + AllOf(Diag(Test.range("macroarg"), + "side effects in the 1st macro argument 'X' are " + "repeated in " + "macro expansion"), + DiagSource(Diag::ClangTidy), + DiagName("bugprone-macro-repeated-side-effects"), + WithNote(Diag(Test.range("macrodef"), + "macro 'SQUARE' defined here"))), Diag(Test.range("macroarg"), - "side effects in the 1st macro argument 'X' are repeated in " - "macro expansion"), - DiagSource(Diag::ClangTidy), - DiagName("bugprone-macro-repeated-side-effects"), - WithNote( - Diag(Test.range("macrodef"), "macro 'SQUARE' defined here"))), - Diag(Test.range("macroarg"), - "multiple unsequenced modifications to 'y'"), - AllOf( - Diag(Test.range("main"), - "use a trailing return type for this function"), - DiagSource(Diag::ClangTidy), - DiagName("modernize-use-trailing-return-type"), - // Verify that we don't have "[check-name]" suffix in the message. - WithFix( - FixMessage("use a trailing return type for this function"))), - Diag(Test.range("foo"), - "function 'foo' is within a recursive call chain"), - Diag(Test.range("bar"), - "function 'bar' is within a recursive call chain"))); + "multiple unsequenced modifications to 'y'"), + AllOf(Diag(Test.range("main"), + "use a trailing return type for this function"), + DiagSource(Diag::ClangTidy), + DiagName("modernize-use-trailing-return-type"), + // Verify there's no "[check-name]" suffix in the message. + WithFix(FixMessage( + "use a trailing return type for this function"))), + Diag(Test.range("foo"), + "function 'foo' is within a recursive call chain"), + Diag(Test.range("bar"), + "function 'bar' is within a recursive call chain"), + // We make sure here that the entire token is highlighted + AllOf( + Diag( + Test.range("constructor"), + "single-argument constructors must be marked explicit to " + "avoid unintentional implicit conversions"), + WithFix(Fix(Test.range("explicit"), "explicit ", + "insert 'explicit '")))), + /*Else=*/ElementsAre( + Diag(Test.range("macroarg"), + "multiple unsequenced modifications to 'y'")))); } TEST(DiagnosticsTest, ClangTidyEOF) { @@ -313,9 +336,9 @@ TU.ClangTidyProvider = addTidyChecks("llvm-include-order"); EXPECT_THAT( *TU.build().getDiagnostics(), - Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"), - DiagSource(Diag::ClangTidy), - DiagName("llvm-include-order")))); + ifTidyChecks(Contains( + AllOf(Diag(Test.range(), "#includes are not sorted properly"), + DiagSource(Diag::ClangTidy), DiagName("llvm-include-order"))))); } TEST(DiagnosticTest, TemplatesInHeaders) { @@ -385,9 +408,9 @@ TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert"); EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre(::testing::AllOf( + ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "use range-based for loop instead"), - DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert")))); + DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))))); } TEST(DiagnosticTest, RespectsDiagnosticConfig) { @@ -430,10 +453,11 @@ TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division"); EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre(::testing::AllOf( + ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "result of integer division used in a floating " "point context; possible loss of precision"), - DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division")))); + DiagSource(Diag::ClangTidy), + DiagName("bugprone-integer-division"))))); } TEST(DiagnosticTest, ClangTidyWarningAsError) { @@ -448,11 +472,11 @@ addTidyChecks("bugprone-integer-division", "bugprone-integer-division"); EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre(::testing::AllOf( + ifTidyChecks(UnorderedElementsAre(::testing::AllOf( Diag(Main.range(), "result of integer division used in a floating " "point context; possible loss of precision"), DiagSource(Diag::ClangTidy), DiagName("bugprone-integer-division"), - DiagSeverity(DiagnosticsEngine::Error)))); + DiagSeverity(DiagnosticsEngine::Error))))); } TEST(DiagnosticTest, LongFixMessages) { @@ -528,9 +552,9 @@ )cpp"); TestTU TU = TestTU::withCode(Main.code()); TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return"); - EXPECT_THAT( - *TU.build().getDiagnostics(), - ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'"))); + EXPECT_THAT(*TU.build().getDiagnostics(), + ifTidyChecks(ElementsAre( + Diag(Main.range(), "do not use 'else' after 'return'")))); } TEST(DiagnosticsTest, Preprocessor) {