[clang-tidy] Fix redundant-string-init check with msvc 14 headers.

[etienneb]

Summary

The string constructors are not defined using optional parameters and are not recognized by the redundant-string-init checker.

The following patch fixes the redundant-string-init checker for the Visual Studio 14 headers file.
The matcher now accept both variant (with 1 and 2 parameters).

Also added new unittests.

Similar issue than: review

In the xstring.h header, the constructors are defined this way:

basic_string(const _Myt& _Right) [...]
basic_string(const _Myt& _Right, const _Alloc& _Al) [...]

[hokein]

The patch looks good to me.

[alexfh]

LG

[etienneb]

Rebased + fixing invalid paths.

The diff paths were wrong, invalid diff.

clang-tidy/readability/RedundantStringInitCheck.cpp

 namespace clang {
 namespace tidy {
 namespace readability {
 
 namespace {
 
 AST_MATCHER(StringLiteral, lengthIsZero) { return Node.getLength() == 0; }
 
+AST_MATCHER_P(Expr, ignoringImplicit,
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  return InnerMatcher.matches(*Node.IgnoreImplicit(), Finder, Builder);
+}
+
 } // namespace
 
 void RedundantStringInitCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
-  const auto StringCtorExpr = cxxConstructExpr(
+  // Match string constructor.
+  const auto StringConstructorExpr = expr(anyOf(
+      cxxConstructExpr(argumentCountIs(1),
+                       hasDeclaration(cxxMethodDecl(hasName("basic_string")))),
+      // If present, the second argument is the alloc object which must not
+      // be present explicitly.
+      cxxConstructExpr(argumentCountIs(2),
                        hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
-      argumentCountIs(2),
-      hasArgument(0, ignoringParenImpCasts(stringLiteral(lengthIsZero()))),
-      hasArgument(1, cxxDefaultArgExpr()));
+                       hasArgument(1, cxxDefaultArgExpr()))));
+
+  // Match a string constructor expression with an empty string literal.
+  const auto EmptyStringCtorExpr =
+      cxxConstructExpr(StringConstructorExpr,
+          hasArgument(0, ignoringParenImpCasts(
+                             stringLiteral(lengthIsZero()))));
+
+  const auto EmptyStringCtorExprWithTemporaries =
+      expr(ignoringImplicit(
+          cxxConstructExpr(StringConstructorExpr,
+              hasArgument(0, ignoringImplicit(EmptyStringCtorExpr)))));
 
-  // string foo = "";
-  // OR
-  // string bar("");
+  // Match a variable declaration with an empty string literal as initializer.
+  // Examples:
+  //     string foo = "";
+  //     string bar("");
   Finder->addMatcher(
       namedDecl(varDecl(hasType(cxxRecordDecl(hasName("basic_string"))),
                         hasInitializer(
-                            expr(anyOf(StringCtorExpr,
-                                       exprWithCleanups(has(expr(anyOf(
-                                           StringCtorExpr,
-                                           cxxConstructExpr(hasArgument(
-                                               0, cxxBindTemporaryExpr(has(
-                                                      StringCtorExpr))))))))))
-                                .bind("expr"))))
+                            expr(anyOf(EmptyStringCtorExpr,
+                                       EmptyStringCtorExprWithTemporaries))
           .bind("decl"),
       this);
 }
 
 void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CtorExpr = Result.Nodes.getNodeAs<Expr>("expr");
   const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl");
   diag(CtorExpr->getExprLoc(), "redundant string initialization")
       << FixItHint::CreateReplacement(CtorExpr->getSourceRange(),
                                       Decl->getName());
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

test/clang-tidy/readability-redundant-string-init-msvc.cpp

+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
+
+namespace std {
+template <typename T>
+class allocator {};
+template <typename T>
+class char_traits {};
+template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C>>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string&);
+  // MSVC headers define two constructors instead of using optional arguments.
+  basic_string(const C *);
+  basic_string(const C *, const A &);
+  ~basic_string();
+};
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+}
+
+void f() {
+  std::string a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization [readability-redundant-string-init]
+  // CHECK-FIXES: std::string a;
+  std::string b("");
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string b;
+  std::string c = R"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string c;
+  std::string d(R"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string d;
+
+  std::string u = "u";
+  std::string w("w");
+  std::string x = R"(x)";
+  std::string y(R"(y)");
+  std::string z;
+}
+
+void g() {
+  std::wstring a = L"";
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring a;
+  std::wstring b(L"");
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring b;
+  std::wstring c = LR"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring c;
+  std::wstring d(LR"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring d;
+
+  std::wstring u = L"u";
+  std::wstring w(L"w");
+  std::wstring x = LR"(x)";
+  std::wstring y(LR"(y)");
+  std::wstring z;
+}
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
+
+namespace std {
+template <typename T>
+class allocator {};
+template <typename T>
+class char_traits {};
+template <typename C, typename T = std::char_traits<C>, typename A = std::allocator<C>>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string&);
+  // MSVC headers define two constructors instead of using optional arguments.
+  basic_string(const C *);
+  basic_string(const C *, const A &);
+  ~basic_string();
+};
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+}
+
+void f() {
+  std::string a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization [readability-redundant-string-init]
+  // CHECK-FIXES: std::string a;
+  std::string b("");
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string b;
+  std::string c = R"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string c;
+  std::string d(R"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string d;
+
+  std::string u = "u";
+  std::string w("w");
+  std::string x = R"(x)";
+  std::string y(R"(y)");
+  std::string z;
+}
+
+void g() {
+  std::wstring a = L"";
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring a;
+  std::wstring b(L"");
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring b;
+  std::wstring c = LR"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring c;
+  std::wstring d(LR"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES: std::wstring d;
+
+  std::wstring u = L"u";
+  std::wstring w(L"w");
+  std::wstring x = LR"(x)";
+  std::wstring y(LR"(y)");
+  std::wstring z;
+}

test/clang-tidy/readability-redundant-string-init.cpp

void h() {
 
   N
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: redundant string initialization
   // CHECK-FIXES: N
   N
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: redundant string initialization
   // CHECK-FIXES: N
 }
+
+typedef std::string MyString;
+#define STRING MyString
+#define DECL_STRING(name, val) STRING name = val
+
+void i() {
+  MyString a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: redundant string initialization
+  // CHECK-FIXES: MyString a;
+  STRING b = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: redundant string initialization
+  // CHECK-FIXES: STRING b;
+  MyString c = "" "" "";
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: redundant string initialization
+  // CHECK-FIXES: MyString c;
+  STRING d = "" "" "";
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: redundant string initialization
+  // CHECK-FIXES: STRING d;
+  DECL_STRING(e, "");
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+
+  MyString f = "u";
+  STRING g = "u";
+  DECL_STRING(h, "u");
+}
+
+#define EMPTY_STR ""
+void j() {
+  std::string a(EMPTY_STR);
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string a;
+  std::string b = (EMPTY_STR);
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string b;
+
+  std::string c(EMPTY_STR "u" EMPTY_STR);
+}
+
+void k() {
+  std::string a = "", b = "", c = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:23: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-3]]:31: warning: redundant string initialization
+  // CHECK-FIXES: std::string a, b, c;
+
+  std::string d = "u", e = "u", f = "u";
+}