mirror
/
mongo
mirror of https://github.com/mongodb/mongo
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SERVER-96684 update mongo tidy checks to support LLVM 19 API, switch to v5 clang-tidy (#33414) 

GitOrigin-RevId: 557fbe7c6c3774dfab67b5e786799eacf9cd3faa
This commit is contained in:
James Bronsted 2025-03-12 18:29:40 -04:00 committed by MongoDB Bot
parent 4335bc5075
commit 07046bb4f5
26 changed files with 173 additions and 223 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
.bazelrc
View File

@ -330,7 +330,6 @@ common --remote_upload_local_results=False
# Settings specific for clang-tidy
--config=clang-tidy
common:clang-tidy --//bazel/config:clang_tidy_toolchain_version=v4
common:clang-tidy --config=dbg
common:clang-tidy --build_tag_filters=-third_party,-mongo-tidy-tests
common:clang-tidy --//bazel/config:compiler_type=clang
@ -341,17 +340,6 @@ common:clang-tidy --@bazel_clang_tidy//:clang_tidy_executable=//:clang_tidy
common:clang-tidy --@bazel_clang_tidy//:clang_tidy_additional_deps=//:toolchain_files
common:clang-tidy --@bazel_clang_tidy//:clang_tidy_plugin_deps=//src/mongo/tools/mongo_tidy_checks:mongo_tidy_checks
# clang-tidy config using the v4 toolchain
--config=clang-tidy-v4
common:clang-tidy-v4 --config=clang-tidy
common:clang-tidy --//bazel/config:mongo_toolchain_version=v4
# clang-tidy config using the v5 toolchain
--config=clang-tidy-v5
common:clang-tidy-v5 --config=clang-tidy
common:clang-tidy-v5 --//bazel/config:mongo_toolchain_version=v5
common:clang-tidy-v5 --//bazel/config:clang_tidy_toolchain_version=v5
--config=mod-scanner
common:mod-scanner --config=dbg
common:mod-scanner --build_tag_filters=-third_party,-mongo-tidy-tests

1
.clang-tidy.in
View File

@ -74,7 +74,6 @@ Checks: '-*,
mongo-invariant-status-is-ok-check,
mongo-macro-definition-leaks-check,
mongo-mutex-check,
mongo-polyfill-check,
mongo-rand-check,
mongo-std-atomic-check,
mongo-std-optional-check,

63
bazel/config/BUILD.bazel
View File

@ -6,7 +6,6 @@ load(
"asan",
"build_enterprise",
"build_otel",
"clang_tidy_toolchain_version",
"compiler_type",
"compress_debug_compile",
"dbg",
@ -343,77 +342,19 @@ mongo_toolchain_version(
)
config_setting(
name = "mongo_toolchain_v4_user",
name = "mongo_toolchain_v4",
flag_values = {
"//bazel/config:mongo_toolchain_version": "v4",
},
)
config_setting(
name = "mongo_toolchain_v5_user",
name = "mongo_toolchain_v5",
flag_values = {
"//bazel/config:mongo_toolchain_version": "v5",
},
)
clang_tidy_toolchain_version(
name = "clang_tidy_toolchain_version",
build_setting_default = "auto",
)
config_setting(
name = "clang_tidy_toolchain_version_auto",
flag_values = {
"//bazel/config:clang_tidy_toolchain_version": "auto",
},
)
config_setting(
name = "clang_tidy_toolchain_version_v4",
flag_values = {
"//bazel/config:clang_tidy_toolchain_version": "v4",
},
)
config_setting(
name = "clang_tidy_toolchain_version_v5",
flag_values = {
"//bazel/config:clang_tidy_toolchain_version": "v5",
},
)
selects.config_setting_group(
name = "clang_tidy_toolchain_version_auto_or_v5",
match_any = [
":clang_tidy_toolchain_version_auto",
":clang_tidy_toolchain_version_v5",
],
)
selects.config_setting_group(
name = "mongo_toolchain_v4_override",
match_all = [
":mongo_toolchain_v5_user",
":clang_tidy_toolchain_version_v4",
],
)
selects.config_setting_group(
name = "mongo_toolchain_v4",
match_any = [
":mongo_toolchain_v4_user",
":mongo_toolchain_v4_override",
],
)
selects.config_setting_group(
name = "mongo_toolchain_v5",
match_all = [
":mongo_toolchain_v5_user",
":clang_tidy_toolchain_version_auto_or_v5",
],
)
# ==========
# opt
# ==========

22
bazel/config/configs.bzl
View File

@ -682,25 +682,3 @@ server_js = rule(
implementation = lambda ctx: server_js_provider(enabled = ctx.build_setting_value),
build_setting = config.bool(flag = True),
)
# ============================
# clang_tidy_toolchain_version
# ============================
clang_tidy_toolchain_version_values = ["auto", "v4", "v5"]
clang_tidy_toolchain_version_provider = provider(
doc = "Choose which toolchain we use for clang-tidy",
fields = {"clang_tidy_toolchain_version": "Choose one of " + ", ".join(clang_tidy_toolchain_version_values)},
)
def clang_tidy_toolchain_version_impl(ctx):
clang_tidy_toolchain_version_value = ctx.build_setting_value
if clang_tidy_toolchain_version_value not in clang_tidy_toolchain_version_values:
fail(str(ctx.label) + "clang_tidy_toolchain_version allowed to take values {" + ", ".join(clang_tidy_toolchain_version_values) + "} but was set to unallowed value " + clang_tidy_toolchain_version_value)
return clang_tidy_toolchain_version_provider(clang_tidy_toolchain_version = clang_tidy_toolchain_version_value)
clang_tidy_toolchain_version = rule(
implementation = clang_tidy_toolchain_version_impl,
build_setting = config.string(flag = True),
)

2
etc/evergreen_yml_components/tasks/compile_tasks.yml
View File

@ -379,7 +379,7 @@ tasks:
vars:
# Note that we only run clang-tidy on the Mongo-authored targets, not on third-party code.
targets: //src/mongo/...
args: --config=clang-tidy-${clang_tidy_toolchain|v4} --mongo_toolchain_version=${clang_tidy_toolchain|v4} --keep_going
args: --config=clang-tidy-${clang_tidy_toolchain|v5} --mongo_toolchain_version=${clang_tidy_toolchain|v5} --keep_going
run_for_clang_tidy: "true"
- func: "generate clang-tidy report"

6
etc/evergreen_yml_components/variants/amazon/test_dev.yml
View File

@ -98,7 +98,7 @@ buildvariants:
tags: ["required", "bazel_check", "forbid_tasks_tagged_with_experimental"]
expansions:
<<: *linux-arm64-static-enterprise-compile-expansions
clang_tidy_toolchain: v4
clang_tidy_toolchain: v5
build_mongot: true
download_mongot_release: true
compile_variant: *amazon-linux2023-arm64-static-compile
@ -207,7 +207,7 @@ buildvariants:
SHLINKFLAGS_EXTRA="-Wl,-Bsymbolic
-Wl,--no-gnu-unique"
CCFLAGS="-fno-gnu-unique"
clang_tidy_toolchain: v4
clang_tidy_toolchain: v5
num_scons_unit_cc_jobs_available: 0.75
compile_variant: *commit-queue
depends_on: []
@ -257,7 +257,7 @@ buildvariants:
SHLINKFLAGS_EXTRA="-Wl,-Bsymbolic
-Wl,--no-gnu-unique"
CCFLAGS="-fno-gnu-unique"
clang_tidy_toolchain: v4
clang_tidy_toolchain: v5
num_scons_unit_cc_jobs_available: 0.75
compile_variant: *commit-queue-toolchain-v5
depends_on: []

2
etc/evergreen_yml_components/variants/amazon/test_dev_master_branch_only.yml
View File

@ -586,7 +586,7 @@ buildvariants:
--link-model=dynamic
unittest_compile_flags: >-
--link-model=dynamic
clang_tidy_toolchain: v4
clang_tidy_toolchain: v5
build_mongot: true
download_mongot_release: true
compile_variant: *amazon-linux2023-arm64-static-compile-toolchain-v5

2
etc/evergreen_yml_components/variants/rhel/test_dev_master_branch_only.yml
View File

@ -259,7 +259,7 @@ buildvariants:
--variables-files=etc/scons/mongodbtoolchain_stable_gcc.vars
--link-model=dynamic
compile_variant: *linux-x86-dynamic-grpc
clang_tidy_toolchain: v4
clang_tidy_toolchain: v5
large_distro_name: rhel8.8-xlarge
test_flags: >-
--additionalFeatureFlags "featureFlagGRPC"

5
src/mongo/tools/mongo_tidy_checks/BUILD.bazel
View File

@ -91,7 +91,7 @@ cc_library(
"-Woverloaded-virtual",
"-Wwrite-strings",
"-Wsuggest-override",
"-Iexternal/mongo_toolchain_v4/stow/llvm-v4/include/",
"-Iexternal/mongo_toolchain_v5/stow/llvm-v5/include",
],
local_defines = [
"GTEST_HAS_RTTI=0",
@ -109,7 +109,7 @@ cc_library(
"//conditions:default": [],
}),
deps = select({
"@//bazel/platforms:use_mongo_toolchain": ["@mongo_toolchain_v4//:llvm_headers"],
"@//bazel/platforms:use_mongo_toolchain": ["@mongo_toolchain_v5//:llvm_headers"],
"//conditions:default": [],
}),
)
@ -158,6 +158,7 @@ cc_shared_library(
"-Wl,-O3",
"-Wl,--gc-sections",
"-Wl,-rpath,\\$ORIGIN/../lib", # Ensure that the shared library can find its dependencies.
"-Wl,-rpath-link,external/mongo_toolchain_v5/stow/llvm-v5/lib",
],
deps = [
":mongo_tidy_checks_static",

68
src/mongo/tools/mongo_tidy_checks/MongoAssertCheck.cpp
View File

@ -29,57 +29,41 @@
#include "MongoAssertCheck.h"
#include <clang/Lex/Lexer.h>
#include <clang/Lex/PPCallbacks.h>
#include <clang/Lex/Preprocessor.h>
namespace mongo::tidy {
using namespace clang;
using namespace clang::ast_matchers;
// Callbacks for handling preprocessor events
class MongoAssertMacroPPCallbacks : public clang::PPCallbacks {
public:
explicit MongoAssertMacroPPCallbacks(MongoAssertCheck& Check) : Check(Check) {}
void MacroExpands(const clang::Token& MacroNameTok,
const clang::MacroDefinition& MD,
SourceRange Range,
const MacroArgs* Args) override {
if (MacroNameTok.getIdentifierInfo()->getName() == "assert") {
Check.diag(Range.getBegin(),
"Illegal use of the bare assert macro, use a macro function from "
"assert_util.h instead");
}
}
private:
MongoAssertCheck& Check;
};
MongoAssertCheck::MongoAssertCheck(StringRef Name, clang::tidy::ClangTidyContext* Context)
: ClangTidyCheck(Name, Context) {}
// Custom AST Matcher that checks if an expression is an 'assert' macro expansion
AST_MATCHER(Expr, isAssertMacroExpansion) {
// Return false if the expression is not a macro expansion
if (!Node.getBeginLoc().isMacroID()) {
return false;
}
auto& SM = Finder->getASTContext().getSourceManager();
auto MacroLoc = SM.getImmediateMacroCallerLoc(Node.getBeginLoc());
// Get the name of the macro being expanded
llvm::StringRef MacroSpellingRef =
clang::Lexer::getImmediateMacroName(MacroLoc, SM, Finder->getASTContext().getLangOpts());
std::string Spelling = MacroSpellingRef.str();
if (Spelling == "assert") {
// Check if the file name contains "assert.h"
// <cassert> will also be caught because it redirects to assert.h
auto FileName = SM.getFilename(SM.getSpellingLoc(MacroLoc));
return llvm::StringRef(FileName).contains("assert.h") &&
!llvm::StringRef(FileName).contains("src/mongo/unittest/assert.h");
}
return false;
}
void MongoAssertCheck::registerMatchers(ast_matchers::MatchFinder* Finder) {
// Add the custom matcher 'isAssertMacroExpansion' to the MatchFinder
Finder->addMatcher(expr(isAssertMacroExpansion()).bind("assertMacroExpansion"), this);
}
void MongoAssertCheck::check(const ast_matchers::MatchFinder::MatchResult& Result) {
// Check assert macro expansion
const auto* AssertMacroExpansion = Result.Nodes.getNodeAs<Expr>("assertMacroExpansion");
if (AssertMacroExpansion) {
diag(AssertMacroExpansion->getBeginLoc(),
"Illegal use of the bare assert function, use a function from assert_util.h instead");
}
void MongoAssertCheck::registerPPCallbacks(const clang::SourceManager& SM,
clang::Preprocessor* PP,
clang::Preprocessor* ModuleExpanderpp) {
PP->addPPCallbacks(std::make_unique<MongoAssertMacroPPCallbacks>(*this));
}
} // namespace mongo::tidy

5
src/mongo/tools/mongo_tidy_checks/MongoAssertCheck.h
View File

@ -46,8 +46,9 @@ class MongoAssertCheck : public clang::tidy::ClangTidyCheck {
public:
MongoAssertCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context);
void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override;
void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override;
void registerPPCallbacks(const clang::SourceManager& SM,
clang::Preprocessor* PP,
clang::Preprocessor* ModuleExpanderpp) override;
};
} // namespace mongo::tidy

7
src/mongo/tools/mongo_tidy_checks/MongoCctypeCheck.cpp
View File

@ -47,16 +47,17 @@ public:
llvm::StringRef FileName,
bool IsAngled,
clang::CharSourceRange FilenameRange,
const clang::FileEntry* File,
clang::OptionalFileEntryRef File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
const clang::Module* Imported,
const clang::Module* SuggestedModule,
bool ModuleImported,
clang::SrcMgr::CharacteristicKind FileType) override {
// match following cases
// #include <cctype>
// #include <ctype.h>
if (FileName.equals("cctype") || FileName.equals("ctype.h")) {
if (FileName == "cctype" || FileName == "ctype.h") {
std::string Replacement = "\"mongo/util/ctype.h\"";
Check.diag(FilenameRange.getBegin(),
"Use of prohibited %0%1%2 header, use \"mongo/util/ctype.h\"")

10
src/mongo/tools/mongo_tidy_checks/MongoCollectionShardingRuntimeCheck.cpp
View File

@ -47,12 +47,14 @@ MongoCollectionShardingRuntimeCheck::MongoCollectionShardingRuntimeCheck(
// Accept relative paths
int origLength = static_cast<int>(exceptionDirs.size());
for (int i = 0; i < origLength; i++) {
exceptionDirs.push_back(std::filesystem::current_path() / exceptionDirs[i]);
exceptionDirs.push_back(
llvm::StringRef(std::filesystem::current_path() / exceptionDirs[i].str()));
}
origLength = static_cast<int>(exceptionFiles.size());
for (int i = 0; i < origLength; i++) {
exceptionFiles.push_back(std::filesystem::current_path() / exceptionFiles[i]);
exceptionFiles.push_back(
llvm::StringRef(std::filesystem::current_path() / exceptionFiles[i].str()));
}
}
@ -97,14 +99,14 @@ void MongoCollectionShardingRuntimeCheck::check(const MatchFinder::MatchResult&
// If the file path is in an exception directory, skip the check.
for (const auto& dir : this->exceptionDirs) {
if (FilePath.startswith(dir)) {
if (FilePath.contains(dir)) {
return;
}
}
// If the file path is one of the exception files, skip the check.
for (const auto& file : this->exceptionFiles) {
if (FilePath.equals(file)) {
if (FilePath == file) {
return;
}
}

4
src/mongo/tools/mongo_tidy_checks/MongoCollectionShardingRuntimeCheck.h
View File

@ -53,8 +53,8 @@ public:
clang::tidy::ClangTidyContext* Context);
void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override;
void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override;
std::vector<std::string> exceptionDirs;
std::vector<std::string> exceptionFiles;
std::vector<llvm::StringRef> exceptionDirs;
std::vector<llvm::StringRef> exceptionFiles;
};
} // namespace mongo::tidy

9
src/mongo/tools/mongo_tidy_checks/MongoConfigHeaderCheck.cpp
View File

@ -50,7 +50,7 @@ public:
if (ConfigHeaderIncluded)
return;
llvm::StringRef macroName = MacroNameTok.getIdentifierInfo()->getName();
if (macroName.startswith("MONGO_CONFIG_")) {
if (macroName.starts_with("MONGO_CONFIG_")) {
Check.diag(Loc, "MONGO_CONFIG define used without prior inclusion of config.h");
}
}
@ -110,13 +110,14 @@ public:
llvm::StringRef FileName,
bool IsAngled,
clang::CharSourceRange FilenameRange,
const clang::FileEntry* File,
clang::OptionalFileEntryRef File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
const clang::Module* Imported,
const clang::Module* SuggestedModule,
bool ModuleImported,
clang::SrcMgr::CharacteristicKind FileType) override {
if (FileName.equals("mongo/config.h")) {
if (FileName == "mongo/config.h") {
ConfigHeaderIncluded = true;
}
}

12
src/mongo/tools/mongo_tidy_checks/MongoCxx20BannedIncludesCheck.cpp
View File

@ -48,22 +48,22 @@ public:
llvm::StringRef FileName,
bool IsAngled,
clang::CharSourceRange FilenameRange,
const clang::FileEntry* File,
clang::OptionalFileEntryRef File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
const clang::Module* Imported,
const clang::Module* SuggestedModule,
bool ModuleImported,
clang::SrcMgr::CharacteristicKind FileType) override {
if (FileName.equals("coroutine") || FileName.equals("format")) {
if (FileName == "coroutine" || FileName == "format") {
Check.diag(FilenameRange.getBegin(),
"Use of prohibited %0%1%2 header. There are no override waivers issued for "
"this header. For questions please reach out to #cxx-discuss.")
<< (IsAngled ? "<" : "\"") << FileName << (IsAngled ? ">" : "\"");
}
if (FileName.equals("syncstream") || FileName.equals("ranges") ||
FileName.equals("barrier") || FileName.equals("latch") ||
FileName.equals("semaphore")) {
if (FileName == "syncstream" || FileName == "ranges" || FileName == "barrier" ||
FileName == "latch" || FileName == "semaphore") {
Check.diag(FilenameRange.getBegin(),
"Use of prohibited %0%1%2 header. There are override waivers issued for "
"this header. You need to follow the process in PM-3140 to override this "

33
src/mongo/tools/mongo_tidy_checks/MongoHeaderBracketCheck.cpp
View File

@ -42,9 +42,11 @@ namespace mongo::tidy {
namespace {
bool startsWithAny(llvm::StringRef fullString, std::vector<std::string> const& startings) {
return std::any_of(startings.cbegin(), startings.cend(), [&](llvm::StringRef starting) {
return fullString.startswith(starting);
bool containsAny(llvm::StringRef fullString, clang::ArrayRef<clang::StringRef> const& patterns) {
return std::any_of(patterns.begin(), patterns.end(), [&](llvm::StringRef pattern) {
// Ensure the pattern ends with a directory delimiter to avoid matching partial names.
auto patternStr = pattern.str();
return fullString.contains(llvm::StringRef(std::filesystem::path(patternStr) / ""));
});
}
@ -61,10 +63,11 @@ public:
llvm::StringRef FileName,
bool IsAngled,
clang::CharSourceRange FilenameRange,
const clang::FileEntry* File,
clang::OptionalFileEntryRef File,
llvm::StringRef SearchPath,
llvm::StringRef RelativePath,
const clang::Module* Imported,
const clang::Module* SuggestedModule,
bool ModuleImported,
clang::SrcMgr::CharacteristicKind FileType) override {
// This represents the full path from the project root directory
@ -82,13 +85,13 @@ public:
}
// if this is a mongo source file including a real file
if (!llvm::StringRef(origin_source_path).startswith("<built-in>:") &&
startsWithAny(llvm::StringRef(origin_source_path.lexically_normal()),
if (!llvm::StringRef(origin_source_path).starts_with("<built-in>:") &&
containsAny(llvm::StringRef(origin_source_path.lexically_normal()),
Check.mongoSourceDirs)) {
// Check that the a third party header which is included from a mongo source file
// is used angle brackets.
if (!IsAngled && !startsWithAny(llvm::StringRef(header_path), Check.mongoSourceDirs)) {
if (!IsAngled && !containsAny(llvm::StringRef(header_path), Check.mongoSourceDirs)) {
std::string Replacement = (llvm::Twine("<") + FileName + ">").str();
Check.diag(FilenameRange.getBegin(),
@ -99,9 +102,9 @@ public:
// Check that the a third party header which is in our vendored tree is not including
// the third_party in the include path.
if (!startsWithAny(llvm::StringRef(header_path), Check.mongoSourceDirs) &&
llvm::StringRef(header_path).startswith("src/third_party") &&
FileName.startswith("third_party")) {
if (!containsAny(llvm::StringRef(header_path), Check.mongoSourceDirs) &&
llvm::StringRef(header_path).contains("src/third_party/") &&
FileName.contains("third_party/")) {
Check.diag(FilenameRange.getBegin(),
"third_party include '%0' should not start with 'third_party/'. The "
"included file should be useable in either context of system or "
@ -111,11 +114,10 @@ public:
// Check that the a mongo header which is included from a mongo source file
// is used double quotes.
else if (IsAngled &&
startsWithAny(llvm::StringRef(header_path), Check.mongoSourceDirs)) {
else if (IsAngled && containsAny(llvm::StringRef(header_path), Check.mongoSourceDirs)) {
// TODO(SERVER-95253): Explicitly handle third party headers inside of the
// enterprise module directory.
if (!FileName.startswith("bsoncxx/") && !FileName.startswith("mongocxx/")) {
if (!FileName.contains("bsoncxx/") && !FileName.contains("mongocxx/")) {
std::string Replacement = (llvm::Twine("\"") + FileName + "\"").str();
Check.diag(FilenameRange.getBegin(),
"mongo include '%0' should use double quotes")
@ -143,7 +145,8 @@ MongoHeaderBracketCheck::MongoHeaderBracketCheck(llvm::StringRef Name,
// Accept relative paths
int origLength = static_cast<int>(mongoSourceDirs.size());
for (int i = 0; i < origLength; i++) {
mongoSourceDirs.push_back(std::filesystem::current_path() / mongoSourceDirs[i]);
mongoSourceDirs.push_back(
llvm::StringRef(std::filesystem::current_path() / mongoSourceDirs[i].str()));
}
}

2
src/mongo/tools/mongo_tidy_checks/MongoHeaderBracketCheck.h
View File

@ -46,7 +46,7 @@ public:
void registerPPCallbacks(const clang::SourceManager& SM,
clang::Preprocessor* PP,
clang::Preprocessor* ModuleExpanderPP) override;
std::vector<std::string> mongoSourceDirs;
std::vector<llvm::StringRef> mongoSourceDirs;
};
} // namespace mongo::tidy

3
src/mongo/tools/mongo_tidy_checks/MongoMacroDefinitionLeaksCheck.cpp
View File

@ -32,6 +32,7 @@
#include <clang/Lex/PPCallbacks.h>
#include <clang/Lex/Preprocessor.h>
#include <stack>
namespace mongo::tidy {
@ -80,7 +81,7 @@ public:
if (Reason == EnterFile) {
// Push the file to the stack
fileStack.push(CurrentFile->getName().str());
fileStack.push(CurrentFile->tryGetRealPathName().str());
defineUndefDiff = 0;
} else if (Reason == ExitFile && !fileStack.empty()) {
// Get the top file from the stack

2
src/mongo/tools/mongo_tidy_checks/MongoNoUniqueAddressCheck.cpp
View File

@ -57,7 +57,7 @@ void MongoNoUniqueAddressCheck::check(const ast_matchers::MatchFinder::MatchResu
auto charRange =
Lexer::getAsCharRange(sm.getExpansionRange(attr->getLocation()), sm, lo);
auto originalMacro = Lexer::getSourceText(charRange, sm, lo);
if (originalMacro.equals("MONGO_COMPILER_NO_UNIQUE_ADDRESS")) {
if (originalMacro == "MONGO_COMPILER_NO_UNIQUE_ADDRESS") {
return;
}
}

4
src/mongo/tools/mongo_tidy_checks/MongoStdAtomicCheck.cpp
View File

@ -55,8 +55,8 @@ void MongoStdAtomicCheck::check(const ast_matchers::MatchFinder::MatchResult& Re
const auto* loc_match = Result.Nodes.getNodeAs<TypeLoc>("loc_atomic");
if (loc_match) {
diag(loc_match->getBeginLoc(),
"Illegal use of prohibited std::atomic<T>, use AtomicWord<T> or other types from "
"\"mongo/platform/atomic_word.h\"");
"Illegal use of prohibited std::atomic<T>, use Atomic<T> or other types from "
"\"mongo/platform/atomic.h\"");
}
}

2
src/mongo/tools/mongo_tidy_checks/MongoUninterruptibleLockGuardCheck.cpp
View File

@ -39,7 +39,7 @@ static constexpr auto kBannedMember = "uninterruptibleLocksRequested_DO_NOT_USE"
AST_MATCHER(clang::CXXMemberCallExpr, matchesForbiddenCall) {
if (auto memberDecl = Node.getMethodDecl()) {
if (auto identifierInfo = memberDecl->getDeclName().getAsIdentifierInfo()) {
return identifierInfo->getName().equals(kBannedMember);
return identifierInfo->getName() == kBannedMember;
}
}
return false;

8
src/mongo/tools/mongo_tidy_checks/MongoVolatileCheck.cpp
View File

@ -52,13 +52,13 @@ void MongoVolatileCheck::check(const MatchFinder::MatchResult& Result) {
if (var_match) {
diag(var_match->getBeginLoc(),
"Illegal use of the volatile storage keyword, use AtomicWord instead from "
"\"mongo/platform/atomic_word.h\"");
"Illegal use of the volatile storage keyword, use Atomic instead from "
"\"mongo/platform/atomic.h\"");
}
if (field_match) {
diag(field_match->getBeginLoc(),
"Illegal use of the volatile storage keyword, use AtomicWord instead from "
"\"mongo/platform/atomic_word.h\"");
"Illegal use of the volatile storage keyword, use Atomic instead from "
"\"mongo/platform/atomic.h\"");
}
}

2
src/mongo/tools/mongo_tidy_checks/SConscript
View File

@ -73,7 +73,6 @@ env["CCFLAGS"] = [
"-fno-strict-aliasing",
"-O3",
"-g2",
"-DNDEBUG",
"-fPIC",
"-fno-exceptions",
"-fno-rtti",
@ -107,7 +106,6 @@ env["SHLINKFLAGS"] = [
"-Woverloaded-virtual",
"-fno-strict-aliasing",
"-O3",
"-DNDEBUG",
"-Wl,-z,nodelete",
f"-Wl,-rpath-link,{tidy_lib}",
"-Wl,-O3",

104
src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py
View File

@ -193,14 +193,19 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = "Use of std::optional, use boost::optional instead."
opt = "mongo-std-optional-check,-warnings-as-errors"
self.expected_output = [
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\nvoid f(std::optional<std::string> parameterDeclTest) {",
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\n std::optional<std::string> variableDeclTest;",
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\n std::optional<int> fieldDeclTest = 5;",
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\nvoid functionName(const std::optional<int>& referenceDeclTest) {",
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\nstd::optional<std::string> functionReturnTypeDeclTest(StringData name);",
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\nstd::optional<T> templateDeclTest;",
"Use of std::optional, use boost::optional instead. [mongo-std-optional-check,-warnings-as-errors]\nusing std::optional;",
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(36, "void f(std::optional<std::string> parameterDeclTest) {"),
(37, " std::optional<std::string> variableDeclTest;"),
(42, " std::optional<int> fieldDeclTest = 5;"),
(46, "void functionName(const std::optional<int>& referenceDeclTest) {"),
(52, "std::optional<std::string> functionReturnTypeDeclTest(StringData name);"),
(55, "std::optional<T> templateDeclTest;"),
(57, "using std::optional;"),
]
]
self.run_clang_tidy()
@ -213,10 +218,15 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = 'Illegal use of the volatile storage keyword, use Atomic instead from "mongo/platform/atomic.h"'
opt = "mongo-volatile-check,-warnings-as-errors"
self.expected_output = [
'Illegal use of the volatile storage keyword, use AtomicWord instead from "mongo/platform/atomic_word.h" [mongo-volatile-check,-warnings-as-errors]\nvolatile int varVolatileTest;',
'Illegal use of the volatile storage keyword, use AtomicWord instead from "mongo/platform/atomic_word.h" [mongo-volatile-check,-warnings-as-errors]\n volatile int fieldVolatileTest;',
'Illegal use of the volatile storage keyword, use AtomicWord instead from "mongo/platform/atomic_word.h" [mongo-volatile-check,-warnings-as-errors]\nvoid functionName(volatile int varVolatileTest) {}',
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(31, "volatile int varVolatileTest;"),
(33, " volatile int fieldVolatileTest;"),
(35, "void functionName(volatile int varVolatileTest) {}"),
]
]
self.run_clang_tidy()
@ -229,9 +239,14 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = "Illegal use of prohibited tracing support, this is only for local development use and should not be committed."
opt = "mongo-trace-check,-warnings-as-errors"
self.expected_output = [
"Illegal use of prohibited tracing support, this is only for local development use and should not be committed. [mongo-trace-check,-warnings-as-errors]\n TracerProvider::initialize();",
"Illegal use of prohibited tracing support, this is only for local development use and should not be committed. [mongo-trace-check,-warnings-as-errors]\n TracerProvider provider = TracerProvider::get();",
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(12, " TracerProvider::initialize();"),
(13, " TracerProvider provider = TracerProvider::get();"),
]
]
self.run_clang_tidy()
@ -244,9 +259,14 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = 'Illegal use of prohibited std::atomic<T>, use Atomic<T> or other types from "mongo/platform/atomic.h"'
opt = "mongo-std-atomic-check,-warnings-as-errors"
self.expected_output = [
'Illegal use of prohibited std::atomic<T>, use AtomicWord<T> or other types from "mongo/platform/atomic_word.h" [mongo-std-atomic-check,-warnings-as-errors]\nstd::atomic<int> atomic_var;',
'Illegal use of prohibited std::atomic<T>, use AtomicWord<T> or other types from "mongo/platform/atomic_word.h" [mongo-std-atomic-check,-warnings-as-errors]\n std::atomic<int> field_decl;',
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(6, "std::atomic<int> atomic_var;"),
(10, " std::atomic<int> field_decl;"),
]
]
self.run_clang_tidy()
@ -260,7 +280,7 @@ class MongoTidyTests(unittest.TestCase):
)
self.expected_output = [
"error: Illegal use of the bare assert function, use a function from assert_util.h instead",
"error: Illegal use of the bare assert macro, use a macro function from assert_util.h instead",
]
self.run_clang_tidy()
@ -287,9 +307,14 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = "error: Illegal use of unstructured logging, this is only for local development use and should not be committed"
opt = "mongo-unstructured-log-check,-warnings-as-errors"
self.expected_output = [
"error: Illegal use of unstructured logging, this is only for local development use and should not be committed [mongo-unstructured-log-check,-warnings-as-errors]\n logd();",
"error: Illegal use of unstructured logging, this is only for local development use and should not be committed [mongo-unstructured-log-check,-warnings-as-errors]\n doUnstructuredLogImpl();",
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(11, " logd();"),
(12, " doUnstructuredLogImpl();"),
]
]
self.run_clang_tidy()
@ -303,12 +328,17 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = "error: MONGO_CONFIG define used without prior inclusion of config.h"
opt = "mongo-config-header-check,-warnings-as-errors"
self.expected_output = [
"error: MONGO_CONFIG define used without prior inclusion of config.h [mongo-config-header-check,-warnings-as-errors]\n#define MONGO_CONFIG_TEST1 1",
"error: MONGO_CONFIG define used without prior inclusion of config.h [mongo-config-header-check,-warnings-as-errors]\n#ifdef MONGO_CONFIG_TEST1",
"error: MONGO_CONFIG define used without prior inclusion of config.h [mongo-config-header-check,-warnings-as-errors]\n#if MONGO_CONFIG_TEST1 == 1",
"error: MONGO_CONFIG define used without prior inclusion of config.h [mongo-config-header-check,-warnings-as-errors]\n#ifndef MONGO_CONFIG_TEST2",
"error: MONGO_CONFIG define used without prior inclusion of config.h [mongo-config-header-check,-warnings-as-errors]\n#if defined(MONGO_CONFIG_TEST1)",
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(4, "#define MONGO_CONFIG_TEST1 1"),
(6, "#ifdef MONGO_CONFIG_TEST1"),
(9, "#if MONGO_CONFIG_TEST1 == 1"),
(12, "#ifndef MONGO_CONFIG_TEST2"),
(15, "#if defined(MONGO_CONFIG_TEST1)"),
]
]
self.run_clang_tidy()
@ -323,9 +353,17 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = (
"error: Illegal use of CollectionShardingRuntime outside of mongo/db/s/; "
"use CollectionShardingState instead; see src/mongo/db/s/collection_sharding_state.h for details."
)
opt = "mongo-collection-sharding-runtime-check,-warnings-as-errors"
self.expected_output = [
'error: Illegal use of CollectionShardingRuntime outside of mongo/db/s/; use CollectionShardingState instead; see src/mongo/db/s/collection_sharding_state.h for details. [mongo-collection-sharding-runtime-check,-warnings-as-errors]\n CollectionShardingRuntime csr(5, "Test");',
'error: Illegal use of CollectionShardingRuntime outside of mongo/db/s/; use CollectionShardingState instead; see src/mongo/db/s/collection_sharding_state.h for details. [mongo-collection-sharding-runtime-check,-warnings-as-errors]\n int result = CollectionShardingRuntime::functionTest(7, "Test");',
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(21, ' CollectionShardingRuntime csr(5, "Test");'),
(24, ' int result = CollectionShardingRuntime::functionTest(7, "Test");'),
]
]
self.run_clang_tidy()
@ -386,9 +424,14 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = "error: Use of rand or srand, use <random> or PseudoRandom instead."
opt = "mongo-rand-check,-warnings-as-errors"
self.expected_output = [
"error: Use of rand or srand, use <random> or PseudoRandom instead. [mongo-rand-check,-warnings-as-errors]\n srand(time(0));",
"error: Use of rand or srand, use <random> or PseudoRandom instead. [mongo-rand-check,-warnings-as-errors]\n int random_number = rand();",
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(5, " srand(time(0));"),
(6, " int random_number = rand();"),
]
]
self.run_clang_tidy()
@ -401,9 +444,14 @@ class MongoTidyTests(unittest.TestCase):
""")
)
msg = "error: Prefer using other mutex types over `WriteRarelyRWMutex`."
opt = "mongo-rwmutex-check,-warnings-as-errors"
self.expected_output = [
"error: Prefer using other mutex types over `WriteRarelyRWMutex`. [mongo-rwmutex-check,-warnings-as-errors]\nWriteRarelyRWMutex mutex_vardecl;",
"error: Prefer using other mutex types over `WriteRarelyRWMutex`. [mongo-rwmutex-check,-warnings-as-errors]\n WriteRarelyRWMutex mutex_fielddecl;",
f"{msg} [{opt}]\n{n:5} | {src}"
for n, src in [
(5, "WriteRarelyRWMutex mutex_vardecl;"),
(8, " WriteRarelyRWMutex mutex_fielddecl;"),
]
]
self.run_clang_tidy()

4
src/mongo/tools/mongo_tidy_checks/tests/SConscript
View File

@ -99,6 +99,10 @@ if env.GetOption("ninja") == "disabled":
),
"" if mongo_tidy_test_env.Verbose() else "Runnning mongo tidy checks unittests.",
),
ENV={
"MONGO_TOOLCHAIN_VERSION": env["MONGO_TOOLCHAIN_VERSION"],
"MONGO_TOOLCHAIN_FROM_BAZEL": "false",
},
)
mongo_tidy_test_env.Alias("+mongo-tidy-tests", test)