[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:82
+  if (Call->getNumArgs() == 1) {
+    diag(Op->getBeginLoc(), "call to absl::StrCat does nothing");
+    return;
----------------
JonasToth wrote:
> please use 'absl::StrCat' in the diagnostics (same below) to signify that it is code.
"does nothing" is not quite correct either; it does something, just not what the user expects. How about `call to 'absl::StrCat' has no effect`?


================
Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:91-92
+  diag(Op->getBeginLoc(),
+       "please use absl::StrAppend instead of absl::StrCat when appending to "        
+       "an existing string")
+      << FixItHint::CreateReplacement(
----------------
This wins the award for "kindest diagnostic message". :-D How about: `call 'absl::StrAppend' instead of 'absl::StrCat' when appending to a string`?

That said, the diagnostic doesn't say *why* the code is wrong in the first place. The documentation doesn't really go into it either. Is it a correctness issue, a performance issue, something else?


https://reviews.llvm.org/D51061