[PATCH] Refactor: Move remove-cstr-calls from a standalone executable to a clang-tidy check readability-redundant-string-cstr

[Alexander Kornienko]

The code looks fine now, but if you don't mind, I'd ask you to fix a few more nits (mostly left from the original tool's code). Once done, I'd be happy to commit the patch for you.

Thanks!


================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:38
@@ +37,3 @@
+  }
+  if (const auto op = dyn_cast<CXXOperatorCallExpr>(&ExprNode)) {
+    return op->getNumArgs() == 2 && op->getOperator() != OO_PlusPlus &&
----------------
s/const auto/const auto*/

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:38
@@ +37,3 @@
+  }
+  if (const auto op = dyn_cast<CXXOperatorCallExpr>(&ExprNode)) {
+    return op->getNumArgs() == 2 && op->getOperator() != OO_PlusPlus &&
----------------
alexfh wrote:
> s/const auto/const auto*/
Use LLVM naming style: `Op`

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:51
@@ +50,3 @@
+                  const Expr &ExprNode) {
+  if (const auto Op = dyn_cast<clang::UnaryOperator>(&ExprNode)) {
+    if (Op->getOpcode() == UO_AddrOf) {
----------------
s/const auto/const auto*/

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:67
@@ +66,3 @@
+
+const char *const StringConstructor =
+    "::std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
----------------
const char StringConstructor[] = ...

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:71
@@ +70,3 @@
+
+const char *const StringCStrMethod =
+    "::std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
----------------
ditto

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:92
@@ +91,3 @@
+          hasArgument(
+              0, id("call", memberCallExpr(
+                                callee(id("member", memberExpr())),
----------------
`id` is an old-style construct, now we prefer using `.bind()`:

  id("...", X) -> X.bind("...")

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:95
@@ +94,3 @@
+                                callee(methodDecl(hasName(StringCStrMethod))),
+                                on(id("arg", expr()))))),
+          // The second argument is the alloc object which must not be
----------------
ditto

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:116
@@ +115,3 @@
+          hasArgument(
+              0, id("call", memberCallExpr(
+                                callee(id("member", memberExpr())),
----------------
ditto

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:119
@@ +118,3 @@
+                                callee(methodDecl(hasName(StringCStrMethod))),
+                                on(id("arg", expr())))))),
+      this);
----------------
ditto

================
Comment at: clang-tidy/readability/RedundantStringCStrCheck.cpp:124
@@ +123,3 @@
+void RedundantStringCStrCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto Call = Result.Nodes.getStmtAs<CallExpr>("call");
+  const auto Arg = Result.Nodes.getStmtAs<Expr>("arg");
----------------
const auto*

================
Comment at: test/clang-tidy/readability-redundant-string-cstr.cpp:24
@@ +23,3 @@
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^ *}}f1(s);{{$}}
+}
----------------
nit: The number of spaces here should remain the same, so I'd better replace `{{^ *}}` with `{{^  }}`.

http://reviews.llvm.org/D7318

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/