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 @@ -288,16 +288,26 @@ Tidy.CheckOptions.emplace_back(std::make_pair( std::string("example-check.ExampleOption"), std::string("0"))); EXPECT_TRUE(compileAndApply()); - EXPECT_EQ( - Conf.Diagnostics.ClangTidy.Checks, - "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*"); EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.size(), 2U); EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.lookup("StrictMode"), "true"); EXPECT_EQ(Conf.Diagnostics.ClangTidy.CheckOptions.lookup( "example-check.ExampleOption"), "0"); +#if CLANGD_TIDY_CHECKS + EXPECT_EQ( + Conf.Diagnostics.ClangTidy.Checks, + "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*"); EXPECT_THAT(Diags.Diagnostics, IsEmpty()); +#else // !CLANGD_TIDY_CHECKS + EXPECT_EQ(Conf.Diagnostics.ClangTidy.Checks, "llvm-*,-readability-*"); + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre( + DiagMessage( + "clang-tidy check 'bugprone-use-after-move' was not found"), + DiagMessage("clang-tidy check 'llvm-include-order' was not found"))); +#endif } TEST_F(ConfigCompileTests, TidyBadChecks) { 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,18 @@ 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) { + if (!CLANGD_TIDY_CHECKS) + return IsEmpty(); + return M; +} + TEST(DiagnosticsTest, DiagnosticRanges) { // Check we report correct ranges, including various edge-cases. Annotations Test(R"cpp( @@ -121,8 +134,11 @@ #define ID(X) X namespace test{}; void $decl[[foo]](); - class T{$explicit[[]]$constructor[[T]](int a);}; int main() { + struct Container { int* begin(); int* end(); } *container; + for (auto i : $insertstar[[]]$range[[container]]) { + } + $typo[[go\ o]](); foo()$semicolon[[]]//with comments @@ -135,10 +151,14 @@ } )cpp"); auto TU = TestTU::withCode(Test.code()); - TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor"); EXPECT_THAT( *TU.build().getDiagnostics(), ElementsAre( + // Make sure the whole token is highlighted. + AllOf(Diag(Test.range("range"), + "invalid range expression of type 'struct Container *'; " + "did you mean to dereference it with '*'?"), + WithFix(Fix(Test.range("insertstar"), "*", "insert '*'"))), // This range spans lines. AllOf(Diag(Test.range("typo"), "use of undeclared identifier 'goo'; did you mean 'foo'?"), @@ -163,13 +183,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 +226,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 +246,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) { @@ -265,9 +279,10 @@ "modernize-deprecated-headers," "modernize-use-trailing-return-type," "misc-no-recursion"); + TU.ExtraArgs.push_back("-Wno-unsequenced"); EXPECT_THAT( *TU.build().getDiagnostics(), - UnorderedElementsAre( + ifTidyChecks(UnorderedElementsAre( AllOf(Diag(Test.range("deprecated"), "inclusion of deprecated C++ header 'assert.h'; consider " "using 'cassert' instead"), @@ -277,28 +292,25 @@ "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"), - "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"))), + 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"))), + 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"))); + "function 'bar' is within a recursive call chain")))); } TEST(DiagnosticsTest, ClangTidyEOF) { @@ -313,9 +325,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 +397,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 +442,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 +461,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 +541,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) {