diff --git a/src/mongo/tools/mongo_tidy_checks/BUILD.bazel b/src/mongo/tools/mongo_tidy_checks/BUILD.bazel index 46ed5a25ce9..940ec39ca78 100644 --- a/src/mongo/tools/mongo_tidy_checks/BUILD.bazel +++ b/src/mongo/tools/mongo_tidy_checks/BUILD.bazel @@ -20,6 +20,7 @@ cc_library( "MongoCxx20StdChronoCheck.cpp", "MongoFCVConstantCheck.cpp", "MongoHeaderBracketCheck.cpp", + "MongoHeaderIncludePathCheck.cpp", "MongoInvariantDDLCoordinatorCheck.cpp", "MongoInvariantStatusIsOKCheck.cpp", "MongoMacroDefinitionLeaksCheck.cpp", @@ -45,6 +46,7 @@ cc_library( "MongoCxx20StdChronoCheck.h", "MongoFCVConstantCheck.h", "MongoHeaderBracketCheck.h", + "MongoHeaderIncludePathCheck.h", "MongoInvariantDDLCoordinatorCheck.h", "MongoInvariantStatusIsOKCheck.h", "MongoMacroDefinitionLeaksCheck.h", diff --git a/src/mongo/tools/mongo_tidy_checks/MongoHeaderIncludePathCheck.cpp b/src/mongo/tools/mongo_tidy_checks/MongoHeaderIncludePathCheck.cpp new file mode 100644 index 00000000000..f872246db2a --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/MongoHeaderIncludePathCheck.cpp @@ -0,0 +1,206 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + + +#include "MongoHeaderIncludePathCheck.h" + +#include + +#include +#include +#include +#include +#include +#include + +namespace mongo::tidy { + +namespace { + +bool containsAny(llvm::StringRef fullString, clang::ArrayRef 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) / "")) || + // also accept relative paths + fullString.contains(llvm::StringRef(std::filesystem::current_path() / + std::filesystem::path(patternStr) / "")); + }); +} + +static std::optional canonicalFromSrc(const std::filesystem::path& full) { + std::filesystem::path rel = full.lexically_normal(); + + // If absolute, try to make it relative to CWD (best effort; OK if it fails) + if (rel.is_absolute()) { + std::error_code ec; + auto tmp = std::filesystem::relative(rel, std::filesystem::current_path(), ec); + if (!ec) + rel = std::move(tmp); + } + + bool seenSrc = false; + std::filesystem::path out; + for (const auto& comp : rel) { + // Note: on Windows, comp is a path; compare as string ignoring slashes. + if (!seenSrc) { + if (comp == "src") + seenSrc = true; + } else { + out /= comp; + } + } + if (!seenSrc || out.empty()) + return std::nullopt; + return out.generic_string(); // forward slashes +} + +// Skip canonical-path enforcement for generated protobuf headers (.pb.h, .grpc.pb.h, .pb.hpp) +static bool isGeneratedProtoHeader(llvm::StringRef spelledName, llvm::StringRef resolvedFullPath) { + std::string spelled = spelledName.str(); + std::replace(spelled.begin(), spelled.end(), '\\', '/'); + std::string full = resolvedFullPath.str(); + std::replace(full.begin(), full.end(), '\\', '/'); + + auto endsWith = [](llvm::StringRef s, llvm::StringRef suf) { + return s.ends_with_insensitive(suf); + }; + if (endsWith(spelled, ".pb.h") || endsWith(spelled, ".grpc.pb.h") || + endsWith(spelled, ".pb.hpp")) { + return true; + } + + auto has = [](const std::string& hay, llvm::StringRef needle) { + return hay.find(needle.str()) != std::string::npos; + }; + if ((has(full, ".pb.h") || has(full, ".grpc.pb.h") || has(full, ".pb.hpp"))) { + return true; + } + return false; +} + +inline bool isMongoPath(const std::filesystem::path& p, + clang::ArrayRef mongoSourceDirs) { + return containsAny(llvm::StringRef(p.lexically_normal().string()), mongoSourceDirs); +} + +inline bool isExcludedFromCanonicalization(llvm::StringRef spelledName, + llvm::StringRef resolvedFullPath) { + // Skip generated protos and a few allow-listed paths. + if (isGeneratedProtoHeader(spelledName, resolvedFullPath)) + return true; + if (spelledName.contains("bsoncxx/") || spelledName.contains("mongocxx/")) + return true; + if (containsAny(resolvedFullPath, llvm::ArrayRef{"mongo_tidy_checks/"})) + return true; + return false; +} + +class MongoIncludeIncludePathPPCallbacks : public clang::PPCallbacks { +public: + explicit MongoIncludeIncludePathPPCallbacks(MongoHeaderIncludePathCheck& Check, + clang::LangOptions LangOpts, + const clang::SourceManager& SM) + : Check(Check), LangOpts(LangOpts), SM(SM) {} + + + void InclusionDirective(clang::SourceLocation HashLoc, + const clang::Token& IncludeTok, + llvm::StringRef FileName, + bool IsAngled, + clang::CharSourceRange FilenameRange, + clang::OptionalFileEntryRef File, + llvm::StringRef SearchPath, + llvm::StringRef RelativePath, + const clang::Module* SuggestedModule, + bool ModuleImported, + clang::SrcMgr::CharacteristicKind FileType) override { + // Resolve header path on disk (best effort). + const std::filesystem::path headerPath = + (std::filesystem::path(SearchPath.str()) / std::filesystem::path(RelativePath.str())) + .lexically_normal(); + if (!std::filesystem::exists(headerPath)) + return; // nothing to do + + // Resolve the including source file (avoid "path:line:col" by using getFilename()). + const llvm::StringRef originFile = SM.getFilename(HashLoc); + if (originFile.empty() || originFile.starts_with(":")) + return; + + // Guard clauses for scope of this check. + std::filesystem::path originPath(originFile.str()); + if (!isMongoPath(originPath, Check.mongoSourceDirs)) + return; // only enforce for mongo sources + if (!isMongoPath(headerPath, Check.mongoSourceDirs)) + return; // only canonicalize mongo headers + if (isExcludedFromCanonicalization(FileName, llvm::StringRef(headerPath.string()))) + return; + + // Compute canonical 'src/'-relative form and compare against spelled name. + if (auto canon = canonicalFromSrc(headerPath)) { + std::string spelled = FileName.str(); + std::replace(spelled.begin(), spelled.end(), '\\', '/'); // normalize for compare + + if (spelled != *canon) { + const std::string replacement = + (llvm::Twine("\"") + llvm::StringRef(*canon) + "\"").str(); + Check.diag(FilenameRange.getBegin(), + "mongo include '%0' should be referenced from 'src/' as '%1'") + << FileName << *canon + << clang::FixItHint::CreateReplacement(FilenameRange.getAsRange(), replacement); + } + } + } + +private: + MongoHeaderIncludePathCheck& Check; + clang::LangOptions LangOpts; + const clang::SourceManager& SM; +}; + +} // namespace + +MongoHeaderIncludePathCheck::MongoHeaderIncludePathCheck(llvm::StringRef Name, + clang::tidy::ClangTidyContext* Context) + : ClangTidyCheck(Name, Context), + mongoSourceDirs(clang::tidy::utils::options::parseStringList( + Options.get("mongoSourceDirs", "src/mongo"))) {} + +void MongoHeaderIncludePathCheck::storeOptions(clang::tidy::ClangTidyOptions::OptionMap& Opts) { + Options.store( + Opts, "mongoSourceDirs", clang::tidy::utils::options::serializeStringList(mongoSourceDirs)); +} +void MongoHeaderIncludePathCheck::registerPPCallbacks(const clang::SourceManager& SM, + clang::Preprocessor* PP, + clang::Preprocessor* ModuleExpanderPP) { + PP->addPPCallbacks( + ::std::make_unique(*this, getLangOpts(), SM)); +} + +} // namespace mongo::tidy diff --git a/src/mongo/tools/mongo_tidy_checks/MongoHeaderIncludePathCheck.h b/src/mongo/tools/mongo_tidy_checks/MongoHeaderIncludePathCheck.h new file mode 100644 index 00000000000..7f2c21c76de --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/MongoHeaderIncludePathCheck.h @@ -0,0 +1,57 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ +#pragma once + +#include +#include + +namespace mongo::tidy { + +/** + Overrides the default PPCallback class to primarily override + the InclusionDirective call which is called for each include included. This + allows the chance to evaluate specifically the include and determine whether + it is considered a "mongo" include or not and if it is using the appropriate include style. +*/ +class MongoHeaderIncludePathCheck : public clang::tidy::ClangTidyCheck { +public: + MongoHeaderIncludePathCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context); + void storeOptions(clang::tidy::ClangTidyOptions::OptionMap& Opts) override; + void registerPPCallbacks(const clang::SourceManager& SM, + clang::Preprocessor* PP, + clang::Preprocessor* ModuleExpanderPP) override; + // used to store option `mongoSourceDirs`; supports both absolute and relative paths + std::vector mongoSourceDirs; + + // The path component that denotes the repo's root source directory (default: "src"). + // Used to compute canonical include paths "from src/". + std::string srcRootComponent; +}; + +} // namespace mongo::tidy diff --git a/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp b/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp index 96fd9297a94..97a0b65182f 100644 --- a/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp +++ b/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp @@ -37,6 +37,7 @@ #include "MongoCxx20StdChronoCheck.h" #include "MongoFCVConstantCheck.h" #include "MongoHeaderBracketCheck.h" +#include "MongoHeaderIncludePathCheck.h" #include "MongoInvariantDDLCoordinatorCheck.h" #include "MongoInvariantStatusIsOKCheck.h" #include "MongoMacroDefinitionLeaksCheck.h" @@ -64,6 +65,8 @@ public: CheckFactories.registerCheck( "mongo-uninterruptible-lock-guard-check"); CheckFactories.registerCheck("mongo-header-bracket-check"); + CheckFactories.registerCheck( + "mongo-header-include-path-check"); CheckFactories.registerCheck("mongo-cctype-check"); CheckFactories.registerCheck("mongo-config-header-check"); CheckFactories.registerCheck( diff --git a/src/mongo/tools/mongo_tidy_checks/tests/BUILD.bazel b/src/mongo/tools/mongo_tidy_checks/tests/BUILD.bazel index c5088ecc543..9676f86d331 100644 --- a/src/mongo/tools/mongo_tidy_checks/tests/BUILD.bazel +++ b/src/mongo/tools/mongo_tidy_checks/tests/BUILD.bazel @@ -19,6 +19,7 @@ tests = [ # This list represents the test source files, which should contain a single issue which will be flagged # by a clang tidy check. The issue should be isolated in as minimal way as possible. "test_MongoHeaderBracketCheck", + "test_MongoHeaderIncludePathCheck", "test_MongoVolatileCheck", "test_MongoUninterruptibleLockGuardCheck", "test_MongoUninterruptibleLockGuardCheckForOpCtxMember", @@ -59,6 +60,7 @@ tests = [ "-Wno-unused-variable", "-Wno-return-type", "-Isrc/mongo/tools/mongo_tidy_checks/tests", + "-Isrc/mongo", # for mongo header path test ] + select({ "//bazel/config:compiler_type_gcc": ["-Wno-volatile"], "//bazel/config:compiler_type_clang": ["-Wno-deprecated-volatile"], diff --git a/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py b/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py index 4fbb249ae49..c85b4180b3e 100644 --- a/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py +++ b/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py @@ -77,6 +77,14 @@ class MongoTidyTests(unittest.TestCase): self.run_clang_tidy() + def test_MongoHeaderIncludePathCheck(self): + self.expected_output = [ + "mongo include 'base/data_range.h' should be referenced from 'src/' as 'mongo/base/data_range.h'", + "mongo include 'base/data_type.h' should be referenced from 'src/' as 'mongo/base/data_type.h'", + ] + + self.run_clang_tidy() + def test_MongoUninterruptibleLockGuardCheck(self): self.expected_output = ( "Potentially incorrect use of UninterruptibleLockGuard, " diff --git a/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.cpp b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.cpp new file mode 100644 index 00000000000..8cb12fc80fb --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.cpp @@ -0,0 +1,3 @@ +#include "test_MongoHeaderIncludePathCheck.h" + +#include "base/data_range.h" diff --git a/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.h b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.h new file mode 100644 index 00000000000..d92f741900b --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.h @@ -0,0 +1 @@ +#include "base/data_type.h" diff --git a/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.tidy_config b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.tidy_config new file mode 100644 index 00000000000..3f0adb6af19 --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoHeaderIncludePathCheck.tidy_config @@ -0,0 +1,3 @@ +Checks: '-*,mongo-header-include-path-check' +WarningsAsErrors: '*' +HeaderFilterRegex: 'mongo/[-A-Za-z0-9_/]*\.(h|hpp|ipp|inl|cstruct.h|defs|def.h)'