[PATCH] D15411: [clang-tidy] Check for suspicious string assignments.

[Gábor Horváth via cfe-commits]

xazax.hun marked 7 inline comments as done.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:19
@@ +18,3 @@
+namespace {
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
----------------
aaron.ballman wrote:
> I think this would be reasonable to add to ASTMatchers.h, but with the name isAnyCharacterType.
I choose the name isAnyCharacter to be consistent with isInteger which also lacks the Type suffix. Do you still prefer the isAnyCharacterType name?

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:44
@@ +43,3 @@
+  SourceLocation Loc = Argument->getLocStart();
+
+  auto Diag = diag(Loc, "probably missing to_string operation");
----------------
aaron.ballman wrote:
> Would it make sense to allow = 0 or += 0 for null terminators? Requiring '\0' isn't the end of the world, but I think 0 is pretty common for that particular case.
The = 0 or += 0 will not compile due to ambiguous overload (it could both be converted to const CharT * or CharT).

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:46
@@ +45,3 @@
+  auto Diag = diag(Loc, "probably missing to_string operation");
+  if (!Loc.isMacroID() && getLangOpts().CPlusPlus11) {
+    Diag << FixItHint::CreateInsertion(Loc, "std::to_string(")
----------------
aaron.ballman wrote:
> I don't think to_string() is required for all cases. For instance:
> ```
> s += 12; // --> s += "12";
> s = 32; // --> s = ' '; or s = "32";
> s = 4; // --> s = '4'; or s = "4";
> s += some_integer; // --> s += std::to_string(some_integer);
> ```
> This is also currently doing the wrong thing for std::wstring objects, or std::basic_string<char32_t>, etc.
I think it might be even better to not to suggest fixits in this case, since there are some false positives. But if you think it is ok to have fixits I will come up some heuristics like:
For one digit constants use 'const' or L'const'.
For more than one digit constants use "const" or L"const".
For unknown character types do not recommend fixits.
For other cases and C++11 use to_string or to_wstring.


http://reviews.llvm.org/D15411