njames93 updated this revision to Diff 238764.
njames93 added a comment.

- Updated to trunk, fix release notes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72448/new/

https://reviews.llvm.org/D72448

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -34,6 +34,12 @@
   std::string d(R"()");
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
   // CHECK-FIXES: std::string d;
+  std::string e{""};
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string e;
+  std::string f = {""};
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES: std::string f;
 
   std::string u = "u";
   std::string w("w");
@@ -227,3 +233,53 @@
   other::wstring e = L"";
   other::wstring f(L"");
 }
+
+class Foo {
+  std::string A = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES:  std::string A;
+  std::string B;
+  std::string C;
+  std::string D;
+  std::string E = "NotEmpty";
+
+public:
+  // Check redundant constructor where Field has a redundant initializer.
+  Foo() : A("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: redundant string initialization
+  // CHECK-FIXES:  Foo()  {}
+
+  // Check redundant constructor where Field has no initializer.
+  Foo(char) : B("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(char)  {}
+
+  // Check redundant constructor where Field has a valid initializer.
+  Foo(long) : E("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(long) : E() {}
+
+  // Check how it handles removing 1 initializer, and defaulting the other.
+  Foo(int) : B(""), E("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:21: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(int) :  E() {}
+
+  Foo(short) : B{""} {}
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(short)  {}
+
+  Foo(float) : A{""}, B{""} {}
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:23: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(float)  {}
+
+
+  // Check how it handles removing some redundant initializers while leaving
+  // valid initializers intact.
+  Foo(std::string Arg) : A(Arg), B(""), C("NonEmpty"), D(R"()"), E("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:34: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:56: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-3]]:66: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(std::string Arg) : A(Arg),  C("NonEmpty"),  E() {}
+};
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,6 +91,11 @@
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`readability-redundant-string-init
+  <clang-tidy/checks/readability-redundant-string-init>` check now supports a
+  `StringNames` option enabling its application to custom string classes. The 
+  check now detects in class initializers and constructor initializers which 
+  are deemed to be redundant.
 
 Renamed checks
 ^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -20,6 +20,30 @@
 
 const char DefaultStringNames[] = "::std::basic_string";
 
+static const CXXConstructExpr *
+getConstructExpr(const CXXCtorInitializer &CtorInit) {
+  const Expr *InitExpr = CtorInit.getInit();
+  if (const auto *CleanUpExpr = dyn_cast<ExprWithCleanups>(InitExpr))
+    InitExpr = CleanUpExpr->getSubExpr();
+  return dyn_cast<CXXConstructExpr>(InitExpr);
+}
+
+static llvm::Optional<SourceRange>
+getConstructExprArgRange(const CXXConstructExpr &Construct) {
+  SourceLocation B, E;
+  for (const Expr *Arg : Construct.arguments()) {
+    if (B.isInvalid()) {
+      B = Arg->getBeginLoc();
+    }
+    if (Arg->getEndLoc().isValid()) {
+      E = Arg->getEndLoc();
+    }
+  }
+  if (B.isInvalid() || E.isInvalid())
+    return llvm::None;
+  return SourceRange(B, E);
+}
+
 RedundantStringInitCheck::RedundantStringInitCheck(StringRef Name,
                                                    ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
@@ -65,29 +89,77 @@
       cxxConstructExpr(StringConstructorExpr,
                        hasArgument(0, ignoringImplicit(EmptyStringCtorExpr)));
 
+  const auto StringType = hasType(hasUnqualifiedDesugaredType(
+      recordType(hasDeclaration(cxxRecordDecl(hasStringTypeName)))));
+  const auto EmptyStringInit = expr(ignoringImplicit(
+      anyOf(EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)));
+
   // Match a variable declaration with an empty string literal as initializer.
   // Examples:
   //     string foo = "";
   //     string bar("");
   Finder->addMatcher(
       namedDecl(
-          varDecl(
-              hasType(hasUnqualifiedDesugaredType(recordType(
-                  hasDeclaration(cxxRecordDecl(hasStringTypeName))))),
-              hasInitializer(expr(ignoringImplicit(anyOf(
-                  EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)))))
-              .bind("vardecl"),
+          varDecl(StringType, hasInitializer(EmptyStringInit)).bind("vardecl"),
           unless(parmVarDecl())),
       this);
+  // Match a field declaration with an empty string literal as initializer.
+  Finder->addMatcher(
+      namedDecl(fieldDecl(StringType, hasInClassInitializer(EmptyStringInit))
+                    .bind("fieldDecl")),
+      this);
+  // Matches Constructor Initializers with an empty string literal as
+  // initializer.
+  // Examples:
+  //     Foo() : SomeString("") {}
+  Finder->addMatcher(
+      cxxCtorInitializer(
+          isWritten(),
+          forField(allOf(StringType, optionally(hasInClassInitializer(
+                                         EmptyStringInit.bind("empty_init"))))),
+          withInitializer(EmptyStringInit))
+          .bind("ctorInit"),
+      this);
 }
 
 void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *VDecl = Result.Nodes.getNodeAs<VarDecl>("vardecl");
-  // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
-  // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
-  SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
-  diag(VDecl->getLocation(), "redundant string initialization")
-      << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
+  if (const auto *VDecl = Result.Nodes.getNodeAs<VarDecl>("vardecl")) {
+    // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
+    // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
+    SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
+    diag(VDecl->getLocation(), "redundant string initialization")
+        << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
+  }
+  if (const auto *FDecl = Result.Nodes.getNodeAs<FieldDecl>("fieldDecl")) {
+    // FieldDecl's getSourceRange() spans 'string foo = ""'.
+    // So start at getLocation() to span just 'foo = ""'.
+    SourceRange ReplaceRange(FDecl->getLocation(), FDecl->getEndLoc());
+    diag(FDecl->getLocation(), "redundant string initialization")
+        << FixItHint::CreateReplacement(ReplaceRange, FDecl->getName());
+  }
+  if (const auto *CtorInit =
+          Result.Nodes.getNodeAs<CXXCtorInitializer>("ctorInit")) {
+    if (const FieldDecl *Member = CtorInit->getMember()) {
+      if (!Member->hasInClassInitializer() ||
+          Result.Nodes.getNodeAs<Expr>("empty_init")) {
+        // The String isn't declared in the class with an initializer or its
+        // declared with a redundant initializer, which will be removed. Either
+        // way the string will be default initialized, therefore we can remove
+        // the constructor initializer entirely.
+        diag(CtorInit->getMemberLocation(), "redundant string initialization")
+            << FixItHint::CreateRemoval(CtorInit->getSourceRange());
+        return;
+      }
+    }
+    const CXXConstructExpr *Construct = getConstructExpr(*CtorInit);
+    if (!Construct)
+      return;
+    if (llvm::Optional<SourceRange> RemovalRange =
+            getConstructExprArgRange(*Construct)) {
+      diag(CtorInit->getMemberLocation(), "redundant string initialization")
+          << FixItHint::CreateRemoval(*RemovalRange);
+    }
+  }
 }
 
 } // namespace readability
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to