Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add clang-tidy plugin to convert implicit conversions to explicit ones #4663

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .github/workflows/clang-tidy-plugin.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
name: Ubuntu

on:
push:
branches:
- develop
- master
- release/*
pull_request:
workflow_dispatch:

permissions:
contents: read

jobs:
ci_test_clang_tidy:
runs-on: ubuntu-latest
strategy:
matrix:
# clang-tidy plugins require at least Clang 16. This means that
# anything older than Ubuntu 24.04 and Debian 12 can't work at all.
include:
- distro: ubuntu:24.04
clang_version: 16
- distro: ubuntu:24.04
clang_version: 17
- distro: ubuntu:24.04
clang_version: 18
- distro: debian:12
clang_version: 16
- distro: debian:12
clang_version: 19
- distro: debian:trixie
clang_version: 17
- distro: debian:trixie
clang_version: 18
- distro: debian:trixie
clang_version: 19
container: ${{ matrix.distro }}
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: List files
run: ls -l
- name: Install packages
run: >-
echo distro=${{matrix.distro}} &&
echo clang_version=${{matrix.clang_version}} &&
apt-get update &&
apt-get install -y
build-essential
clang-${{ matrix.clang_version }}
clang-tidy-${{ matrix.clang_version }}
libclang-${{matrix.clang_version}}-dev
cmake ninja-build
- name: Run CMake
run: cmake -S . -B build -G Ninja -DJSON_CI=On -DJSON_ClangTidyPlugin=ON
- name: Build clang-tidy plugin
run: cmake --build build --target clang_tidy_plugin/libNlohmannJsonClangTidyPlugin.so
- name: Test clang-tidy plugin
run: cd build && ctest -V --output-on-failure -R modernize-nlohmann-json-explicit-conversions-check
4 changes: 4 additions & 0 deletions .reuse/dep5
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ License: BSD-3-Clause
Files: tools/gdb_pretty_printer/*
Copyright: 2020 Hannes Domani <https://github.com/ssbssa>
License: MIT

Files: tests/clang_tidy_plugin/check_clang_tidy.py
Copyright: 2003-2019 University of Illinois at Urbana-Champaign.
License: Apache-2.0 WITH LLVM-exception
9 changes: 8 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ cmake_minimum_required(VERSION 3.1...3.14)
## PROJECT
## name and version
##
project(nlohmann_json VERSION 3.11.3 LANGUAGES CXX)
project(nlohmann_json VERSION 3.11.3 LANGUAGES C CXX)

##
## MAIN_PROJECT CHECK
Expand Down Expand Up @@ -48,6 +48,7 @@ option(JSON_LegacyDiscardedValueComparison "Enable legacy discarded value compar
option(JSON_Install "Install CMake targets during install step." ${MAIN_PROJECT})
option(JSON_MultipleHeaders "Use non-amalgamated version of the library." ON)
option(JSON_SystemInclude "Include as system headers (skip for clang-tidy)." OFF)
option(JSON_ClangTidyPlugin "Build clang-tidy plug-in." OFF)

if (JSON_CI)
include(ci)
Expand Down Expand Up @@ -158,6 +159,12 @@ CONFIGURE_FILE(
@ONLY
)

if (JSON_ClangTidyPlugin)
find_package(LLVM REQUIRED CONFIG)
find_package(Clang REQUIRED CONFIG)
add_subdirectory(clang_tidy_plugin)
endif()

##
## TESTS
## create and configure the unit test target
Expand Down
15 changes: 15 additions & 0 deletions LICENSES/LLVM-exception.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---- LLVM Exceptions to the Apache 2.0 License ----

As an exception, if, as a result of your compiling your source code, portions
of this Software are embedded into an Object form of such source code, you
may redistribute such embedded portions in such Object form without complying
with the conditions of Sections 4(a), 4(b) and 4(d) of the License.

In addition, if you combine or link compiled forms of this Software with
software that is licensed under the GPLv2 ("Combined Software") and if a
court of competent jurisdiction determines that the patent provision (Section
3), the indemnity provision (Section 9) or other Section of the License
conflicts with the conditions of the GPLv2, you may retroactively and
prospectively choose to deem waived or otherwise exclude such Section(s) of
the License, but only in their entirety and only with respect to the Combined
Software.
11 changes: 11 additions & 0 deletions clang_tidy_plugin/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
add_library(NlohmannJsonClangTidyPlugin MODULE "")
target_compile_options(NlohmannJsonClangTidyPlugin PRIVATE -fno-rtti)
target_include_directories(NlohmannJsonClangTidyPlugin
PRIVATE
${CLANG_INCLUDE_DIRS}
${LLVM_INCLUDE_DIRS}
)
target_sources(NlohmannJsonClangTidyPlugin PRIVATE
${CMAKE_CURRENT_LIST_DIR}/Module.cpp
${CMAKE_CURRENT_LIST_DIR}/ModernizeNlohmannJsonExplicitConversionsCheck.cpp
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// SPDX-FileCopyrightText: 2025 Mike Crowe
// SPDX-License-Identifier: MIT

#include "ModernizeNlohmannJsonExplicitConversionsCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"

using namespace clang::ast_matchers;

namespace {
void replaceAll(std::string &Str, const std::string &From, const std::string &To) {
assert(!From.empty());

std::string::size_type StartPos = 0;
while ((StartPos = Str.find(from, StartPos)) != std::string::npos) {
Str.replace(StartPos, From.length(), To);
StartPos += To.length();
}
}
}

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());
replaceAll(DestinationTypeStr, "std::basic_string<char>", "std::string");

const 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// SPDX-FileCopyrightText: 2025 Mike Crowe
// SPDX-License-Identifier: MIT

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NLOHMANNJSONEXPLICITCONVERSIONSCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_NLOHMANNJSONEXPLICITCONVERSIONSCHECK_H

#include "clang-tidy/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
31 changes: 31 additions & 0 deletions clang_tidy_plugin/Module.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// SPDX-FileCopyrightText: 2025 Mike Crowe
// SPDX-License-Identifier: MIT

#include "clang-tidy/ClangTidyModule.h"
#include "clang-tidy/ClangTidyModuleRegistry.h"
#include "ModernizeNlohmannJsonExplicitConversionsCheck.h"

namespace {

using namespace clang::tidy;

class NlohmannJsonChecksModule : public ClangTidyModule
{
public:
void addCheckFactories(ClangTidyCheckFactories& CheckFactories) override
{
CheckFactories.registerCheck<clang::tidy::modernize::NlohmannJsonExplicitConversionsCheck>(
"modernize-nlohmann-json-explicit-conversions");
}
};

} // namespace

namespace clang::tidy {

// Register the module using this statically initialized variable.
static ClangTidyModuleRegistry::Add<::NlohmannJsonChecksModule> nlohmannJsonChecksInit(
"nlohmann-json-checks-module",
"Adds 'modernize-nlohmann-json-explicit-conversions' check.");

} // namespace clang::tidy
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ By default, implicit conversions are enabled.

You can prepare existing code by already defining `JSON_USE_IMPLICIT_CONVERSIONS` to `0` and replace any implicit
conversions with calls to [`get`](../basic_json/get.md).
The [clang-tidy modernize-nlohmann-json-explicit-conversions](../../integration/clang-tidy.md#modernize-nlohmann-json-explicit-conversions)
check can help to do this automatically.

!!! hint "CMake option"

Expand Down
113 changes: 113 additions & 0 deletions docs/mkdocs/docs/integration/clang-tidy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# Clang-Tidy

The library comes with a [clang-tidy](https://clang.llvm.org/extra/clang-tidy/) plugin.
It is disabled by default but can be enabled by enabling the `JSON_ClangTidyPlugin` [CMake option](cmake.md#json_clangtidyplugin).
Clang-tidy [Plugins](https://clang.llvm.org/extra/clang-tidy/ExternalClang-TidyExamples.html) are only supported by Clang 16 and later.

## Building the plugin

You will need to have the development files matching your version of clang-tidy installed to build the plugin.
For example, if you are running on a Debian-derived Linux distribution:

```sh
apt install clang-tidy libclang-dev
```
but if this installs a version that is older than Clang 16 then you might be able to specify a newer version. For example:
```sh
apt install clang-tidy-19 libclang-19-dev
```

```sh
mkdir build
cd build
cmake -DJSON_ClangTidyPlugin=ON ..
cmake -build .
```

# Running the plugin
To tell clang-tidy to use the plugin you must pass a path to it as an argument to the `-load` option.
For example, you can run clang-tidy with only the _modernize-nlohmann-json-explicit-conversion_ check using the plugin on a single file with:
```sh
clang-tidy -load .../path/to/build/clang_tidy_plugin/libNlohmannJsonClangTidyPlugin.so -checks='-*,modernize-nlohmann-json-explicit-conversions` -fix source.cpp
clang-tidy
```
or you can create a `.clang-tidy` file to enable the checks you require.

# Checks

At the moment the plugin contains only a single check.

## modernize-nlohmann-json-explicit-conversions

This check converts code that takes advantage of [implicit conversions](../api/macros/json_use_implicit_conversions.md) to use explicit `get()` calls using the correct templated type.
For example, it turns:
```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
```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:
```c++
void f(const nlohmann::json &j)
{
std::optional<int> oi;
const auto &it = j.find("value");
if (it != j.end())
oi = *it;
// ...
}
```
After you have run this check you can set [JSON_USE_IMPLICIT_CONVERSIONS=0](../api/macros/json_use_implicit_conversions.md) to find any occurrences that the check have not been fixed automatically.

### Limitations

#### Typedefs may leak platform specifics

The check sees through `typedef` which can cause types that ought to vary by platform to become fixed by the check. This is particularly a problem when using the fixed-sized types from `<cstdint>` or other platform-specific types like `size_t` and `off_t`. For example, converting:
```c++
uint64_t u64 = j.at("value");
```
in a 64-bit Linux environment will result in:
```c++
uint64_t u64 = j.at("value").get<unsigned long>();
```
but in a 32-bit Linux environment will result in:
```c++
uint64_t u64 = j.at("value").get<unsigned long long>();
```

This could cause serious bugs. If you use such types in code you apply the check to it is recommended that you run on all your target platforms and compare the results.

Other more-benign uses of `typedef` types may also cause the resultant code to look uglier. The check automatically turns `std::basic_string<char>` back into `std::string` since that is so common.

#### Redundant casts

Code using the library may use casts to force the correct type. Such code will be fixed but the unnecessary cast will remain, which could cause confusion and be brittle when the code is changed. For example, converting:
```c++
const auto value = static_cast<int>(j["value"])
```
will yield:
```
const auto value = static_cast<int>(j["value"].get<int>())
```
Loading