maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Add CMake option to (not) link in clang-tidy checks 

This reduces the size of the dependency graph and makes incremental
development a little more pleasant (less rebuilding).

This introduces a bit of complexity/fragility as some tests verify
clang-tidy behavior. I attempted to isolate these and build/run as much
of the tests as possible in both configs to prevent rot.

Expectation is that (some) developers will use this locally, but
buildbots etc will keep testing clang-tidy.

Fixes https://github.com/clangd/clangd/issues/233

Differential Revision: https://reviews.llvm.org/D105679
This commit is contained in:
Sam McCall 2021-07-09 09:40:18 +02:00
parent d9b9fdd91b
commit 462d4de35b
9 changed files with 92 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
clang-tools-extra/clangd/CMakeLists.txt
View File

@ -19,12 +19,15 @@ if (NOT DEFINED CLANGD_BUILD_XPC)
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 @@ target_link_libraries(clangDaemon
${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")

4
clang-tools-extra/clangd/Features.cpp
View File

@ -48,6 +48,10 @@ std::string featureString() {
#if CLANGD_BUILD_XPC
"+xpc"
#endif
#if !CLANGD_TIDY_CHECKS
"-tidy"
#endif
;
}

1
clang-tools-extra/clangd/Features.inc.in
View File

@ -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@

3
clang-tools-extra/clangd/ParsedAST.cpp
View File

@ -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 {

20
clang-tools-extra/clangd/test/diagnostics.test → clang-tools-extra/clangd/test/diagnostics-tidy.test
View File

@ -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"}

3
clang-tools-extra/clangd/test/lit.cfg.py
View File

@ -31,5 +31,8 @@ if config.clangd_build_xpc:
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')

1
clang-tools-extra/clangd/test/lit.site.cfg.py.in
View File

@ -25,6 +25,7 @@ config.clangd_source_dir = "@CMAKE_CURRENT_SOURCE_DIR@/.."
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.

16
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
View File

@ -288,16 +288,26 @@ TEST_F(ConfigCompileTests, Tidy) {
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) {

103
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
View File

@ -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 @@ Position pos(int line, int character) {
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<std::vector<clangd::Diag>>
ifTidyChecks(::testing::Matcher<std::vector<clangd::Diag>> 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 @@ TEST(DiagnosticsTest, DiagnosticRanges) {
#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 @@ o]]();
}
)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 @@ o]]();
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 @@ TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) {
// 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<typename T>
@ -232,10 +246,10 @@ TEST(DiagnosticsTest, DeduplicatedClangTidyDiagnostics) {
// 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 @@ TEST(DiagnosticsTest, ClangTidy) {
"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 @@ TEST(DiagnosticsTest, ClangTidy) {
"change '\"assert.h\"' to '<cassert>'"))),
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 @@ TEST(DiagnosticsTest, ClangTidyEOF) {
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 @@ TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
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 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) {
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 @@ TEST(DiagnosticTest, ClangTidyWarningAsError) {
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 @@ TEST(DiagnosticTest, ElseAfterReturnRange) {
)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) {