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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:19
@@ +18,3 @@
+namespace {
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
----------------
I think this would be reasonable to add to ASTMatchers.h, but with the name isAnyCharacterType.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:44
@@ +43,3 @@
+  SourceLocation Loc = Argument->getLocStart();
+
+  auto Diag = diag(Loc, "probably missing to_string operation");
----------------
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.

================
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(")
----------------
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.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.h:18
@@ +17,3 @@
+
+/// Finds instances where integer is assigned to a string.
+///
----------------
"where an integer" instead of "where integer".

================
Comment at: docs/clang-tidy/checks/misc-string-integer-assignment.rst:9
@@ +8,3 @@
+
+The source of the problem is that, the numeric types can be implicity casted to most of the
+character types. It possible to assign integers to ``basic_string``.
----------------
Can remove the comma.

================
Comment at: docs/clang-tidy/checks/misc-string-integer-assignment.rst:10
@@ +9,3 @@
+The source of the problem is that, the numeric types can be implicity casted to most of the
+character types. It possible to assign integers to ``basic_string``.
+
----------------
"It is possible to assign an integer"

================
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:39
@@ +38,2 @@
+  s += (char)6;
+}
----------------
I would also like to see some tests for std::wstring.


http://reviews.llvm.org/D15411