Commit 12d7500b authored by Mitchell Balan's avatar Mitchell Balan
Browse files

[clang-tidy] Give readability-redundant-string-init a customizable list of string types to fix 

Summary:
This patch adds a feature requested in https://reviews.llvm.org/D69238 to enable `readability-redundant-string-init` to take a list of strings to apply the fix to rather than hard-coding `basic_string`. It adds a `StringNames` option of semicolon-delimited names of string classes to which to apply this fix. Tests ensure this works with test class out::TestString as well as std::string and std::wstring as before. It should be applicable to llvm::StringRef, QString, etc.

Note: This commit was previously reverted due to a failing unit test. That test has been fixed in this version.

Reviewers: MyDeveloperDay, aaron.ballman, hokein, alexfh, JonasToth, gribozavr2

Patch by: poelmanc

Subscribers: gribozavr2, xazax.hun, Eugene.Zelenko, cfe-commits

Tags: #clang-tools-extra, #clang

Differential Revision: https://reviews.llvm.org/D69548
parent 06f3dabe

clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp

+34 −9
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ 


#include "RedundantStringInitCheck.h"

#include "../utils/Matchers.h"

#include "../utils/OptionsUtils.h"

#include "clang/ASTMatchers/ASTMatchers.h"



using namespace clang::ast_matchers;
@@ -17,18 +18,42 @@ namespace clang { 
namespace tidy {

namespace readability {



const char DefaultStringNames[] = "::std::basic_string";



RedundantStringInitCheck::RedundantStringInitCheck(StringRef Name,

                                                   ClangTidyContext *Context)

    : ClangTidyCheck(Name, Context),

      StringNames(utils::options::parseStringList(

          Options.get("StringNames", DefaultStringNames))) {}



void RedundantStringInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {

  Options.store(Opts, "StringNames", DefaultStringNames);

}



void RedundantStringInitCheck::registerMatchers(MatchFinder *Finder) {

  if (!getLangOpts().CPlusPlus)

    return;

  const auto hasStringTypeName = hasAnyName(

      SmallVector<StringRef, 3>(StringNames.begin(), StringNames.end()));



  // Version of StringNames with namespaces removed

  std::vector<std::string> stringNamesNoNamespace;

  for (const std::string &name : StringNames) {

    std::string::size_type colonPos = name.rfind(':');

    stringNamesNoNamespace.push_back(

        name.substr(colonPos == std::string::npos ? 0 : colonPos + 1));

  }

  const auto hasStringCtorName = hasAnyName(SmallVector<StringRef, 3>(

      stringNamesNoNamespace.begin(), stringNamesNoNamespace.end()));



  // 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.

  const auto StringConstructorExpr = expr(

      anyOf(cxxConstructExpr(argumentCountIs(1),

                             hasDeclaration(cxxMethodDecl(hasStringCtorName))),

            // If present, the second argument is the alloc object which must

            // not be present explicitly.

            cxxConstructExpr(argumentCountIs(2),

                       hasDeclaration(cxxMethodDecl(hasName("basic_string"))),

                             hasDeclaration(cxxMethodDecl(hasStringCtorName)),

                             hasArgument(1, cxxDefaultArgExpr()))));



  // Match a string constructor expression with an empty string literal.
@@ -48,7 +73,7 @@ void RedundantStringInitCheck::registerMatchers(MatchFinder *Finder) { 
      namedDecl(

          varDecl(

              hasType(hasUnqualifiedDesugaredType(recordType(

                  hasDeclaration(cxxRecordDecl(hasName("basic_string")))))),

                  hasDeclaration(cxxRecordDecl(hasStringTypeName))))),

              hasInitializer(expr(ignoringImplicit(anyOf(

                  EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)))))

              .bind("vardecl"),

clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.h

+7 −2
Original line number Diff line number Diff line
@@ -10,6 +10,8 @@ 
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_STRING_INIT_H



#include "../ClangTidyCheck.h"

#include <string>

#include <vector>



namespace clang {

namespace tidy {
@@ -18,10 +20,13 @@ namespace readability { 
/// Finds unnecessary string initializations.

class RedundantStringInitCheck : public ClangTidyCheck {

public:

  RedundantStringInitCheck(StringRef Name, ClangTidyContext *Context)

      : ClangTidyCheck(Name, Context) {}

  RedundantStringInitCheck(StringRef Name, ClangTidyContext *Context);

  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;

  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;



private:

  std::vector<std::string> StringNames;

};



} // namespace readability

clang-tools-extra/docs/ReleaseNotes.rst

+4 −0
Original line number Diff line number Diff line
@@ -170,6 +170,10 @@ Improvements to clang-tidy 
  The check now supports the ``AllowOverrideAndFinal`` option to eliminate

  conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.



- The :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.



Improvements to include-fixer

-----------------------------

clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst

+14 −1
Original line number Diff line number Diff line
@@ -5,7 +5,8 @@ readability-redundant-string-init 


Finds unnecessary string initializations.



Examples:

Examples

--------



.. code-block:: c++
@@ -17,3 +18,15 @@ Examples: 


  std::string a;

  std::string b;



Options

-------



.. option:: StringNames



    Default is `::std::basic_string`.



    Semicolon-delimited list of class names to apply this check to.

    By default `::std::basic_string` applies to ``std::string`` and

    ``std::wstring``. Set to e.g. `::std::basic_string;llvm::StringRef;QString`

    to perform this check on custom classes.

clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

+85 −1
Original line number Diff line number Diff line 
// RUN: %check_clang_tidy %s readability-redundant-string-init %t

// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t \

// RUN:   -config="{CheckOptions: \

// RUN:             [{key: readability-redundant-string-init.StringNames, \

// RUN:               value: '::std::basic_string;our::TestString'}] \

// RUN:             }"

// FIXME: Fix the checker to work in C++17 mode.



namespace std {

template <typename T>
@@ -143,3 +148,82 @@ extern void Param2(const std::string& param = ""); 
void Param3(std::string param = "") {}

void Param4(STRING param = "") {}



namespace our {

struct TestString {

  TestString();

  TestString(const TestString &);

  TestString(const char *);

  ~TestString();

};

}



void ourTestStringTests() {

  our::TestString a = "";

  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization

  // CHECK-FIXES: our::TestString a;

  our::TestString b("");

  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization

  // CHECK-FIXES: our::TestString b;

  our::TestString c = R"()";

  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization

  // CHECK-FIXES: our::TestString c;

  our::TestString d(R"()");

  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization

  // CHECK-FIXES: our::TestString d;



  our::TestString u = "u";

  our::TestString w("w");

  our::TestString x = R"(x)";

  our::TestString y(R"(y)");

  our::TestString z;

}



namespace their {

using TestString = our::TestString;

}



// their::TestString is the same type so should warn / fix

void theirTestStringTests() {

  their::TestString a = "";

  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization

  // CHECK-FIXES: their::TestString a;

  their::TestString b("");

  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization

  // CHECK-FIXES: their::TestString b;

}



namespace other {

// Identical declarations to above but different type

struct TestString {

  TestString();

  TestString(const TestString &);

  TestString(const char *);

  ~TestString();

};



// Identical declarations to above but different type

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 &);

  basic_string(const C *, const A &a = A());

  ~basic_string();

};

typedef basic_string<char> string;

typedef basic_string<wchar_t> wstring;

}



// other::TestString, other::string, other::wstring are unrelated to the types

// being checked. No warnings / fixes should be produced for these types.

void otherTestStringTests() {

  other::TestString a = "";

  other::TestString b("");

  other::string c = "";

  other::string d("");

  other::wstring e = L"";

  other::wstring f(L"");

}