Quote types and functions correctly in redundant-string-cstr.rst.

Documentation updates suggested by @PiotrZSL. Rebase.

In D145885#4187679, @PiotrZSL wrote:

My only complain is documentation, changes in code and tests are correct.
Build failed mainly due to conflicts in previous change.
It should get green once prev patch will land, and this will be re-based.

Rebase.

Rebase. No conflicts.

Thank you for your patience.

In D143342#4187641, @PiotrZSL wrote:

Correct commit message and this could land.

I can squash this with https://reviews.llvm.org/D143342 if required.

In D143342#4186809, @PiotrZSL wrote:

Code is fine, probably If would would write this, then I would bother to split into C++20 and C++2B simply because std::print wouldn't compile if it wouldn't be available. So I would just use hasAnyName.
But code looks fine, configuration could be added later...

In D145311#4184159, @PiotrZSL wrote:

It's fine, absl::StrCat returns std::string.
So those changes are correct.

Thanks, and sorry about the erroneous [PATCH] in the summary. :(

Thanks for the speedy re-review. I don't have commit permission, so please can you land this if you are happy to do so?

Fix comment in code as suggested by PiotrZSL. Improve lit check tests.

In D145730#4182949, @PiotrZSL wrote:

Also consider reducing commit message, instead just copying issue description.
Simple description about issue would be sufficient.

In D145313#4182637, @mikecrowe wrote:

Now a descendent of D145724.

Upload using arc diff so more context is available.

Re-upload using arc.

Rebase on top of fb7ef637a84652dbd3d973a1ba7db9470181b5aa (which is a
descendent of ae25e2f19decb94198301f0726ee613f945cc405 aka D144216, on
which this change relies.)

Whoops. I accidentally uploaded https://reviews.llvm.org/D145719 as a new change. :(

In D145313#4169790, @carlosgalvezp wrote:

Could you upload the patch with full context? I believe you need to do something like git show HEAD -U999999 as per the guidelines. Otherwise arc diff should do the job automatically. Reason I ask is that I cannot see the new line numbers referred to by the note comments - Phab says "Context not available".

In D144216#4169772, @carlosgalvezp wrote:

In D144216#4169764, @mikecrowe wrote:

I will double check that this is true once my current build is complete.

Yes, it's true. I stuck a #error in clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h and I saw the expected error from a file including <string>.

Awesome, thanks for checking, I wasn't aware we already had string.h. Then if we do have headers named the same as the standard headers, would it make sense to name this header cstring instead of string?

I will double check that this is true once my current build is complete.

The purpose of this header is to not include any standard header, and yet this is done in line 5 so it kinda defeats the purpose.

Sounds good, should we land this?

The changes that extend the <string> header further and use it in a few more checks are:

This new version uses size_t and size_type rather than the annoyingly-named "size" type that the old implementation in redundant-string-cstr.cpp used and is the basis for using the new <string> header in four further checks to be submitted as separate changes.

In D144216#4135431, @carlosgalvezp wrote:

Fair enough! I think we should merge this patch as is first, so that people can benefit from it already, and work through the technical debt afterwards.

If you're suggesting that I could use the new <string> header to replace declarations of basic_string etc. in other tests then I think that would be possible with some careful checking to make sure it include the necessary functionality. I think that would easier to review separately rather than in a single patch though.

In D144216#4133671, @Eugene.Zelenko wrote:

May be you could use heard in other test in same patch?

I ended up splitting out the std::format and std::print tests to their own file which meant that I didn't need to modify the existing redundant-string-cstr.cpp file in this commit. (Though of course I had to extract the <string> header in D144216 first.) Let me know if you don't like the split.

Thanks for the review and further suggestions.

Incorporate feedback from Nathan James:

The limitation about only transforming the first argument can be alleviated by using the forEachArgumentWithParam matcher

carlosgalvezp wrote:

Please document change in Release Notes, as well as in the check documentation, together with its limitations (can only handle 1 argument at a time).

This check can also work trivially for fmt::format and fmt::print[1] too (in fact, that's what I'm actually using), if the project would allow them to be added too.