[clang-tools-extra] [clang-tidy] Add modernize-nlohmann-json-explicit-conversions check (PR #126425)
Mike Crowe via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 9 10:41:30 PST 2025
https://github.com/mikecrowe created https://github.com/llvm/llvm-project/pull/126425
Add check to convert nlohmann/json implicit conversions to explicit calls to the templated get() method with an explicit type.
>From fe370ac53ccf8c59ebda21a89a4e60e1441b7916 Mon Sep 17 00:00:00 2001
From: Mike Crowe <mac at mcrowe.com>
Date: Sun, 12 Jan 2025 17:59:24 +0000
Subject: [PATCH] [clang-tidy] Add modernize-nlohmann-json-explicit-conversions
check
Add check to convert nlohmann/json implicit conversions to explicit
calls to the templated get() method with an explicit type.
---
.../clang-tidy/modernize/CMakeLists.txt | 1 +
.../modernize/ModernizeTidyModule.cpp | 3 +
.../NlohmannJsonExplicitConversionsCheck.cpp | 72 +++++++++++
.../NlohmannJsonExplicitConversionsCheck.h | 36 ++++++
clang-tools-extra/docs/ReleaseNotes.rst | 7 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../nlohmann-json-explicit-conversions.rst | 58 +++++++++
.../nlohmann-json-explicit-conversions.cpp | 122 ++++++++++++++++++
8 files changed, 300 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/nlohmann-json-explicit-conversions.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/nlohmann-json-explicit-conversions.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index bab1167fb15ff20..bfee70f7e214de2 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -18,6 +18,7 @@ add_clang_library(clangTidyModernizeModule STATIC
MakeUniqueCheck.cpp
MinMaxUseInitializerListCheck.cpp
ModernizeTidyModule.cpp
+ NlohmannJsonExplicitConversionsCheck.cpp
PassByValueCheck.cpp
RawStringLiteralCheck.cpp
RedundantVoidArgCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fc46c72982fdce8..ada628eccad185c 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -19,6 +19,7 @@
#include "MakeSharedCheck.h"
#include "MakeUniqueCheck.h"
#include "MinMaxUseInitializerListCheck.h"
+#include "NlohmannJsonExplicitConversionsCheck.h"
#include "PassByValueCheck.h"
#include "RawStringLiteralCheck.h"
#include "RedundantVoidArgCheck.h"
@@ -74,6 +75,8 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
CheckFactories.registerCheck<MinMaxUseInitializerListCheck>(
"modernize-min-max-use-initializer-list");
+ CheckFactories.registerCheck<NlohmannJsonExplicitConversionsCheck>(
+ "modernize-nlohmann-json-explicit-conversions");
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
CheckFactories.registerCheck<UseDesignatedInitializersCheck>(
"modernize-use-designated-initializers");
diff --git a/clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.cpp
new file mode 100644
index 000000000000000..7a88915f4e58905
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.cpp
@@ -0,0 +1,72 @@
+//===--- NlohmannJsonExplicitConversionsCheck.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 "NlohmannJsonExplicitConversionsCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+void NlohmannJsonExplicitConversionsCheck::registerMatchers(
+ MatchFinder *Finder) {
+ auto Matcher =
+ cxxMemberCallExpr(
+ on(expr().bind("arg")),
+ callee(cxxConversionDecl(ofClass(hasName("nlohmann::basic_json")))
+ .bind("conversionDecl")))
+ .bind("conversionCall");
+ Finder->addMatcher(Matcher, this);
+}
+
+void NlohmannJsonExplicitConversionsCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Decl =
+ Result.Nodes.getNodeAs<CXXConversionDecl>("conversionDecl");
+ const auto *Call =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("conversionCall");
+ const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
+
+ const QualType DestinationType = Decl->getConversionType();
+ std::string DestinationTypeStr =
+ DestinationType.getAsString(Result.Context->getPrintingPolicy());
+ if (DestinationTypeStr == "std::basic_string<char>")
+ DestinationTypeStr = "std::string";
+
+ SourceRange ExprRange = Call->getSourceRange();
+ if (!ExprRange.isValid())
+ return;
+
+ bool Deref = false;
+ if (const auto *Op = llvm::dyn_cast<UnaryOperator>(Arg);
+ Op && Op->getOpcode() == UO_Deref)
+ Deref = true;
+ else if (const auto *Op = llvm::dyn_cast<CXXOperatorCallExpr>(Arg);
+ Op && Op->getOperator() == OO_Star)
+ Deref = true;
+
+ llvm::StringRef SourceText = clang::Lexer::getSourceText(
+ clang::CharSourceRange::getTokenRange(ExprRange), *Result.SourceManager,
+ Result.Context->getLangOpts());
+
+ if (Deref)
+ SourceText.consume_front("*");
+
+ const std::string ReplacementText =
+ (llvm::Twine(SourceText) + (Deref ? "->" : ".") + "get<" +
+ DestinationTypeStr + ">()")
+ .str();
+ diag(Call->getExprLoc(),
+ "implicit nlohmann::json conversion to %0 should be explicit")
+ << DestinationTypeStr
+ << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(ExprRange),
+ ReplacementText);
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.h b/clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.h
new file mode 100644
index 000000000000000..869ba601271dc4b
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.h
@@ -0,0 +1,36 @@
+//===--- NlohmannJsonExplicitConversionsCheck.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_MODERNIZE_NLOHMANNJSONEXPLICITCONVERSIONSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NLOHMANNJSONEXPLICITCONVERSIONSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Convert implicit conversions via operator ValueType() from nlohmann::json to
+/// explicit calls to the get<> method because the next major version of the
+/// library will remove support for implicit conversions.
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/nlohmann-json-explicit-conversions.html
+class NlohmannJsonExplicitConversionsCheck : public ClangTidyCheck {
+public:
+ NlohmannJsonExplicitConversionsCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ 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;
+ }
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NLOHMANNJSONEXPLICITCONVERSIONSCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3bddeeda06e06b9..9b100ca233cdc7c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`modernize-nlohmann-json-explicit-conversions
+ <clang-tidy/checks/modernize/nlohmann-json-explicit-conversions>` check.
+
+ Converts implicit conversions via ``operator ValueType`` in code that
+ uses the `nlohmann/json <https://json.nlohmann.me/>`_ library to calls to
+ the ``get()`` method with an explicit type.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 7b9b905ef76715e..3034fd303397434 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -284,6 +284,7 @@ Clang-Tidy Checks
:doc:`modernize-make-shared <modernize/make-shared>`, "Yes"
:doc:`modernize-make-unique <modernize/make-unique>`, "Yes"
:doc:`modernize-min-max-use-initializer-list <modernize/min-max-use-initializer-list>`, "Yes"
+ :doc:`modernize-nlohmann-json-explicit-conversions <modernize/nlohmann-json-explicit-conversions>`, "Yes"
:doc:`modernize-pass-by-value <modernize/pass-by-value>`, "Yes"
:doc:`modernize-raw-string-literal <modernize/raw-string-literal>`, "Yes"
:doc:`modernize-redundant-void-arg <modernize/redundant-void-arg>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/nlohmann-json-explicit-conversions.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/nlohmann-json-explicit-conversions.rst
new file mode 100644
index 000000000000000..6e1b709f2f3ce1e
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/nlohmann-json-explicit-conversions.rst
@@ -0,0 +1,58 @@
+.. title:: clang-tidy - modernize-nlohmann-json-explicit-conversions
+
+modernize-nlohmann-json-explicit-conversions
+============================================
+
+Converts implicit conversions via ``operator ValueType`` in code that uses
+the `nlohmann/json`_ library to calls to the ``get()`` member function with
+an explicit type. The next major version of the library `will remove
+support for`_ these implicit conversions and support for them `can be
+disabled now`_ by defining ``JSON_USE_IMPLICIT_CONVERSIONS`` to be ``0``.
+
+.. _nlohmann/json: https://json.nlohmann.me/
+.. _will remove support for: https://json.nlohmann.me/integration/migration_guide/#replace-implicit-conversions
+.. _can be disabled now: https://json.nlohmann.me/api/macros/json_use_implicit_conversions/
+
+In other words, it turns:
+
+ .. code-block:: c++
+
+ void f(const nlohmann::json &j1, const nlohmann::json &j2)
+ {
+ int i = j1;
+ double d = j2.at("value");
+ bool b = *j2.find("valid");
+ std::cout << i << " " << d << " " << b << "\n";
+ }
+
+into
+
+ .. code-block:: c++
+
+ void f(const nlohmann::json &j1, const nlohmann::json &j2)
+ {
+ int i = j1.get<int>();
+ double d = j2.at("value").get<double>();
+ bool b = j2.find("valid")->get<bool>();
+ std::cout << i << " " << d << " " << b << "\n";
+ }
+
+by knowing what the target type is for the implicit conversion and turning
+that into an explicit call to the ``get`` method with that type as the
+template parameter.
+
+Unfortunately the check does not work very well if the implicit conversion
+occurs in templated code or in a system header. For example, the following
+won't be fixed because the implicit conversion will happen inside
+``std::optional``'s constructor:
+
+ .. code-block:: c++
+
+ void f(const nlohmann::json &j)
+ {
+ std::optional<int> oi;
+ const auto &it = j.find("value");
+ if (it != j.end())
+ oi = *it;
+ // ...
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/nlohmann-json-explicit-conversions.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/nlohmann-json-explicit-conversions.cpp
new file mode 100644
index 000000000000000..45bb1a00ec4a0af
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/nlohmann-json-explicit-conversions.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s modernize-nlohmann-json-explicit-conversions %t -- -- -isystem %clang_tidy_headers
+
+#include <string>
+
+namespace nlohmann
+{
+ class basic_json
+ {
+ public:
+ template <typename ValueType>
+ ValueType get() const
+ {
+ return ValueType{};
+ }
+
+ // nlohmann::json uses SFINAE to limit the types that can be converted to.
+ // Rather than do that here, let's just provide the overloads we need
+ // instead.
+ operator int() const
+ {
+ return get<int>();
+ }
+
+ operator double() const
+ {
+ return get<double>();
+ }
+
+ operator std::string() const
+ {
+ return get<std::string>();
+ }
+
+ int otherMember() const;
+ };
+
+ class iterator
+ {
+ public:
+ basic_json &operator*();
+ basic_json *operator->();
+ };
+
+ using json = basic_json;
+}
+
+using nlohmann::json;
+using nlohmann::iterator;
+
+int get_int(json &j)
+{
+ return j;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to int should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return j.get<int>();
+}
+
+std::string get_string(json &j)
+{
+ return j;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to std::string should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return j.get<std::string>();
+}
+
+int get_int_ptr(json *j)
+{
+ return *j;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to int should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return j->get<int>();
+}
+
+int get_int_ptr_expr(json *j)
+{
+ return *(j+1);
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to int should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return (j+1)->get<int>();
+}
+
+int get_int_iterator(iterator i)
+{
+ return *i;
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to int should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return i->get<int>();
+}
+
+int get_int_fn()
+{
+ extern json get_json();
+ return get_json();
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to int should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return get_json().get<int>();
+}
+
+double get_double_fn_ref()
+{
+ extern nlohmann::json &get_json_ref();
+ return get_json_ref();
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to double should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return get_json_ref().get<double>();
+}
+
+std::string get_string_fn_ptr()
+{
+ extern json *get_json_ptr();
+ return *get_json_ptr();
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: implicit nlohmann::json conversion to std::string should be explicit [modernize-nlohmann-json-explicit-conversions]
+ // CHECK-FIXES: return get_json_ptr()->get<std::string>();
+}
+
+int call_other_member(nlohmann::json &j)
+{
+ return j.otherMember();
+}
+
+int call_other_member_ptr(nlohmann::json *j)
+{
+ return j->otherMember();
+}
+
+int call_other_member_iterator(iterator i)
+{
+ return i->otherMember();
+}
More information about the cfe-commits
mailing list