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

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL255630: [clang-tidy] Check for suspicious string 
assignments. (authored by xazax).

Changed prior to commit:
  http://reviews.llvm.org/D15411?vs=42710=42824#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15411

Files:
  clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/misc/StringIntegerAssignmentCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/StringIntegerAssignmentCheck.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-string-integer-assignment.rst
  clang-tools-extra/trunk/test/clang-tidy/misc-string-integer-assignment.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/misc-string-integer-assignment.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-string-integer-assignment.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-string-integer-assignment.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s misc-string-integer-assignment %t
+
+namespace std {
+template
+struct basic_string {
+  basic_string& operator=(T);
+  basic_string& operator=(basic_string);
+  basic_string& operator+=(T);
+  basic_string& operator+=(basic_string);
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+}
+
+typedef int MyArcaneChar;
+
+int main() {
+  std::string s;
+  std::wstring ws;
+  int x = 5;
+
+  s = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s = '6';{{$}}
+  s = 66;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a chara
+// CHECK-FIXES: {{^}}  s = "66";{{$}}
+  s = x;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a chara
+// CHECK-FIXES: {{^}}  s = std::to_string(x);{{$}}
+  s = 'c';
+  s = static_cast(6);
+
+// +=
+  ws += 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a chara
+// CHECK-FIXES: {{^}}  ws += L'6';{{$}}
+  ws += 66;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a chara
+// CHECK-FIXES: {{^}}  ws += L"66";{{$}}
+  ws += x;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a chara
+// CHECK-FIXES: {{^}}  ws += std::to_wstring(x);{{$}}
+  ws += L'c';
+  ws += (wchar_t)6;
+
+  std::basic_string as;
+  as = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+// CHECK-FIXES: {{^}}  as = 6;{{$}}
+
+}
Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
@@ -17,6 +17,7 @@
   NonCopyableObjects.cpp
   SizeofContainerCheck.cpp
   StaticAssertCheck.cpp
+  StringIntegerAssignmentCheck.cpp
   SwappedArgumentsCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UndelegatedConstructor.cpp
Index: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
@@ -25,6 +25,7 @@
 #include "NonCopyableObjects.h"
 #include "SizeofContainerCheck.h"
 #include "StaticAssertCheck.h"
+#include "StringIntegerAssignmentCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UndelegatedConstructor.h"
@@ -68,6 +69,8 @@
 CheckFactories.registerCheck("misc-sizeof-container");
 CheckFactories.registerCheck(
 "misc-static-assert");
+CheckFactories.registerCheck(
+"misc-string-integer-assignment");
 CheckFactories.registerCheck(
 "misc-swapped-arguments");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/misc/StringIntegerAssignmentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/StringIntegerAssignmentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/StringIntegerAssignmentCheck.cpp
@@ -0,0 +1,85 @@
+//===--- StringIntegerAssignmentCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StringIntegerAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void 

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

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

xazax.hun added a comment.

In http://reviews.llvm.org/D15411#310539, @alexfh wrote:

> Thank you for the new awesome check!


Thank you for the review. I committed the new matcher to clang as well. I went 
with isAnyCharacter name for now, but we can rename it anytime.


Repository:
  rL LLVM

http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good with a few nits.

Thank you for the new awesome check!



Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:74
@@ +73,3 @@
+return;
+  } else if (IsLiteral) {
+Diag << FixItHint::CreateInsertion(Loc, IsWideCharType ? "L\"" : "\"")

Don't use `else` after a `return`.


Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:27
@@ +26,3 @@
+  s = 66;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a 
character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s = "66";{{$}}

I'd also remove the check name from all CHECK lines after the first one to fit 
the CHECK lines to 80 columns.


Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:46
@@ +45,3 @@
+  ws += L'c';
+  ws += (wchar_t)6;
+

LegalizeAdulthood wrote:
> Use `static_cast<>` instead of C-style cast?
I think, both casts should be tested at least once. And it doesn't matter what 
cast style is used in the rest of the test.


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [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, 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [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`` 
(``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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

xazax.hun updated this revision to Diff 42710.
xazax.hun marked 4 inline comments as done.
xazax.hun added a comment.

- Reimplemented fixits.
- Addressed the review comments.


http://reviews.llvm.org/D15411

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-integer-assignment.rst
  test/clang-tidy/misc-string-integer-assignment.cpp

Index: test/clang-tidy/misc-string-integer-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-string-integer-assignment.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s misc-string-integer-assignment %t
+
+namespace std {
+template
+struct basic_string {
+  basic_string& operator=(T);
+  basic_string& operator=(basic_string);
+  basic_string& operator+=(T);
+  basic_string& operator+=(basic_string);
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+}
+
+typedef int MyArcaneChar;
+
+int main() {
+  std::string s;
+  std::wstring ws;
+  int x = 5;
+
+  s = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s = '6';{{$}}
+  s = 66;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s = "66";{{$}}
+  s = x;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s = std::to_string(x);{{$}}
+  s = 'c';
+  s = (char)6;
+
+// +=
+  ws += 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  ws += L'6';{{$}}
+  ws += 66;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  ws += L"66";{{$}}
+  ws += x;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  ws += std::to_wstring(x);{{$}}
+  ws += L'c';
+  ws += (wchar_t)6;
+
+  std::basic_string as;
+  as = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  as = 6;{{$}}
+
+}
Index: docs/clang-tidy/checks/misc-string-integer-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-integer-assignment.rst
@@ -0,0 +1,31 @@
+misc-string-integer-assignment
+==
+
+The check finds assignments of an integer to ``std::basic_string``
+(``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 );
+
+Numeric types can be implicity casted to character types.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = 6;
+  s = x;
+
+Use the appropriate conversion functions or character literals.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = '6';
+  s = std::to_string(x);
+
+In order to suppress false positives, use an explicit cast.
+
+.. code:: c++
+  std::string s;
+  s = static_cast(6);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -48,6 +48,7 @@
misc-non-copyable-objects
misc-sizeof-container
misc-static-assert
+   misc-string-integer-assignment
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
misc-undelegated-constructor
Index: clang-tidy/misc/StringIntegerAssignmentCheck.h
===
--- /dev/null
+++ clang-tidy/misc/StringIntegerAssignmentCheck.h
@@ -0,0 +1,34 @@
+//===--- StringIntegerAssignmentCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds instances where an integer is assigned to a string.
+///
+/// For more details see:
+/// 

Re: [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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

xazax.hun added inline comments.


Comment at: clang-tidy/misc/StringIntegerAssignmentCheck.cpp:20
@@ +19,3 @@
+AST_MATCHER(QualType, isAnyCharacter) { return Node->isAnyCharacterType(); }
+} // namespace
+

aaron.ballman wrote:
> 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().
Adding this checker to ASTMatchers.h seems like a trivial modification. I think 
I will just add it and alter this patch once this check is accepted and you 
decided how to call it.


Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:26
@@ +25,3 @@
+int main() {
+  std::string s{"foobar"};
+  std::wstring ws{L"foobar"};

alexfh wrote:
> Why do you need to initialize the strings?
I do not need to initialize them. I made the tests more minimal.


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

xazax.hun updated this revision to Diff 42698.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.

- Addressed review comments.
- Removed fixit suggestions.


http://reviews.llvm.org/D15411

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-integer-assignment.rst
  test/clang-tidy/misc-string-integer-assignment.cpp

Index: test/clang-tidy/misc-string-integer-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-string-integer-assignment.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-string-integer-assignment %t
+
+namespace std {
+template
+struct basic_string {
+  basic_string(const T*) {}
+  basic_string& operator=(T) {
+return *this;
+  }
+  basic_string& operator=(basic_string) {
+return *this;
+  }
+  basic_string& operator+=(T) {
+return *this;
+  }
+  basic_string& operator+=(basic_string) {
+return *this;
+  }
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+}
+
+int main() {
+  std::string s{"foobar"};
+  std::wstring ws{L"foobar"};
+
+  s = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+  s = 'c';
+  s = (char)6;
+
+// +=
+  ws += 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: an integer is interpreted as a character code when assigning {{.*}} [misc-string-integer-assignment]
+  ws += L'c';
+  ws += (wchar_t)6;
+}
Index: docs/clang-tidy/checks/misc-string-integer-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-integer-assignment.rst
@@ -0,0 +1,30 @@
+misc-string-integer-assignment
+==
+
+The ``std::basic_string`` has an assignment operator with the following signature:
+
+.. code:: c++
+  basic_string& operator=( CharT ch );
+
+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``.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = 6;
+  s = x;
+
+Use the appropriate conversion functions or character literals.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = '6';
+  s = std::to_string(x);
+
+In order to suppress false positives, use an explicit cast.
+
+.. code:: c++
+  std::string s;
+  s = static_cast(6);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -48,6 +48,7 @@
misc-non-copyable-objects
misc-sizeof-container
misc-static-assert
+   misc-string-integer-assignment
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
misc-undelegated-constructor
Index: clang-tidy/misc/StringIntegerAssignmentCheck.h
===
--- /dev/null
+++ clang-tidy/misc/StringIntegerAssignmentCheck.h
@@ -0,0 +1,34 @@
+//===--- StringIntegerAssignmentCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds instances where an integer is assigned to a string.
+///
+/// For more details see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-assignment.html
+class StringIntegerAssignmentCheck : public ClangTidyCheck {
+public:
+  StringIntegerAssignmentCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+
Index: clang-tidy/misc/StringIntegerAssignmentCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/StringIntegerAssignmentCheck.cpp
@@ -0,0 +1,53 @@
+//===--- StringIntegerAssignmentCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

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

[Richard via cfe-commits]

LegalizeAdulthood added inline comments.


Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:46
@@ +45,3 @@
+  ws += L'c';
+  ws += (wchar_t)6;
+

Use `static_cast<>` instead of C-style cast?


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard via cfe-commits]

LegalizeAdulthood added inline comments.


Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:33
@@ +32,3 @@
+  s = 'c';
+  s = (char)6;
+

What happens if the test code had used `static_cast`?


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard via cfe-commits]

LegalizeAdulthood added a subscriber: LegalizeAdulthood.


Comment at: test/clang-tidy/misc-string-integer-assignment.cpp:25
@@ +24,3 @@
+int main() {
+  std::string s("foobar");
+

Use {} initialization perhaps?


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D15411#307274, @aaron.ballman wrote:

> In http://reviews.llvm.org/D15411#307030, @alexfh wrote:
>
> > BTW, Richard, is the possibility of assignment of an integer to a 
> > std::string a bug in the standard?
>
>
> This is the result of:
>
>   basic_string& operator=(charT c);
>   Returns: *this = basic_string(1,c).
>
>
> So I believe it is by design.


I see how the current behavior can be explained by the standard, however I'm 
not sure this was the intention of the standard to specifically allow this 
behavior.

And if it's unintended (and I'd say undesired), the standard could also be 
changed to enforce a mechanism to disallow assignment of std::string from other 
integer types (e.g. a templated deleted operator= enable_if'd for all integral 
types except for charT, or something like that).

Maybe it wouldn't be reasonable for some reason, but it's worth trying ;)


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [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:45
@@ +44,3 @@
+
+  auto Diag = diag(Loc, "probably missing to_string operation");
+  if (!Loc.isMacroID() && getLangOpts().CPlusPlus11) {

I'd expand the message with an explanation of what actually happens here. Also 
it's not clear what the "to_string operation" refers to. And one more thing I 
missed, is other character types.

So I'd go with something similar to: "an integer is interpreted as a character 
code when assigning it to a string; if this is intended, cast the integer to 
'%0'; if you want a string representation, use 
%select{std::to_string()|std::to_wstring()|appropriate conversion facility}1", 
where %0 should be substituted with `char`/`wchar_t`/whatever the specific 
character type is.

(and please add tests for other character types)


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [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, 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

xazax.hun updated this revision to Diff 42406.
xazax.hun added a comment.

Minor update to docs.


http://reviews.llvm.org/D15411

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringAssignmentCheck.cpp
  clang-tidy/misc/StringAssignmentCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-assignment.rst
  test/clang-tidy/misc-string-assignment.cpp

Index: test/clang-tidy/misc-string-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-string-assignment.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-string-assignment %t
+
+namespace std {
+template
+struct basic_string {
+  basic_string(const T*) {}
+  basic_string& operator=(T) {
+return *this;
+  }
+  basic_string& operator=(basic_string) {
+return *this;
+  }
+  basic_string& operator+=(T) {
+return *this;
+  }
+  basic_string& operator+=(basic_string) {
+return *this;
+  }
+};
+
+typedef basic_string string;
+}
+
+int main() {
+  std::string s("foobar");
+
+  s = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: probably missing to_string operation [misc-string-assignment]
+// CHECK-FIXES: {{^}}  s = std::to_string(6);{{$}}
+  s = 'c';
+  s = (char)6;
+
+// +=
+  s += 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: probably missing to_string operation [misc-string-assignment]
+// CHECK-FIXES: {{^}}  s += std::to_string(6);{{$}}
+  s += 'c';
+  s += (char)6;
+}
Index: docs/clang-tidy/checks/misc-string-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-assignment.rst
@@ -0,0 +1,30 @@
+misc-string-assignment
+==
+
+The ``std::basic_string`` has an assignment operator with the following signature:
+
+.. code:: c++
+  basic_string& operator=( CharT ch );
+
+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``.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = 6;
+  s = x;
+
+Use the appropriate conversion functions or character literals.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = '6';
+  s = std::to_string(x);
+
+In order to suppress false positives, use an explicit cast.
+
+.. code:: c++
+  std::string s;
+  s = static_cast(6);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -47,6 +47,7 @@
misc-non-copyable-objects
misc-sizeof-container
misc-static-assert
+   misc-string-assignment
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
misc-undelegated-constructor
Index: clang-tidy/misc/StringAssignmentCheck.h
===
--- /dev/null
+++ clang-tidy/misc/StringAssignmentCheck.h
@@ -0,0 +1,34 @@
+//===--- StringAssignmentCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_ASSIGNMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_ASSIGNMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds instances where integer is assigned to a string.
+///
+/// For more details see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-assignment.html
+class StringAssignmentCheck : public ClangTidyCheck {
+public:
+  StringAssignmentCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_ASSIGNMENT_H
+
Index: clang-tidy/misc/StringAssignmentCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/StringAssignmentCheck.cpp
@@ -0,0 +1,50 @@
+//===--- StringAssignmentCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StringAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void 

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

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

xazax.hun created this revision.
xazax.hun added a reviewer: alexfh.
xazax.hun added subscribers: dkrupp, cfe-commits.

It is possible to assign arbitrary integer types to strings. Sometimes it is 
the result of missing to_string call or apostrophes. 

This check aims to find such errors.

http://reviews.llvm.org/D15411

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringAssignmentCheck.cpp
  clang-tidy/misc/StringAssignmentCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-assignment.rst
  test/clang-tidy/misc-string-assignment.cpp

Index: test/clang-tidy/misc-string-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-string-assignment.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-string-assignment %t
+
+namespace std {
+template
+struct basic_string {
+  basic_string(const T*) {}
+  basic_string& operator=(T) {
+return *this;
+  }
+  basic_string& operator=(basic_string) {
+return *this;
+  }
+  basic_string& operator+=(T) {
+return *this;
+  }
+  basic_string& operator+=(basic_string) {
+return *this;
+  }
+};
+
+typedef basic_string string;
+}
+
+int main() {
+  std::string s("foobar");
+
+  s = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: probably missing to_string operation [misc-string-assignment]
+// CHECK-FIXES: {{^}}  s = std::to_string(6);{{$}}
+  s = 'c';
+  s = (char)6;
+
+// +=
+  s += 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: probably missing to_string operation [misc-string-assignment]
+// CHECK-FIXES: {{^}}  s += std::to_string(6);{{$}}
+  s += 'c';
+  s += (char)6;
+}
Index: docs/clang-tidy/checks/misc-string-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-assignment.rst
@@ -0,0 +1,24 @@
+misc-string-assignment
+==
+
+The ``std::basic_string`` has an assignment operator with the following signature:
+
+.. code:: c++
+  basic_string& operator=( CharT ch );
+
+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``.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = 6;
+  s = x;
+
+Use the appropriate conversion functions or character literals.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = '6';
+  s = std::to_string(x);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -47,6 +47,7 @@
misc-non-copyable-objects
misc-sizeof-container
misc-static-assert
+   misc-string-assignment
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
misc-undelegated-constructor
Index: clang-tidy/misc/StringAssignmentCheck.h
===
--- /dev/null
+++ clang-tidy/misc/StringAssignmentCheck.h
@@ -0,0 +1,34 @@
+//===--- StringAssignmentCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_ASSIGNMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_ASSIGNMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds instances where integer is assigned to a string.
+///
+/// For more details see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-assignment.html
+class StringAssignmentCheck : public ClangTidyCheck {
+public:
+  StringAssignmentCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_ASSIGNMENT_H
+
Index: clang-tidy/misc/StringAssignmentCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/StringAssignmentCheck.cpp
@@ -0,0 +1,50 @@
+//===--- StringAssignmentCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StringAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace 

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

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

That's quite a surprising behavior. Looks like a bug in the library 
implementation (or in the standard) to me.

But the check is awesome. Thank you!



Comment at: clang-tidy/misc/StringAssignmentCheck.cpp:21
@@ +20,3 @@
+void StringAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+  auto Matcher = cxxOperatorCallExpr(
+  anyOf(hasOverloadedOperatorName("="), hasOverloadedOperatorName("+=")),

Please only register the matcher for C++ code (see almost any other check for 
an example). It also seems that you don't need the `Matcher` variable.


Comment at: clang-tidy/misc/StringAssignmentCheck.cpp:33
@@ +32,3 @@
+
+  if (!ArgType->isIntegerType() || ArgType->isAnyCharacterType())
+return;

The `isIntegerType()` check can be moved to the matcher. I'd also add a matcher 
for `isAnyCharacterType` and move that check to the matcher as well. The latter 
is fine for a follow-up.


Comment at: clang-tidy/misc/StringAssignmentCheck.cpp:36
@@ +35,3 @@
+  SourceLocation Loc = Argument->getLocStart();
+  SmallVector Hints;
+  if (!Loc.isMacroID()) {

Instead of storing the fixits in an array, I'd organize the code as follows:

  auto Diag = diag(...);
  if (!Loc.isMacroID()) {
Diag << FixItHint::CreateInsertion(...) << FixItHint::CreateInsertion(...);
  }


Comment at: clang-tidy/misc/StringAssignmentCheck.cpp:38
@@ +37,3 @@
+  if (!Loc.isMacroID()) {
+Hints.push_back(FixItHint::CreateInsertion(Loc, "std::to_string("));
+Hints.push_back(FixItHint::CreateInsertion(

Please check that we're in C++11 mode before suggesting this fix.


Comment at: clang-tidy/misc/StringAssignmentCheck.h:1
@@ +1,2 @@
+//===--- StringAssignmentCheck.h - clang-tidy*- C++ 
-*-===//
+//

I suggest making the name more specific, e.g. `StringIntegerAssignmentCheck`. 
Same for the check name (`misc-string-integer-assignment`).


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

BTW, Richard, is the possibility of assignment of an integer to a std::string a 
bug in the standard?


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

xazax.hun updated this revision to Diff 42427.
xazax.hun marked 5 inline comments as done.
xazax.hun added a comment.

- Modified the patch according to the review.


http://reviews.llvm.org/D15411

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.cpp
  clang-tidy/misc/StringIntegerAssignmentCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-integer-assignment.rst
  test/clang-tidy/misc-string-integer-assignment.cpp

Index: test/clang-tidy/misc-string-integer-assignment.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-string-integer-assignment.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-string-integer-assignment %t
+
+namespace std {
+template
+struct basic_string {
+  basic_string(const T*) {}
+  basic_string& operator=(T) {
+return *this;
+  }
+  basic_string& operator=(basic_string) {
+return *this;
+  }
+  basic_string& operator+=(T) {
+return *this;
+  }
+  basic_string& operator+=(basic_string) {
+return *this;
+  }
+};
+
+typedef basic_string string;
+}
+
+int main() {
+  std::string s("foobar");
+
+  s = 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: probably missing to_string operation [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s = std::to_string(6);{{$}}
+  s = 'c';
+  s = (char)6;
+
+// +=
+  s += 6;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: probably missing to_string operation [misc-string-integer-assignment]
+// CHECK-FIXES: {{^}}  s += std::to_string(6);{{$}}
+  s += 'c';
+  s += (char)6;
+}
Index: docs/clang-tidy/checks/misc-string-integer-assignment.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-integer-assignment.rst
@@ -0,0 +1,30 @@
+misc-string-integer-assignment
+==
+
+The ``std::basic_string`` has an assignment operator with the following signature:
+
+.. code:: c++
+  basic_string& operator=( CharT ch );
+
+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``.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = 6;
+  s = x;
+
+Use the appropriate conversion functions or character literals.
+
+.. code:: c++
+  std::string s;
+  int x = 5965;
+  s = '6';
+  s = std::to_string(x);
+
+In order to suppress false positives, use an explicit cast.
+
+.. code:: c++
+  std::string s;
+  s = static_cast(6);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -47,6 +47,7 @@
misc-non-copyable-objects
misc-sizeof-container
misc-static-assert
+   misc-string-integer-assignment
misc-swapped-arguments
misc-throw-by-value-catch-by-reference
misc-undelegated-constructor
Index: clang-tidy/misc/StringIntegerAssignmentCheck.h
===
--- /dev/null
+++ clang-tidy/misc/StringIntegerAssignmentCheck.h
@@ -0,0 +1,34 @@
+//===--- StringIntegerAssignmentCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds instances where integer is assigned to a string.
+///
+/// For more details see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-assignment.html
+class StringIntegerAssignmentCheck : public ClangTidyCheck {
+public:
+  StringIntegerAssignmentCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_INTEGER_ASSIGNMENT_H
+
Index: clang-tidy/misc/StringIntegerAssignmentCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/StringIntegerAssignmentCheck.cpp
@@ -0,0 +1,57 @@
+//===--- StringIntegerAssignmentCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 

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

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

xazax.hun added a comment.

Thank you for the review. I have seen a few false positives in the llvm 
codebase. I agree that these kind of conversions are unfortunate but I am sure 
fixing this would cause a huge backward compatibility problem. Nontheless I 
also think that it might be something that is worth to consider.


http://reviews.llvm.org/D15411



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits