New clang-tidy modernize-nlohmann-json-explicit-conversions check

[mikecrowe]

I've written a somewhat rudimentary clang-tidy check that will automatically convert existing implicit conversions to explicit conversions. In other words, it turns:

void f(const nlohmann::json &j1, const nlohmann::json &j2)
{
    int i = j1;
    double d = j2.at("value");
    std::cout << i << " " << d << "\n";
}

into

void f(const nlohmann::json &j1, const nlohmann::json &j2)
{
    int i = j1.get<int>();
    double d = j2.at("value").get<double>();
    std::cout << i << " " << d << "\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. Further examples can be found in the tests for the check.

I've used this check to replace 220 of 221 implicit conversions in our code. It was incapable of recognising an implicit conversion used to construct std::optional and left it alone. Even if it can't do the whole job, I think it's better than nothing.

Using this check may ease the path towards removing these implicit conversions in a future version of the library.

I raise it here in case:

1. you can see any flaws in it or suggest more test cases.

2. you think it's worth of a link from mkdocs/docs/api/macros/json_use_implicit_conversions.md.

I haven't tried to submit it to the upstream llvm-project. I'm not sure whether they'll accept checks for third-party (non-Google) libraries. If the response here is favourable then I can try.

[nlohmann]

Wow, this is awesome! I will give it a try soon. At first glance, I would definitely love to mention this in the documentation for implicit conversions (and also on https://json.nlohmann.me/integration/migration_guide). And yes, please try to submit this upstream! In any case, your project is an excellent starting point for integrating other changes that would make the transition to the next major version as smooth as possible.

[mikecrowe]

Thanks for the encouragement. I've pushed a new version with some more documentation. Did you get a chance to try it? I'll try submitting a pull request over the next few days. Hopefully one of the clang-tidy reviewers can help me make the check work for std::optional too. I'm currently having trouble coming up with a test case for it -- the LLVM lit test cases can't include system headers so I have to implement enough of std::optional and your types to make the tests valid.

[nlohmann]

I'm sorry for not getting back to this earlier. Unfortunately, I did not manage to build the project. Can you provide some basic documentation for this?

[mikecrowe]

Full build instructions are at https://llvm.org/docs/GettingStarted.html#getting-the-source-code-and-building-llvm but I think that I use something like:

mkdir build
cd build
cmake -G Ninja ../llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" -DCMAKE_BUILD_TYPE=Debug
ninja

and then you should find the binary in bin/clang-tidy. It does take a while to build!

[mikecrowe]

I proposed an improved version of the check for inclusion in clang-tidy in llvm/llvm-project#126425 and it seems that they don't want to add checks for third-party libraries. I'm going to try to turn the check into a plugin instead.