hugoeg updated this revision to Diff 162253.
hugoeg added a comment.
minor fixes, style improvements
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-str-cat-append.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-str-cat-append.cpp
Index: test/clang-tidy/abseil-str-cat-append.cpp
===================================================================
--- test/clang-tidy/abseil-str-cat-append.cpp
+++ test/clang-tidy/abseil-str-cat-append.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-str-cat-append %t -- -- -I%S -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
+
+namespace std {
+template <typename T>
+class allocator {};
+template <typename T>
+class char_traits {};
+template <typename C, typename T, typename A>
+struct basic_string {
+ typedef basic_string<C, T, A> _Type;
+ basic_string();
+ basic_string(const C* p, const A& a = A());
+
+ const C* c_str() const;
+ const C* data() const;
+
+ _Type& append(const C* s);
+ _Type& append(const C* s, size n);
+ _Type& assign(const C* s);
+ _Type& assign(const C* s, size n);
+
+ int compare(const _Type&) const;
+ int compare(const C* s) const;
+ int compare(size pos, size len, const _Type&) const;
+ int compare(size pos, size len, const C* s) const;
+
+ size find(const _Type& str, size pos = 0) const;
+ size find(const C* s, size pos = 0) const;
+ size find(const C* s, size pos, size n) const;
+
+ _Type& insert(size pos, const _Type& str);
+ _Type& insert(size pos, const C* s);
+ _Type& insert(size pos, const C* s, size n);
+
+ _Type& operator+=(const _Type& str);
+ _Type& operator+=(const C* s);
+ _Type& operator=(const _Type& str);
+ _Type& operator=(const C* s);
+};
+
+typedef basic_string<char, std::char_traits<char>, std::allocator<char>> string;
+typedef basic_string<wchar_t, std::char_traits<wchar_t>,
+ std::allocator<wchar_t>>
+ wstring;
+typedef basic_string<char16, std::char_traits<char16>, std::allocator<char16>>
+ u16string;
+typedef basic_string<char32, std::char_traits<char32>, std::allocator<char32>>
+ u32string;
+} // namespace std
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
+namespace llvm {
+struct StringRef {
+ StringRef(const char* p);
+ StringRef(const std::string&);
+};
+} // namespace llvm
+
+namespace absl {
+
+struct AlphaNum {
+ AlphaNum(int i);
+ AlphaNum(double f);
+ AlphaNum(const char* c_str);
+ AlphaNum(const std::string& str);
+
+ private:
+ AlphaNum(const AlphaNum&);
+ AlphaNum& operator=(const AlphaNum&);
+};
+
+std::string StrCat(const AlphaNum& A);
+std::string StrCat(const AlphaNum& A, const AlphaNum& B);
+
+template <typename A>
+void Foo(A& a) {
+ a = StrCat(a);
+}
+
+void Bar() {
+ std::string A, B;
+ Foo<std::string>(A);
+
+ std::string C = StrCat(A);
+ A = StrCat(A);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: call to absl::StrCat does nothing
+ A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}} StrAppend(&A, B);
+ B = StrCat(A, B);
+
+#define M(X) X = StrCat(X, A)
+ M(B);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: #define M(X) X = StrCat(X, A)
+}
+
+void Regression_SelfAppend() {
+ std::string A;
+ A = StrCat(A, A);
+}
+
+} // namespace absl
+
+void OutsideAbsl() {
+ std::string A, B;
+ A = absl::StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}} StrAppend(&A, B);
+}
+
+void OutisdeUsingAbsl() {
+ std::string A, B;
+ using absl::StrCat;
+ A = StrCat(A, B);
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: please use absl::StrAppend instead of absl::StrCat when appending to an existing string
+// CHECK-FIXES: {{^}} StrAppend(&A, B);
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-duration-division
+ abseil-str-cat-append
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-str-cat-append.rst
===================================================================
--- docs/clang-tidy/checks/abseil-str-cat-append.rst
+++ docs/clang-tidy/checks/abseil-str-cat-append.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - abseil-str-cat-append
+
+abseil-str-cat-append
+=====================
+
+Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
+``absl::StrAppend()`` should be used instead.
+
+.. code-block:: c++
+
+ a = absl::StrCat(a, b); // Use absl::StrAppend(&a, b) instead.
+
+Does not diagnose cases where ``abls::StrCat()`` is used as a template
+argument for a functor.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,6 +64,12 @@
floating-point context, and recommends the use of a function that
returns a floating-point value.
+- New :doc:`abseil-str-cat-append
+ <clang-tidy/checks/abseil-str-cat-append>` check.
+
+ Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests
+ ``absl::StrAppend()`` should be used instead.
+
- New :doc:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.
Index: clang-tidy/abseil/StrCatAppendCheck.h
===================================================================
--- clang-tidy/abseil/StrCatAppendCheck.h
+++ clang-tidy/abseil/StrCatAppendCheck.h
@@ -0,0 +1,36 @@
+//===--- StrCatAppendCheck.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_ABSEIL_STRCATAPPENDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRCATAPPENDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Flags uses of absl::StrCat to append to a string. Suggests absl::StrAppend
+/// should be used instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-str-cat-append.html
+class StrCatAppendCheck : public ClangTidyCheck {
+public:
+ StrCatAppendCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRCATAPPENDCHECK_H
Index: clang-tidy/abseil/StrCatAppendCheck.cpp
===================================================================
--- clang-tidy/abseil/StrCatAppendCheck.cpp
+++ clang-tidy/abseil/StrCatAppendCheck.cpp
@@ -0,0 +1,102 @@
+//===--- StrCatAppendCheck.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 "StrCatAppendCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+namespace {
+
+// Skips any combination of temporary materialization, temporary binding and
+// implicit casting.
+AST_MATCHER_P(Stmt, ignoringTemporaries, ast_matchers::internal::Matcher<Stmt>,
+ InnerMatcher) {
+ const Stmt *E = &Node;
+ while (true) {
+ if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
+ E = MTE->getTemporary();
+ if (const auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+ E = BTE->getSubExpr();
+ if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
+ else
+ break;
+ }
+
+ return InnerMatcher.matches(*E, Finder, Builder);
+}
+
+} // namespace
+
+// TODO: str += StrCat(...)
+// str.append(StrCat(...))
+
+void StrCatAppendCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+ // Look for calls to StrCat.
+ const auto StrCat = functionDecl(hasName("::absl::StrCat"));
+ // The arguments of StrCat are implicitly converted to AlphaNum. Match that.
+ const auto AlphaNum = ignoringTemporaries(cxxConstructExpr(
+ argumentCountIs(1), hasType(cxxRecordDecl(hasName("::absl::AlphaNum"))),
+ hasArgument(0, ignoringImpCasts(declRefExpr(to(equalsBoundNode("LHS")),
+ expr().bind("Arg0"))))));
+
+ const auto has_another_reference_to_lhs =
+ callExpr(hasAnyArgument(expr(hasDescendant(declRefExpr(
+ to(equalsBoundNode("LHS")), unless(equalsBoundNode("Arg0")))))));
+
+ // Now look for calls to operator= with an object on the LHS and a call to
+ // StrCat on the RHS. The first argument of the StrCat call should be the same
+ // as the LHS. Ignore calls from template instantiations.
+ Finder->addMatcher(
+ cxxOperatorCallExpr(
+ unless(isInTemplateInstantiation()), hasOverloadedOperatorName("="),
+ hasArgument(0, declRefExpr(to(decl().bind("LHS")))),
+ hasArgument(1, ignoringTemporaries(
+ callExpr(callee(StrCat), hasArgument(0, AlphaNum),
+ unless(has_another_reference_to_lhs))
+ .bind("Call"))))
+ .bind("Op"),
+ this);
+}
+
+void StrCatAppendCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Op = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("Op");
+ const auto *Call = Result.Nodes.getNodeAs<CallExpr>("Call");
+
+ // Handles the case where x = absl::StrCat(x), which does nothing.
+ if (Call->getNumArgs() == 1) {
+ diag(Op->getBeginLoc(), "call to absl::StrCat does nothing");
+ return;
+ }
+
+ // Emit a warning and emit fixits to go from
+ // x = absl::StrCat(x, ...)
+ // to
+ // absl::StrAppend(&x, ...)
+ diag(Op->getBeginLoc(),
+ "please use absl::StrAppend instead of absl::StrCat when appending to "
+ "an existing string")
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(Op->getBeginLoc(),
+ Call->getCallee()->getEndLoc()),
+ "StrAppend")
+ << FixItHint::CreateInsertion(Call->getArg(0)->getBeginLoc(), "&");
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -3,6 +3,7 @@
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
DurationDivisionCheck.cpp
+ StrCatAppendCheck.cpp
StringFindStartswithCheck.cpp
LINK_LIBS
Index: clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tidy/abseil/AbseilTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "DurationDivisionCheck.h"
+#include "StrCatAppendCheck.h"
#include "StringFindStartswithCheck.h"
namespace clang {
@@ -22,6 +23,8 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<DurationDivisionCheck>(
"abseil-duration-division");
+ CheckFactories.registerCheck<StrCatAppendCheck>(
+ "abseil-str-cat-append");
CheckFactories.registerCheck<StringFindStartswithCheck>(
"abseil-string-find-startswith");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
