SERVER-67234 add clang-tidy rule to ban fcv constant usage in compare

.clang-tidy.in

@@ -43,6 +43,7 @@ Checks:          '-*,
                   mongo-uninterruptible-lock-guard-check,
                   mongo-volatile-check,
                   mongo-trace-check,
+                  mongo-fcv-constant-check,
                   performance-faster-string-find,
                   performance-implicit-conversion-in-loop,
                   performance-inefficient-algorithm,

src/mongo/db/repl/tenant_migration_util.h

@@ -250,14 +250,6 @@ inline void protocolTenantIdCompatibilityCheck(const MigrationProtocolEnum proto
 
 inline void protocolTenantIdsCompatibilityCheck(
     const MigrationProtocolEnum protocol, const boost::optional<std::vector<TenantId>>& tenantIds) {
-    if (serverGlobalParams.featureCompatibility.isLessThan(
-            multiversion::FeatureCompatibilityVersion::kVersion_6_3)) {
-        uassert(ErrorCodes::InvalidOptions,
-                "'tenantIds' is not supported for FCV below 6.3'",
-                !tenantIds);
-        return;
-    }
-
     switch (protocol) {
         case MigrationProtocolEnum::kShardMerge: {
             {

src/mongo/rpc/metadata/impersonated_user_metadata.cpp

@@ -103,17 +103,8 @@ void writeAuthDataToImpersonatedUserMetadata(OperationContext* opCtx, BSONObjBui
     }
 
     ImpersonatedUserMetadata metadata;
-    if (serverGlobalParams.featureCompatibility.isLessThanOrEqualTo(
-            multiversion::FeatureCompatibilityVersion::kVersion_6_2)) {
-        if (userName) {
-            metadata.setUsers({{userName.value()}});
-        } else {
-            metadata.setUsers({});
-        }
-    } else {
-        if (userName) {
-            metadata.setUser(userName.value());
-        }
+    if (userName) {
+        metadata.setUser(userName.value());
     }
 
     metadata.setRoles(roleNameIteratorToContainer<std::vector<RoleName>>(roleNames));

src/mongo/tools/mongo_tidy_checks/MongoFCVConstantCheck.cpp

@@ -0,0 +1,71 @@
+/**
+ *    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
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    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 "MongoFCVConstantCheck.h"
+
+#include <clang/AST/ComputeDependence.h>
+
+namespace mongo::tidy {
+
+using namespace clang;
+using namespace clang::ast_matchers;
+
+MongoFCVConstantCheck::MongoFCVConstantCheck(StringRef Name, clang::tidy::ClangTidyContext* Context)
+    : ClangTidyCheck(Name, Context) {}
+
+void MongoFCVConstantCheck::registerMatchers(ast_matchers::MatchFinder* Finder) {
+
+    Finder->addMatcher(
+        // Match a FCV comparison function whose argument is a FCV constant.
+        declRefExpr(
+            // Find a reference to FeatureCompatibilityVersion enum.
+            hasDeclaration(enumConstantDecl(
+                hasType(enumDecl(hasName("mongo::multiversion::FeatureCompatibilityVersion"))))),
+            // Find a call to FCV comparison functions.
+            hasParent(callExpr(anyOf(
+                callee(functionDecl(hasName("FeatureCompatibility::isLessThan"))),
+                callee(functionDecl(hasName("FeatureCompatibility::isGreaterThan"))),
+                callee(functionDecl(hasName("FeatureCompatibility::isLessThanOrEqualTo"))),
+                callee(functionDecl(hasName("FeatureCompatibility::isGreaterThanOrEqualTo"))),
+                callee(functionDecl(hasName("FeatureCompatibility::isUpgradingOrDowngrading")))))))
+            .bind("fcv_constant"),
+        this);
+}
+
+void MongoFCVConstantCheck::check(const ast_matchers::MatchFinder::MatchResult& Result) {
+
+    const auto* loc_match = Result.Nodes.getNodeAs<DeclRefExpr>("fcv_constant");
+    if (loc_match) {
+        diag(loc_match->getBeginLoc(),
+             "Illegal use of FCV constant in FCV comparison check functions. FCV gating should be "
+             "done through feature flags instead.");
+    }
+}
+
+}  // namespace mongo::tidy

src/mongo/tools/mongo_tidy_checks/MongoFCVConstantCheck.h

@@ -0,0 +1,54 @@
+/**
+ *    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
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    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 <clang-tidy/ClangTidy.h>
+#include <clang-tidy/ClangTidyCheck.h>
+
+namespace mongo {
+namespace tidy {
+/**
+ * MongoFCVConstantCheck is a custom clang-tidy check for detecting the usage of
+ * comparing FCV using the FeatureCompatibilityVersion enums, e.g.
+ * FeatureCompatibilityVersion::kVersion_X_Y.
+ *
+ * It extends ClangTidyCheck and overrides the registerMatchers
+ * and check functions. The registerMatchers function adds matchers
+ * to identify the usage of nongmongo assert, while
+ * the check function flags the matched occurrences
+ */
+class MongoFCVConstantCheck : public clang::tidy::ClangTidyCheck {
+public:
+    MongoFCVConstantCheck(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;
+};
+
+}  // namespace tidy
+}  // namespace mongo

src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp

@@ -30,6 +30,7 @@
 #include "MongoAssertCheck.h"
 #include "MongoCctypeCheck.h"
 #include "MongoCxx20BannedIncludesCheck.h"
+#include "MongoFCVConstantCheck.h"
 #include "MongoHeaderBracketCheck.h"
 #include "MongoMutexCheck.h"
 #include "MongoStdAtomicCheck.h"
@@ -61,6 +62,7 @@ public:
         CheckFactories.registerCheck<MongoStdAtomicCheck>("mongo-std-atomic-check");
         CheckFactories.registerCheck<MongoMutexCheck>("mongo-mutex-check");
         CheckFactories.registerCheck<MongoAssertCheck>("mongo-assert-check");
+        CheckFactories.registerCheck<MongoFCVConstantCheck>("mongo-fcv-constant-check");
     }
 };
 

src/mongo/tools/mongo_tidy_checks/SConscript

@@ -132,6 +132,7 @@ mongo_custom_check = env.SharedLibrary(
         "MongoTraceCheck.cpp",
         "MongoMutexCheck.cpp",
         "MongoAssertCheck.cpp",
+        "MongoFCVConstantCheck.cpp",
     ],
     LIBDEPS_NO_INHERIT=[
         '$BUILD_DIR/third_party/shim_allocator',

src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py

@@ -247,6 +247,19 @@ class MongoTidyTests(unittest.TestCase):
 
         self.run_clang_tidy()
 
+    def test_MongoFCVConstantCheck(self):
+
+        self.write_config(
+            textwrap.dedent("""\
+                Checks: '-*,mongo-fcv-constant-check'
+                WarningsAsErrors: '*'
+                """))
+
+        self.expected_output = [
+            "error: Illegal use of FCV constant in FCV comparison check functions. FCV gating should be done through feature flags instead.",
+        ]
+
+        self.run_clang_tidy()
 
 if __name__ == '__main__':
 

src/mongo/tools/mongo_tidy_checks/tests/SConscript

@@ -35,6 +35,7 @@ if env.GetOption('ninja') == 'disabled':
         'test_MongoStdAtomicCheck.cpp',
         'test_MongoMutexCheck.cpp',
         'test_MongoAssertCheck.cpp',
+        'test_MongoFCVConstantCheck.cpp',
     ]
 
     # So that we can do fast runs, we will generate a separate compilation database file for each

src/mongo/tools/mongo_tidy_checks/tests/test_MongoFCVConstantCheck.cpp

@@ -0,0 +1,56 @@
+/**
+ *    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
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    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.
+ */
+namespace mongo {
+namespace multiversion {
+
+enum class FeatureCompatibilityVersion {
+    kInvalid,
+    kVersion_7_0,
+};
+
+}  // namespace multiversion
+
+using FCV = multiversion::FeatureCompatibilityVersion;
+
+struct ServerGlobalParams {
+    struct FeatureCompatibility {
+        bool isLessThan(FCV version) const {
+            return true;
+        }
+    } mutableFeatureCompatibility;
+    const FeatureCompatibility& featureCompatibility = mutableFeatureCompatibility;
+};
+
+void testFCVConstant() {
+    const ServerGlobalParams mockParam = {};
+    mockParam.featureCompatibility.isLessThan(
+        multiversion::FeatureCompatibilityVersion::kVersion_7_0);
+}
+
+}  // namespace mongo