[PATCH] D19146: [clang-tidy] New checker to detect suspicious string constructor.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

I wonder whether `misc-swapped-arguments` can be tuned to handle these cases in a more generic way? The only missing part so far seems to be the lack of `cxxConstructExpr` support.


================
Comment at: clang-tidy/misc/StringConstructorCheck.cpp:104
@@ +103,3 @@
+  Finder->addMatcher(
+      cxxConstructExpr(LiteralStringConstructor, hasArgument(1, ZeroExpr))
+          .bind("empty-string"),
----------------
It seems to be wasteful to have several matchers sharing a bunch checks (the `LiteralStringConstructor` part). I'd suggest to merge the matchers and use anyOf to handle different sub-cases:
  cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
                   hasArgument(0, hasType(CharPtrType)),
                   hasArgument(1, hasType(isInteger())),
                   anyOf(cxxConstructExpr(hasArgument(1, ZeroExpr)).bind("empty-string"),
                         cxxConstructExpr(...).bind("case2"), ...)

================
Comment at: docs/clang-tidy/checks/misc-string-constructor.rst:15
@@ +14,3 @@
+
+  std::string('x', 50) str // should be std::string(50, 'x') 
+
----------------
nit: add a semicolon


http://reviews.llvm.org/D19146