diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h --- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h @@ -32,6 +32,7 @@ private: const bool WarnOnLargeLength; const unsigned int LargeLengthThreshold; + std::vector StringNames; }; } // namespace bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "StringConstructorCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Tooling/FixIt.h" @@ -21,17 +22,36 @@ AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) { return Node.getValue().getZExtValue() > N; } + +const char DefaultStringNames[] = + "::std::basic_string;::std::basic_string_view"; + +static std::vector +removeNamespaces(const std::vector &Names) { + std::vector Result; + Result.reserve(Names.size()); + for (StringRef Name : Names) { + std::string::size_type ColonPos = Name.rfind(':'); + Result.push_back( + Name.substr(ColonPos == std::string::npos ? 0 : ColonPos + 1)); + } + return Result; +} + } // namespace StringConstructorCheck::StringConstructorCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), WarnOnLargeLength(Options.get("WarnOnLargeLength", true)), - LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)) {} + LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)), + StringNames(utils::options::parseStringList( + Options.get("StringNames", DefaultStringNames))) {} void StringConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "WarnOnLargeLength", WarnOnLargeLength); Options.store(Opts, "LargeLengthThreshold", LargeLengthThreshold); + Options.store(Opts, "StringNames", DefaultStringNames); } void StringConstructorCheck::registerMatchers(MatchFinder *Finder) { @@ -80,7 +100,8 @@ // parameters. [i.e. string (const char* s, size_t n);] Finder->addMatcher( cxxConstructExpr( - hasDeclaration(cxxMethodDecl(hasName("basic_string"))), + hasDeclaration(cxxConstructorDecl(ofClass( + cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))), hasArgument(0, hasType(CharPtrType)), hasArgument(1, hasType(isInteger())), anyOf( @@ -100,11 +121,17 @@ // Check the literal string constructor with char pointer. // [i.e. string (const char* s);] Finder->addMatcher( - traverse(TK_AsIs, - cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))), - hasArgument(0, expr().bind("from-ptr")), - hasArgument(1, unless(hasType(isInteger())))) - .bind("constructor")), + traverse(TK_AsIs, + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl( + hasAnyName(removeNamespaces(StringNames)))))), + hasArgument(0, expr().bind("from-ptr")), + // do not match std::string(ptr, int) + // match std::string(ptr, alloc) + // match std::string(ptr) + anyOf(hasArgument(1, unless(hasType(isInteger()))), + argumentCountIs(1))) + .bind("constructor")), this); } diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst @@ -21,6 +21,7 @@ .. code-block:: c++ std::string("test", 200); // Will include random characters after "test". + std::string_view("test", 200); Creating an empty string from constructors with parameters is considered suspicious. The programmer should use the empty constructor instead. @@ -30,6 +31,7 @@ .. code-block:: c++ std::string("test", 0); // Creation of an empty string. + std::string_view("test", 0); Options ------- @@ -42,3 +44,12 @@ .. option:: LargeLengthThreshold An integer specifying the large length threshold. Default is `0x800000`. + +.. option:: StringNames + + Default is `::std::basic_string;::std::basic_string_view`. + + 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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp @@ -14,6 +14,15 @@ }; typedef basic_string string; typedef basic_string wstring; + +template > +struct basic_string_view { + basic_string_view(); + basic_string_view(const C *, unsigned int size); + basic_string_view(const C *); +}; +typedef basic_string_view string_view; +typedef basic_string_view wstring_view; } const char* kText = ""; @@ -52,11 +61,35 @@ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour } +void TestView() { + std::string_view q0("test", 0); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructor creating an empty string + std::string_view q1(kText, -4); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: negative value used as length parameter + std::string_view q2("test", 200); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size + std::string_view q3(kText, 200); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size + std::string_view q4(kText2, 200); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size + std::string_view q5(kText3, 0x1000000); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: suspicious large length parameter + std::string_view q6(nullptr); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour + std::string_view q7 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:25: warning: constructing string from nullptr is undefined behaviour +} + std::string StringFromZero() { return 0; // CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour } +std::string_view StringViewFromZero() { + return 0; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour +} + void Valid() { std::string empty(); std::string str(4, 'x'); @@ -64,6 +97,11 @@ std::string s1("test", 4); std::string s2("test", 3); std::string s3("test"); + + std::string_view emptyv(); + std::string_view sv1("test", 4); + std::string_view sv2("test", 3); + std::string_view sv3("test"); } namespace instantiation_dependent_exprs { @@ -71,5 +109,6 @@ struct S { bool x; std::string f() { return x ? "a" : "b"; } + std::string_view g() { return x ? "a" : "b"; } }; }