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

[Alexander Kornienko]

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:23
@@ +22,3 @@
+template <typename T>
+StringRef getText(const ast_matchers::MatchFinder::MatchResult &Result, T const &Node) {
+    return Lexer::getSourceText(
----------------
clang-format?

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:50
@@ +49,3 @@
+// when it already begins with '&'.  Return empty string on failure.
+std::string formatDereference(const ast_matchers::MatchFinder::MatchResult &Result,
+                              const Expr &ExprNode) {
----------------
clang-format?

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:58
@@ +57,3 @@
+  }
+  const std::string Text = getText(Result, ExprNode);
+  if (Text.empty()) return std::string();
----------------
It's better to make this a `StringRef` and replace the concatenation of strings below with:
      return (llvm::Twine("*(") + Text + ")").str().

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:129
@@ +128,3 @@
+{
+    const CallExpr *Call = Result.Nodes.getStmtAs<CallExpr>("call");
+    const Expr *Arg = Result.Nodes.getStmtAs<Expr>("arg");
----------------
This is a good place to use `auto` or `const auto *`.

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:131
@@ +130,3 @@
+    const Expr *Arg = Result.Nodes.getStmtAs<Expr>("arg");
+    const bool Arrow = Result.Nodes.getStmtAs<MemberExpr>("member")->isArrow();
+    // Replace the "call" node with the "arg" node, prefixed with '*'
----------------
s/const bool/bool/

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:134
@@ +133,3 @@
+    // if the call was using '->' rather than '.'.
+    const std::string ArgText = Arrow ?
+            formatDereference(Result, *Arg) :
----------------
s/const std::string/std::string/

================
Comment at: clang-tidy/readability/RemoveCStrCall.cpp:136
@@ +135,3 @@
+            formatDereference(Result, *Arg) :
+            static_cast<std::string>(getText(Result, *Arg));
+    if (ArgText.empty()) return;
----------------
Why `static_cast` instead of `.str()`?

================
Comment at: clang-tidy/readability/RemoveCStrCall.h:20
@@ +19,3 @@
+/// \brief Finds unnecessary calls to std::string::c_str().
+class RemoveCStrCall : public ClangTidyCheck {
+public:
----------------
The fact that it removes something doesn't make it much different from other checks, and it doesn't deserve a place in the name. Let's call this check "readability-redundant-cstr" or "readability-redundant-cstr-calls" instead (and the class `RedundantCStrCheck` or `RedundantCStrCallsCheck`).

I posted a relevant comment earlier, but it has fallen through the cracks.

http://reviews.llvm.org/D7318

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