[clang-tidy] Add modernize-nlohmann-json-explicit-conversions check

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

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");

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

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

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
 ^^^^^^^^^^^^^^^^^
 

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"

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;
+       // ...
+     }

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();
+}