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

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:20
@@ +19,3 @@
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
+
----------------
I prefer `isAnyCharacter` for consistency with `isInteger`. I'd also suggest to move the matcher to ASTMatchers.h eventually.

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:47
@@ +46,3 @@
+      diag(Loc, "an integer is interpreted as a character code when assigning "
+                "it to a string; if this is intended, cast the integer to the "
+                "appropriate character type; if you want a string "
----------------
I'd suggest to still add fixits. The heuristics you suggest seems reasonable. Fine for a follow-up.

================
Comment at: docs/clang-tidy/checks/misc-string-integer-assignment.rst:2
@@ +1,3 @@
+misc-string-integer-assignment
+======================
+
----------------
nit: Please add more =s

================
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 an integer to ``basic_string``.
----------------
"The source of the problem" seems slightly confusing here (we didn't yet define "the problem"). I'd say slightly differently:

  The check finds assignments of an integer to ``std::basic_string<T>`` (``std::string``, ``std::wstring``, etc.). The source of the problem is the following assignment operator of ``std::basic_string``:

  .. code:: c++
    basic_string& operator=( CharT ch );

  and the fact that numeric types can be implicitly converted to most character types.

================
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:7
@@ +6,3 @@
+  basic_string(const T*) {}
+  basic_string& operator=(T) {
+    return *this;
----------------
I don't think the definitions of the methods are helpful here.

================
Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:26
@@ +25,3 @@
+int main() {
+  std::string s{"foobar"};
+  std::wstring ws{L"foobar"};
----------------
Why do you need to initialize the strings?


http://reviews.llvm.org/D15411