[clang-tools-extra] [clang-tidy] Add ClangQueryChecks config option (PR #123734)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 21 03:38:28 PST 2025
https://github.com/DeNiCoN created https://github.com/llvm/llvm-project/pull/123734
This addresses the #107680.
This pull request adds new config option that allows to define new checks using the [matchers](https://clang.llvm.org/docs/LibASTMatchersReference.html) syntax by incorporating clang-query parser
Example:
``` yaml
Checks: -*,custom-*
ClangQueryChecks:
custom-math: |
let expr callExpr(
callee(functionDecl(
hasAnyName("acos", "asin", "atan", "atan2", "cos", "sin", "tan", "cosh", "sinh", "tanh", "ctan"),
anyOf(
hasDeclContext(namespaceDecl(hasName("std"))),
unless(hasParent(namespaceDecl()))
)
))
).bind("Please use different math functions")
match expr
custom-sort: |
match callExpr(
callee(functionDecl(
hasAnyName("sort", "nth_element", "partial_sort", "partition"),
anyOf(
hasDeclContext(namespaceDecl(hasName("std"))),
unless(hasParent(namespaceDecl()))
)
))
).bind("Please use different sort functions")
custom-long: |
match varDecl(
hasType(asString("long")),
hasTypeLoc(typeLoc().bind("Please use int instead of long"))
)
```
## Unimplemented ideas
- Let queries could be made global so that checks defined at the bottom could reuse let expressions from the checks at the top. Currently let query is local to the defined check
- File queries are disabled. But they can be used to implement the initial idea of using the clang-query files for custom checks. But as I understand clang tools use VFS interface to access files which might not correspond to a real filesystem and I haven't explored deep how to access that interface during clang-tidy configuration parsing so I left it as a FIXME comment
## Some thoughts on further possible work
If [transformers](https://clang.llvm.org/docs/ClangTransformerTutorial.html) parser will be implemented in the future(it [seems](https://github.com/llvm/llvm-project/blob/7acad6893b9b3b43e5e4a8e56404b1b19c07c79f/clang/include/clang/Tooling/Transformer/Parsing.h#L11-L12) like in the past there were some work on it being done by @ymand) it can be added to clang-query and as a consequence it can be made that custom checks defined by this new option will support automatic fixes being provided
```
makeRule(declRefExpr(to(functionDecl(hasName("MkX")))),
changeTo(cat("MakeX")),
cat("MkX has been renamed MakeX"));
```
>From 148f94551d0660d434b6717c87cd68ec65ce6b82 Mon Sep 17 00:00:00 2001
From: DeNiCoN <denicon1234 at gmail.com>
Date: Tue, 14 Jan 2025 07:35:38 +0100
Subject: [PATCH] [clang-tidy] Add ClangQueryChecks config option
---
clang-tools-extra/clang-tidy/CMakeLists.txt | 2 +
.../clang-tidy/ClangQueryCheck.cpp | 30 +++++++
.../clang-tidy/ClangQueryCheck.h | 43 ++++++++++
clang-tools-extra/clang-tidy/ClangTidy.cpp | 8 ++
.../clang-tidy/ClangTidyOptions.cpp | 84 +++++++++++++++++++
.../clang-tidy/ClangTidyOptions.h | 8 ++
.../clang-tidy/tool/ClangTidyMain.cpp | 15 +++-
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
clang-tools-extra/docs/clang-tidy/index.rst | 12 +++
.../infrastructure/query-checks.cpp | 53 ++++++++++++
10 files changed, 257 insertions(+), 1 deletion(-)
create mode 100644 clang-tools-extra/clang-tidy/ClangQueryCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/ClangQueryCheck.h
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/query-checks.cpp
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 93117cf1d6373a..76585b012d1742 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -11,6 +11,7 @@ include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR})
add_clang_library(clangTidy STATIC
ClangTidy.cpp
ClangTidyCheck.cpp
+ ClangQueryCheck.cpp
ClangTidyModule.cpp
ClangTidyDiagnosticConsumer.cpp
ClangTidyOptions.cpp
@@ -38,6 +39,7 @@ clang_target_link_libraries(clangTidy
clangSerialization
clangTooling
clangToolingCore
+ clangQuery
)
if(CLANG_TIDY_ENABLE_STATIC_ANALYZER)
diff --git a/clang-tools-extra/clang-tidy/ClangQueryCheck.cpp b/clang-tools-extra/clang-tidy/ClangQueryCheck.cpp
new file mode 100644
index 00000000000000..a9f46116f70899
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/ClangQueryCheck.cpp
@@ -0,0 +1,30 @@
+//===--- ClangQueryCheck.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 "ClangQueryCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+void ClangQueryCheck::registerMatchers(MatchFinder *Finder) {
+ for (const auto &Matcher : Matchers) {
+ bool Ok = Finder->addDynamicMatcher(Matcher, this);
+ assert(Ok && "Expected to get top level matcher from query parser");
+ }
+}
+
+void ClangQueryCheck::check(const MatchFinder::MatchResult &Result) {
+ auto Map = Result.Nodes.getMap();
+ for (const auto &[k, v] : Map) {
+ diag(v.getSourceRange().getBegin(), k) << v.getSourceRange();
+ }
+}
+
+} // namespace clang::tidy::misc
diff --git a/clang-tools-extra/clang-tidy/ClangQueryCheck.h b/clang-tools-extra/clang-tidy/ClangQueryCheck.h
new file mode 100644
index 00000000000000..3c3c7029720687
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/ClangQueryCheck.h
@@ -0,0 +1,43 @@
+//===--- ClangQueryCheck.h - clang-tidy --------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGQUERYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGQUERYCHECK_H
+
+#include "ClangTidyCheck.h"
+#include "clang/ASTMatchers/Dynamic/VariantValue.h"
+#include <vector>
+
+namespace clang::query {
+class QuerySession;
+} // namespace clang::query
+
+namespace clang::tidy::misc {
+
+/// A check that matches a given matchers printing their binds as warnings
+class ClangQueryCheck : public ClangTidyCheck {
+ using MatcherVec = std::vector<ast_matchers::dynamic::DynTypedMatcher>;
+
+public:
+ ClangQueryCheck(StringRef Name, ClangTidyContext *Context,
+ MatcherVec Matchers)
+ : ClangTidyCheck(Name, Context), Matchers(std::move(Matchers)) {}
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+
+private:
+ MatcherVec Matchers;
+};
+
+} // namespace clang::tidy::misc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGQUERYCHECK_H
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 959b11777e88d4..75466b6612b253 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -15,6 +15,7 @@
//===----------------------------------------------------------------------===//
#include "ClangTidy.h"
+#include "ClangQueryCheck.h"
#include "ClangTidyCheck.h"
#include "ClangTidyDiagnosticConsumer.h"
#include "ClangTidyModuleRegistry.h"
@@ -350,6 +351,13 @@ ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
std::unique_ptr<ClangTidyModule> Module = E.instantiate();
Module->addCheckFactories(*CheckFactories);
}
+
+ for (const auto &[k, v] : Context.getOptions().ClangQueryChecks) {
+ CheckFactories->registerCheckFactory(k, [v](StringRef Name,
+ ClangTidyContext *Context) {
+ return std::make_unique<misc::ClangQueryCheck>(Name, Context, v.Matchers);
+ });
+ }
}
#if CLANG_TIDY_ENABLE_STATIC_ANALYZER
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 8bac6f161fa05b..0d6edb88a1d603 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "ClangTidyOptions.h"
+#include "../clang-query/Query.h"
+#include "../clang-query/QueryParser.h"
#include "ClangTidyModuleRegistry.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallString.h"
@@ -126,6 +128,83 @@ void yamlize(IO &IO, ClangTidyOptions::OptionMap &Val, bool,
}
}
+std::vector<clang::ast_matchers::dynamic::DynTypedMatcher>
+processQuerySource(IO &IO, StringRef SourceRef,
+ clang::query::QuerySession &QS) {
+ namespace query = clang::query;
+ std::vector<clang::ast_matchers::dynamic::DynTypedMatcher> Matchers;
+
+ while (!SourceRef.empty()) {
+ query::QueryRef Q = query::QueryParser::parse(SourceRef, 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);
+ for (const auto &Line : llvm::split(InvalidQuerry.ErrStr, "\n")) {
+ IO.setError(Line);
+ }
+ break;
+ }
+ // FIXME FileQuerry should also be supported, but what to do with relative
+ // paths?
+ 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: {
+ IO.setError("unsupported querry kind");
+ }
+ }
+ SourceRef = Q->RemainingContent;
+ }
+
+ return Matchers;
+}
+
+template <>
+void yamlize(IO &IO, ClangTidyOptions::QueryCheckMap &Val, bool,
+ EmptyContext &Ctx) {
+ IO.beginMapping();
+ if (IO.outputting()) {
+ for (auto &[k, v] : Val) {
+ IO.mapRequired(k.data(), v);
+ }
+ } else {
+ for (StringRef Key : IO.keys()) {
+ IO.mapRequired(Key.data(), Val[Key]);
+ }
+ }
+ IO.endMapping();
+}
+
+template <>
+void yamlize(IO &IO, ClangTidyOptions::QueryCheckValue &Val, bool,
+ EmptyContext &Ctx) {
+ if (IO.outputting()) {
+ StringRef SourceRef = Val.Source;
+ IO.blockScalarString(SourceRef);
+ } else {
+ StringRef SourceRef;
+ IO.blockScalarString(SourceRef);
+ Val.Source = SourceRef;
+ clang::query::QuerySession QS({});
+ Val.Matchers = processQuerySource(IO, SourceRef, QS);
+ }
+}
+
struct ChecksVariant {
std::optional<std::string> AsString;
std::optional<std::vector<std::string>> AsVector;
@@ -181,6 +260,7 @@ template <> struct MappingTraits<ClangTidyOptions> {
IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
IO.mapOptional("UseColor", Options.UseColor);
IO.mapOptional("SystemHeaders", Options.SystemHeaders);
+ IO.mapOptional("ClangQueryChecks", Options.ClangQueryChecks);
}
};
@@ -249,6 +329,10 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
ClangTidyValue(KeyValue.getValue().Value,
KeyValue.getValue().Priority + Order));
}
+
+ for (const auto &KeyValue : Other.ClangQueryChecks) {
+ ClangQueryChecks.insert_or_assign(KeyValue.getKey(), KeyValue.getValue());
+ }
return *this;
}
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index dd78c570d25d9b..3a015ed5163cdc 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -9,6 +9,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H
+#include "clang/ASTMatchers/Dynamic/VariantValue.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
@@ -126,8 +127,15 @@ struct ClangTidyOptions {
using StringPair = std::pair<std::string, std::string>;
using OptionMap = llvm::StringMap<ClangTidyValue>;
+ struct QueryCheckValue {
+ std::string Source;
+ std::vector<ast_matchers::dynamic::DynTypedMatcher> Matchers;
+ };
+ using QueryCheckMap = llvm::StringMap<QueryCheckValue>;
+
/// Key-value mapping used to store check-specific options.
OptionMap CheckOptions;
+ QueryCheckMap ClangQueryChecks;
using ArgList = std::vector<std::string>;
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index fa8887e4639b4a..c60e6fcb8c5fa7 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -60,6 +60,18 @@ Configuration files:
Checks - Same as '--checks'. Additionally, the list of
globs can be specified as a list instead of a
string.
+ ClangQueryChecks - List of key-value pairs. Key specifies a name
+ of the new check and value specifies a list
+ of matchers in the form of clang-query
+ syntax. Example:
+ ClangQueryChecks:
+ custom-check: |
+ let matcher varDecl(
+ hasTypeLoc(
+ typeLoc().bind("Custom message")
+ )
+ )
+ match matcher
ExcludeHeaderFilterRegex - Same as '--exclude-header-filter'.
ExtraArgs - Same as '--extra-arg'.
ExtraArgsBefore - Same as '--extra-arg-before'.
@@ -483,7 +495,8 @@ static StringRef closest(StringRef Value, const StringSet<> &Allowed) {
return Closest;
}
-static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n";
+static constexpr llvm::StringLiteral VerifyConfigWarningEnd =
+ " [-verify-config]\n";
static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
StringRef Source) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 33a452f525f763..b845b32569872c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,9 @@ Improvements to clang-tidy
:doc:`readability-redundant-access-specifiers <clang-tidy/checks/readability/redundant-access-specifiers>`, CheckFirstDeclaration
:doc:`readability-redundant-casting <clang-tidy/checks/readability/redundant-casting>`, IgnoreTypeAliases
+- New :program:`clang-tidy` config property `ClangQueryChecks` that allows adding
+ custom checks based on a :program:`clang-query` syntax
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index b7a366e874130e..7ad35aceb094f9 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -292,6 +292,18 @@ An overview of all the command-line options:
Checks - Same as '--checks'. Additionally, the list of
globs can be specified as a list instead of a
string.
+ ClangQueryChecks - List of key-value pairs. Key specifies a name
+ of the new check and value specifies a list
+ of matchers in the form of clang-query
+ syntax. Example:
+ ClangQueryChecks:
+ custom-check: |
+ let matcher varDecl(
+ hasTypeLoc(
+ typeLoc().bind("Custom message")
+ )
+ )
+ match matcher
ExcludeHeaderFilterRegex - Same as '--exclude-header-filter'.
ExtraArgs - Same as '--extra-arg'.
ExtraArgsBefore - Same as '--extra-arg-before'.
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/query-checks.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/query-checks.cpp
new file mode 100644
index 00000000000000..e84516a6977b77
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/query-checks.cpp
@@ -0,0 +1,53 @@
+// DEFINE: %{custom-call-yaml} = custom-call: 'm callExpr().bind(\"Custom message\")'
+//
+// DEFINE: %{custom-let-call-yaml} = custom-let-call: \" \
+// DEFINE: let expr varDecl( \
+// DEFINE: hasType(asString(\\\"long long\\\")), \
+// DEFINE: hasTypeLoc(typeLoc().bind(\\\"Let message\\\")) \
+// DEFINE: ) \n \
+// DEFINE: match expr\"
+//
+// DEFINE: %{full-config} = "{ClangQueryChecks: {%{custom-call-yaml},%{custom-let-call-yaml}}}"
+
+//Check single match expression
+// RUN: clang-tidy %s -checks='-*, custom-*' \
+// RUN: -config="{ClangQueryChecks: {%{custom-call-yaml}}}" \
+// RUN: -- | FileCheck %s -check-prefix=CHECK-CUSTOM-CALL
+
+void a() {
+}
+
+// CHECK-CUSTOM-CALL: warning: Custom message [custom-call]
+// CHECK-CUSTOM-CALL-NEXT: a();{{$}}
+void b() {
+ a();
+}
+
+//Check let with match expression
+// RUN: clang-tidy %s -checks='-*, custom-*' \
+// RUN: -config="{ClangQueryChecks: {%{custom-let-call-yaml}}}" \
+// RUN: -- | FileCheck %s -check-prefix=CHECK-CUSTOM-LET
+void c() {
+ // CHECK-CUSTOM-LET: warning: Let message [custom-let-call]
+ // CHECK-CUSTOM-LET-NEXT: long long test_long_long = 0;{{$}}
+ long long test_long_long_nolint = 0; //NOLINT(custom-let-call)
+ long long test_long_long = 0;
+}
+
+//Check multiple checks in one config
+// RUN: clang-tidy %s -checks='-*, custom-*' \
+// RUN: -config=%{full-config} \
+// RUN: -- | FileCheck %s -check-prefixes=CHECK-CUSTOM-CALL,CHECK-CUSTOM-LET
+
+//Check multiple checks in one config but only one enabled
+// RUN: clang-tidy %s -checks='-*, custom-call' \
+// RUN: -config=%{full-config} \
+// RUN: -- | FileCheck %s -check-prefixes=CHECK-CUSTOM-CALL --implicit-check-not warning:
+
+//Check config dump
+// RUN: clang-tidy -dump-config -checks='-*, custom-*' \
+// RUN: -config=%{full-config} \
+// RUN: -- | FileCheck %s -check-prefix=CHECK-CONFIG
+// CHECK-CONFIG: ClangQueryChecks:
+// CHECK-CONFIG-DAG: custom-let-call:
+// CHECK-CONFIG-DAG: custom-call: |{{$[[:space:]]}} m callExpr().bind("Custom message")
More information about the cfe-commits
mailing list