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

[Aaron Ballman via cfe-commits]

aaron.ballman added a reviewer: klimek.

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

(CCing in Manuel for his opinion as well.)

I like the consistency idea, but it was my understanding that we want all AST matcher functions to match the C++ API they are exposing, which is why I still have a slight preference for isAnyCharacterType().

================
Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:45
@@ +44,3 @@
+
+  auto Diag =
+      diag(Loc, "an integer is interpreted as a character code when assigning "
----------------
> The = 0 or += 0 will not compile due to ambiguous overload (it could both be converted to const CharT * or CharT).

Excellent point. :-)

================
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 "
----------------
alexfh wrote:
> I'd suggest to still add fixits. The heuristics you suggest seems reasonable. Fine for a follow-up.
> I'd suggest to still add fixits. The heuristics you suggest seems reasonable. Fine for a follow-up.

Agreed.


http://reviews.llvm.org/D15411