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 <m...@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(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits