etienneb created this revision.
etienneb added a reviewer: alexfh.
etienneb added a subscriber: cfe-commits.
Checker to validate string constructor parameters.
A common mistake is to swap parameter for the fill-constructor.
```
std::string str('x', 4);
std::string str('4', x);
```
http://reviews.llvm.org/D19146
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/StringConstructorCheck.cpp
clang-tidy/misc/StringConstructorCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-string-constructor.rst
test/clang-tidy/misc-string-constructor.cpp
Index: test/clang-tidy/misc-string-constructor.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-string-constructor.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s misc-string-constructor %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 C*, unsigned int size);
+ basic_string(unsigned int size, C c);
+};
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+}
+
+const char* kText = "";
+const char kText2[] = "";
+extern const char kText3[];
+
+void Test() {
+ std::string str('x', 4);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor parameters are probably swapped [misc-string-constructor]
+ std::wstring wstr(L'x', 4);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: constructor parameters are probably swapped
+ std::string s0(0, 'x');
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
+ std::string s1(-4, 'x');
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as length parameter
+
+ std::string q0("test", 0);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
+ std::string q1(kText, -4);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: negative value used as length parameter
+ std::string q2("test", 200);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
+ std::string q3(kText, 200);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
+ std::string q4(kText2, 200);
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: length is bigger then string literal size
+}
+
+void Valid() {
+ std::string empty();
+ std::string str(4, 'x');
+ std::wstring wstr(4, L'x');
+ std::string s1("test", 4);
+ std::string s2("test", 3);
+}
Index: docs/clang-tidy/checks/misc-string-constructor.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-string-constructor.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - misc-string-constructor
+
+misc-string-constructor
+=======================
+
+Finds string constructors that are suspicious and probably errors.
+
+
+A common mistake is to swap parameters to the 'fill' string-constructor.
+
+Examples:
+
+.. code:: c++
+
+ std::string('x', 50) str // should be std::string(50, 'x')
+
+
+Calling the string-literal constructor with a length bigger than the literal is
+suspicious and adds extra random characters to the string.
+
+Examples:
+
+.. code:: c++
+
+ std::string("test", 200); // Will include random characters after "test".
+
+
+Creating an empty string from constructors with parameters is considered
+suspicious. The programmer should use the empty constructor instead.
+
+Examples:
+
+.. code:: c++
+
+ std::string("test", 0); // Creation of an empty string.
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -67,6 +67,7 @@
misc-non-copyable-objects
misc-sizeof-container
misc-static-assert
+ misc-string-constructor
misc-string-integer-assignment
misc-string-literal-with-embedded-nul
misc-suspicious-missing-comma
Index: clang-tidy/misc/StringConstructorCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/StringConstructorCheck.h
@@ -0,0 +1,39 @@
+//===--- StringConstructorCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_CONSTRUCTOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_CONSTRUCTOR_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds suspicious string constructor and check their parameters.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-constructor.html
+class StringConstructorCheck : public ClangTidyCheck {
+public:
+ StringConstructorCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool WarnOnLargeLength;
+ const unsigned int LargeLengthThreshold;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STRING_CONSTRUCTOR_H
Index: clang-tidy/misc/StringConstructorCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/StringConstructorCheck.cpp
@@ -0,0 +1,153 @@
+//===--- StringConstructorCheck.cpp - clang-tidy---------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringConstructorCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
+ return Node.getValue().getZExtValue() > N;
+}
+
+StringConstructorCheck::StringConstructorCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ WarnOnLargeLength(Options.get("WarnOnLargeLength", 1) != 0),
+ LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)) {}
+
+void StringConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "WarnOnLargeLength", WarnOnLargeLength);
+ Options.store(Opts, "LargeLengthThreshold", LargeLengthThreshold);
+}
+
+void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ const auto ZeroExpr = ignoringParenImpCasts(integerLiteral(equals(0)));
+ const auto NegativeExpr = ignoringParenImpCasts(
+ unaryOperator(hasOperatorName("-"),
+ hasUnaryOperand(integerLiteral(unless(equals(0))))));
+ const auto LargeLengthExpr =
+ ignoringParenImpCasts(integerLiteral(isBiggerThan(LargeLengthThreshold)));
+ const auto CharPtrType = type(anyOf(pointerType(), arrayType()));
+
+ // Match a string-literal; even through a declaration with initializer.
+ const auto BoundStringLiteral = stringLiteral().bind("str");
+ const auto ConstStrLiteralDecl = varDecl(
+ isDefinition(), hasType(constantArrayType()), hasType(isConstQualified()),
+ hasInitializer(ignoringParenImpCasts(BoundStringLiteral)));
+ const auto ConstPtrStrLiteralDecl = varDecl(
+ isDefinition(),
+ hasType(pointerType(pointee(isAnyCharacter(), isConstQualified()))),
+ hasInitializer(ignoringParenImpCasts(BoundStringLiteral)));
+ auto ConstStrLiteral =
+ expr(anyOf(BoundStringLiteral,
+ declRefExpr(hasDeclaration(
+ anyOf(ConstPtrStrLiteralDecl, ConstStrLiteralDecl)))));
+
+ // Check the fill constructor. Fills the string with n consecutive copies of
+ // character c. [i.e string(size_t n, char c);].
+ const auto FillStringConstructor =
+ cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+ hasArgument(0, hasType(qualType(isInteger()))),
+ hasArgument(1, hasType(qualType(isInteger()))));
+
+ // Detect the expression: string('x', 40);
+ Finder->addMatcher(cxxConstructExpr(FillStringConstructor,
+ hasArgument(0, ignoringParenImpCasts(
+ characterLiteral())))
+ .bind("swapped-parameter"),
+ this);
+
+ // Detect the expression: string(0, ...);
+ Finder->addMatcher(
+ cxxConstructExpr(FillStringConstructor, hasArgument(0, ZeroExpr))
+ .bind("empty-string"),
+ this);
+
+ // Detect the expression: string(-4, ...);
+ Finder->addMatcher(
+ cxxConstructExpr(FillStringConstructor, hasArgument(0, NegativeExpr))
+ .bind("negative-length"),
+ this);
+
+ // Detect the expression: string(0x1234567, ...);
+ if (WarnOnLargeLength) {
+ Finder->addMatcher(
+ cxxConstructExpr(FillStringConstructor, hasArgument(0, LargeLengthExpr))
+ .bind("large-length"),
+ this);
+ }
+
+ // Check the literal string constructor with char pointer and length
+ // parameters. [i.e. string (const char* s, size_t n);]
+ const auto LiteralStringConstructor =
+ cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+ hasArgument(0, hasType(CharPtrType)),
+ hasArgument(1, hasType(isInteger())));
+
+ // Detect the expression: string("...", 0);
+ Finder->addMatcher(
+ cxxConstructExpr(LiteralStringConstructor, hasArgument(1, ZeroExpr))
+ .bind("empty-string"),
+ this);
+
+ // Detect the expression: string("...", -4);
+ Finder->addMatcher(
+ cxxConstructExpr(LiteralStringConstructor, hasArgument(1, NegativeExpr))
+ .bind("negative-length"),
+ this);
+
+ // Detect the expression: string("lit", 5)
+ Finder->addMatcher(
+ cxxConstructExpr(
+ LiteralStringConstructor,
+ hasArgument(0, ignoringParenImpCasts(ConstStrLiteral)),
+ hasArgument(1, ignoringParenImpCasts(integerLiteral().bind("int"))))
+ .bind("check-literal-with-length"),
+ this);
+
+ // Detect the expression: string("lit", 0x1234567);
+ if (WarnOnLargeLength) {
+ Finder->addMatcher(cxxConstructExpr(LiteralStringConstructor,
+ hasArgument(1, LargeLengthExpr))
+ .bind("large-length"),
+ this);
+ }
+}
+
+void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *E = Result.Nodes.getNodeAs<Expr>("swapped-parameter")) {
+ diag(E->getLocStart(), "constructor parameters are probably swapped");
+ } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("empty-string")) {
+ diag(E->getLocStart(), "constructor creating an empty string");
+ } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("negative-length")) {
+ diag(E->getLocStart(), "negative value used as length parameter");
+ } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("large-length")) {
+ diag(E->getLocStart(), "suspicious large length parameter");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("check-literal-with-length")) {
+ const auto *S = Result.Nodes.getNodeAs<StringLiteral>("str");
+ const auto *L = Result.Nodes.getNodeAs<IntegerLiteral>("int");
+ if (L->getValue().ugt(S->getLength())) {
+ diag(E->getLocStart(), "length is bigger then string literal size");
+ }
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -31,6 +31,7 @@
#include "NonCopyableObjects.h"
#include "SizeofContainerCheck.h"
#include "StaticAssertCheck.h"
+#include "StringConstructorCheck.h"
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
#include "SuspiciousMissingCommaCheck.h"
@@ -91,6 +92,8 @@
CheckFactories.registerCheck<SizeofContainerCheck>("misc-sizeof-container");
CheckFactories.registerCheck<StaticAssertCheck>(
"misc-static-assert");
+ CheckFactories.registerCheck<StringConstructorCheck>(
+ "misc-string-constructor");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
"misc-string-integer-assignment");
CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -23,6 +23,7 @@
NonCopyableObjects.cpp
SizeofContainerCheck.cpp
StaticAssertCheck.cpp
+ StringConstructorCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
SuspiciousMissingCommaCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
