[clang-tidy] Add modernize-nlohmann-json-explicit-conversions check #126425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add check to convert nlohmann/json implicit conversions to explicit calls to the templated get() method with an explicit type.
|
@llvm/pr-subscribers-clang-tidy Author: Mike Crowe (mikecrowe) ChangesAdd check to convert nlohmann/json implicit conversions to explicit calls to the templated get() method with an explicit type. Full diff: https://github.com/llvm/llvm-project/pull/126425.diff 8 Files Affected:
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();
+}
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Mike Crowe (mikecrowe) ChangesAdd check to convert nlohmann/json implicit conversions to explicit calls to the templated get() method with an explicit type. Full diff: https://github.com/llvm/llvm-project/pull/126425.diff 8 Files Affected:
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();
+}
|
|
I am not sure whether we should accept this kind of "common" used library's custom check in clang-tidy.
I think we should a guideline this kind of cases to avoid to waste contributors effort. It is frustrating to write code and find out it can't possibly be merged. |
|
I have some mixed feelings as well, this could open the door for clang-tidy becoming a tool for modernizing "any" 3rd-party library usage. It feels like instead nlohmann/json should be the one responsible for providing a "migration path" / tooling for these types of deprecated features. It could also easily become a mess when we have to deal with / support different versions of the libraries. Thus I would vote for keeping clang-tidy as a general tool for C++/STL only. Perhaps these types of utilities could be implemented as a clang-tidy plugin in the respective 3rd-party repository? |
N.B. we already have |
+1
That said, I don't think we've ever had a policy about this sort of thing. If we have a consensus opinion forming, we should probably write an RFC and propose some docs to see if the rest of the community buys into it. |
|
I had anticipated some pushback for this check. The capabilities that clang-tidy provides give library writers the ability to make breaking changes whilst keeping their users happy because avoiding the breakage can be (at least mostly) automated. I think that such changes are far more common outside the standard library and C++ language itself and they allow correcting the mistakes of the past. There are many heavily-used libraries that would benefit from clang-tidy checks. I believe that such automation is planned to be an integral part of the Carbon compatibility story. Until now I hadn't realised that clang-tidy plugins were possible - support for them has been added since I started writing clang-tidy checks and I clearly haven't been paying attention! I will investigate turning this check into a plugin instead. I think that the situation for "modernize" checks like this one is different to other checks too: such checks are more likely to only be run a few times for testing and then finally once for real. They won't be incorporated into CI pipelines. Once the code has been modernised the checks will never be used on that code again. This means that having to build the check by hand is more acceptable. Nevertheless, I think that it would be worth having a policy on clang-tidy support for libraries to manage expectations. Perhaps that policy could even mean moving some of the existing built-in checks out to plugins in the long term? Even assuming that this check will never land, I'm still interested in review of the code that will hopefully eventually end up in a plugin if anyone wishes to look at it. @HerrCai0907 wrote:
In this case I'm not particularly worried. I wrote the check so I could use it. The amount of extra work required for the release notes etc. included in this change was small. @carosgalvezp wrote:
clang-tidy provides an excellent basis for doing that so it should be no surprise that library authors and users would want to build on it. Hopefully a plugin can work well enough, but I am slightly worried about compatibility with different versions of clang-tidy. I'll find out! Thanks to everyone for their comments. |
clang-tools-extra/clang-tidy/modernize/NlohmannJsonExplicitConversionsCheck.cpp
Show resolved
Hide resolved
EugeneZelenko
requested review from
PiotrZSL,
carlosgalvezp,
5chmidti and
HerrCai0907
Comment on lines
+39
to
+40
| if (DestinationTypeStr == "std::basic_string<char>") | ||
| DestinationTypeStr = "std::string"; |
There was a problem hiding this comment.
Do you need to worry about wchar_t and std::wstring as well?
Also, what about things like string_view?
There was a problem hiding this comment.
Thanks for the review. As far as I can tell, the library doesn't support implicit conversion to std::wstring or std::string_view but I will double check with the author.
|
Thanks for the reviews. I have been able to turn this check into a plugin successfully and I will propose that it is included with the nlohmann/json library. |
mikecrowe
added a commit
to mikecrowe/nlohmann-json
that referenced
this pull request
Feb 23, 2025
Add a clang-tidy plugin containing one check to replace implicit conversions with explicit ones. Being able to test the plugin in a similar way to how checks are tested within clang-tidy itself requires copying the check_clang_tidy.py script from the LLVM code itself. The check itself is virtually identical to the one proposed for inclusion in clang-tidy itself at llvm/llvm-project#126425 . Unfortunately it is necessary to add "C" to the languages for the project in CMakeLists.txt for find_package to work for LLVM.
mikecrowe
added a commit
to mikecrowe/nlohmann-json
that referenced
this pull request
Feb 23, 2025
Add a clang-tidy plugin containing one check to replace implicit conversions (as enabled by default with JSON_USE_IMPLICIT_CONVERSIONS) with explicit ones. This will make it easier for library users to switch away from using implicit conversions which should make it possible for the library to start disallowing them sooner. Being able to test the plugin in a similar way to how checks are tested within clang-tidy itself requires copying the check_clang_tidy.py script from the LLVM code itself. The check itself is virtually identical to the one proposed for inclusion in clang-tidy itself at llvm/llvm-project#126425 . Unfortunately it is necessary to add "C" to the languages for the project in CMakeLists.txt for find_package to work for LLVM. Signed-off-by: Mike Crowe <mac@mcrowe.com>
5 tasks
mikecrowe
added a commit
to mikecrowe/nlohmann-json
that referenced
this pull request
Mar 9, 2025
Add a clang-tidy plugin containing one check to replace implicit conversions (as enabled by default with JSON_USE_IMPLICIT_CONVERSIONS) with explicit ones. This will make it easier for library users to switch away from using implicit conversions which should make it possible for the library to start disallowing them sooner. Being able to test the plugin in a similar way to how checks are tested within clang-tidy itself requires copying the check_clang_tidy.py script from the LLVM code itself. The check itself is virtually identical to the one proposed for inclusion in clang-tidy itself at llvm/llvm-project#126425 . Unfortunately it is necessary to add "C" to the languages for the project in CMakeLists.txt for find_package to work for LLVM. Signed-off-by: Mike Crowe <mac@mcrowe.com>
mikecrowe
added a commit
to mikecrowe/nlohmann-json
that referenced
this pull request
Mar 9, 2025
Add a clang-tidy plugin containing one check to replace implicit conversions (as enabled by default with JSON_USE_IMPLICIT_CONVERSIONS) with explicit ones. This will make it easier for library users to switch away from using implicit conversions which should make it possible for the library to start disallowing them sooner. Being able to test the plugin in a similar way to how checks are tested within clang-tidy itself requires copying the check_clang_tidy.py script from the LLVM code itself. The check itself is virtually identical to the one proposed for inclusion in clang-tidy itself at llvm/llvm-project#126425 . Unfortunately it is necessary to add "C" to the languages for the project in CMakeLists.txt for find_package to work for LLVM. Signed-off-by: Mike Crowe <mac@mcrowe.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add check to convert nlohmann/json implicit conversions to explicit calls to the templated get() method with an explicit type.