-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[clang-tidy] support query based custom check #131804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 11 commits
a686695
421b26b
6fa8f7e
6ab7149
db6b315
dba8a8c
0df6f7e
5bb2c6c
64c45dc
b650594
b56c07c
5bbf53d
b43991f
b2cecb8
71eeaa1
8be412c
1367f0b
595e0a6
190ad52
5500699
4ce48a5
fd15b05
73dfc6e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
set(LLVM_LINK_COMPONENTS | ||
support | ||
) | ||
|
||
add_clang_library(clangTidyCustomModule STATIC | ||
CustomTidyModule.cpp | ||
QueryCheck.cpp | ||
|
||
LINK_LIBS | ||
clangTidy | ||
clangTidyUtils | ||
|
||
DEPENDS | ||
ClangDriverOptions | ||
) | ||
|
||
clang_target_link_libraries(clangTidyCustomModule | ||
PRIVATE | ||
clangQuery | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now clang-tidy is made to be dependent on clangQuery, which is a major change. Is this OK? How are build times affected? It would be great to have @AaronBallman's opinion on this one :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably would be fine, would be good to have cmake options, to enable custom checks, and then if not enabled then support for them woudn't be included in binary There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, something similar to what's done with the static-analyzer checks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the suggestion from @PiotrZSL is a reasonable compromise. I'm presuming it's not an issue that the |
||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
#include "../ClangTidy.h" | ||
#include "../ClangTidyModule.h" | ||
#include "../ClangTidyModuleRegistry.h" | ||
#include "../ClangTidyOptions.h" | ||
#include "QueryCheck.h" | ||
#include "llvm/ADT/SmallSet.h" | ||
#include "llvm/ADT/SmallString.h" | ||
#include "llvm/ADT/StringRef.h" | ||
#include <memory> | ||
|
||
namespace clang::tidy { | ||
namespace custom { | ||
|
||
class CustomModule : public ClangTidyModule { | ||
public: | ||
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {} | ||
}; | ||
|
||
// We need to register the checks more flexibly than builtin modules. The checks | ||
// will changed dynamically when switching to different source file. | ||
extern void registerCustomChecks(ClangTidyOptions const &Options, | ||
HerrCai0907 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ClangTidyCheckFactories &Factories) { | ||
static llvm::SmallSet<llvm::SmallString<32>, 8> CustomCheckNames{}; | ||
if (!Options.CustomChecks.has_value() || Options.CustomChecks->empty()) | ||
return; | ||
for (llvm::SmallString<32> const &Name : CustomCheckNames) | ||
Factories.erase(Name); | ||
HerrCai0907 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (const ClangTidyOptions::CustomCheckValue &V : | ||
Options.CustomChecks.value()) { | ||
llvm::SmallString<32> Name = llvm::StringRef{"custom-" + V.Name}; | ||
Factories.registerCheckFactory( | ||
// add custom- prefix to avoid conflicts with builtin checks | ||
Name, [&V](llvm::StringRef Name, ClangTidyContext *Context) { | ||
return std::make_unique<custom::QueryCheck>(Name, V, Context); | ||
}); | ||
CustomCheckNames.insert(std::move(Name)); | ||
} | ||
} | ||
|
||
} // namespace custom | ||
|
||
// Register the CustomTidyModule using this statically initialized variable. | ||
static ClangTidyModuleRegistry::Add<custom::CustomModule> | ||
X("custom-module", "Adds custom query lint checks."); | ||
|
||
// This anchor is used to force the linker to link in the generated object file | ||
// and thus register the AlteraModule. | ||
volatile int CustomModuleAnchorSource = 0; | ||
HerrCai0907 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
} // namespace clang::tidy |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
//===--- QueryCheck.cpp - clang-tidy --------------------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "QueryCheck.h" | ||
#include "../../clang-query/Query.h" | ||
#include "../../clang-query/QueryParser.h" | ||
#include "clang/ASTMatchers/ASTMatchers.h" | ||
#include "clang/ASTMatchers/Dynamic/VariantValue.h" | ||
#include "clang/Basic/DiagnosticIDs.h" | ||
#include "llvm/ADT/SmallVector.h" | ||
#include "llvm/ADT/StringRef.h" | ||
#include <string> | ||
|
||
using namespace clang::ast_matchers; | ||
|
||
namespace clang::tidy::custom { | ||
|
||
QueryCheck::QueryCheck(llvm::StringRef Name, | ||
const ClangTidyOptions::CustomCheckValue &V, | ||
ClangTidyContext *Context) | ||
: ClangTidyCheck(Name, Context) { | ||
for (const ClangTidyOptions::CustomCheckDiag &D : V.Diags) { | ||
auto DiagnosticIdIt = | ||
Diags | ||
.try_emplace(D.Level.value_or(DiagnosticIDs::Warning), | ||
llvm::StringMap<llvm::SmallVector<std::string>>{}) | ||
.first; | ||
auto DiagMessageIt = | ||
DiagnosticIdIt->getSecond() | ||
.try_emplace(D.BindName, llvm::SmallVector<std::string>{}) | ||
.first; | ||
DiagMessageIt->second.emplace_back(D.Message); | ||
} | ||
|
||
clang::query::QuerySession QS({}); | ||
llvm::StringRef QueryStringRef{V.Query}; | ||
while (!QueryStringRef.empty()) { | ||
query::QueryRef Q = query::QueryParser::parse(QueryStringRef, QS); | ||
switch (Q->Kind) { | ||
case query::QK_Match: { | ||
const auto &MatchQuerry = llvm::cast<query::MatchQuery>(*Q); | ||
Matchers.push_back(MatchQuerry.Matcher); | ||
break; | ||
} | ||
case query::QK_Let: { | ||
const auto &LetQuerry = llvm::cast<query::LetQuery>(*Q); | ||
LetQuerry.run(llvm::errs(), QS); | ||
break; | ||
} | ||
case query::QK_Invalid: { | ||
const auto &InvalidQuerry = llvm::cast<query::InvalidQuery>(*Q); | ||
Context->configurationDiag(InvalidQuerry.ErrStr); | ||
break; | ||
} | ||
// FIXME: TODO | ||
case query::QK_File: | ||
case query::QK_DisableOutputKind: | ||
case query::QK_EnableOutputKind: | ||
case query::QK_SetOutputKind: | ||
case query::QK_SetTraversalKind: | ||
case query::QK_Help: | ||
case query::QK_NoOp: | ||
case query::QK_Quit: | ||
case query::QK_SetBool: { | ||
Context->configurationDiag("unsupported querry kind"); | ||
} | ||
} | ||
QueryStringRef = Q->RemainingContent; | ||
} | ||
} | ||
|
||
void QueryCheck::registerMatchers(MatchFinder *Finder) { | ||
for (const ast_matchers::dynamic::DynTypedMatcher &M : Matchers) | ||
Finder->addDynamicMatcher(M, this); | ||
} | ||
|
||
void QueryCheck::check(const MatchFinder::MatchResult &Result) { | ||
auto Emit = [this](DiagMaps const &DiagMaps, std::string const &BindName, | ||
DynTypedNode const &Node, DiagnosticIDs::Level Level) { | ||
HerrCai0907 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!DiagMaps.contains(Level)) | ||
return; | ||
auto &DiagMap = DiagMaps.at(Level); | ||
if (!DiagMap.contains(BindName)) | ||
return; | ||
for (const std::string &Message : DiagMap.at(BindName)) { | ||
HerrCai0907 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
diag(Node.getSourceRange().getBegin(), Message, Level); | ||
} | ||
HerrCai0907 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
for (const auto &[Name, Node] : Result.Nodes.getMap()) | ||
Emit(Diags, Name, Node, DiagnosticIDs::Error); | ||
for (const auto &[Name, Node] : Result.Nodes.getMap()) | ||
Emit(Diags, Name, Node, DiagnosticIDs::Warning); | ||
// place Note last, otherwise it will not be emitted | ||
for (const auto &[Name, Node] : Result.Nodes.getMap()) | ||
Emit(Diags, Name, Node, DiagnosticIDs::Note); | ||
} | ||
} // namespace clang::tidy::custom |
Uh oh!
There was an error while loading. Please reload this page.