diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -44,6 +44,7 @@ StaticAccessedThroughInstanceCheck.cpp StaticDefinitionInAnonymousNamespaceCheck.cpp StringCompareCheck.cpp + StrlenStringCStrCheck.cpp SuspiciousCallArgumentCheck.cpp UniqueptrDeleteReleaseCheck.cpp UppercaseLiteralSuffixCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -47,6 +47,7 @@ #include "StaticAccessedThroughInstanceCheck.h" #include "StaticDefinitionInAnonymousNamespaceCheck.h" #include "StringCompareCheck.h" +#include "StrlenStringCStrCheck.h" #include "SuspiciousCallArgumentCheck.h" #include "UniqueptrDeleteReleaseCheck.h" #include "UppercaseLiteralSuffixCheck.h" @@ -135,6 +136,8 @@ "readability-redundant-string-init"); CheckFactories.registerCheck( "readability-simplify-boolean-expr"); + CheckFactories.registerCheck( + "readability-strlen-string-cstr"); CheckFactories.registerCheck( "readability-suspicious-call-argument"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h b/clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h @@ -0,0 +1,44 @@ +//===--- StrlenStringCStrCheck.h - clang-tidy ----------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLENSTRINGCSTRCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLENSTRINGCSTRCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Finds calls to `strlen(std::string::c_str())`. +class StrlenStringCStrCheck : public ClangTidyCheck { +public: + StrlenStringCStrCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + EnableForDataMethod(Options.get("EnableForDataMethod", false)) {} + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override { + Options.store(Opts, "EnableForDataMethod", EnableForDataMethod); + } + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool EnableForDataMethod; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLENSTRINGCSTRCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.cpp @@ -0,0 +1,65 @@ +//===- StrlenStringCStrCheck.cpp - Check for strlen(string::c_str()) calls +//-----===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file implements a check for calls to `strlen(std::string::c_str())`. +// +//===----------------------------------------------------------------------===// + +#include "StrlenStringCStrCheck.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void StrlenStringCStrCheck::registerMatchers( + ast_matchers::MatchFinder *Finder) { + const auto MethodNameMatcher = + EnableForDataMethod ? hasAnyName("c_str", "data") : hasAnyName("c_str"); + + const auto StrlenFunctions = functionDecl( + hasAnyName("::strlen", "::std::strlen", "::strnlen", "::strnlen_s", + "::wcslen", "::std::wcslen", "::wcsnlen_s")); + const auto CStrCall = cxxMemberCallExpr(callee( + cxxMethodDecl(MethodNameMatcher, + ofClass(cxxRecordDecl(hasName("::std::basic_string")))))); + const auto StrlenCall = + callExpr(callee(StrlenFunctions), hasArgument(0, CStrCall.bind("arg"))) + .bind("strlen"); + Finder->addMatcher(StrlenCall, this); +} + +void StrlenStringCStrCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto *StrlenCall = Result.Nodes.getNodeAs("strlen"); + const auto *Arg = Result.Nodes.getNodeAs("arg"); + + // Remove the `strlen(` (or similar) prefix. + CharSourceRange StrlenRange = CharSourceRange::getCharRange( + StrlenCall->getBeginLoc(), Arg->getBeginLoc()); + // Remove the `)` (and additional strnlen arguments, if any) from the strlen + // call. + CharSourceRange CloseParenRange = + CharSourceRange::getCharRange(Arg->getEndLoc(), StrlenCall->getEndLoc()); + + const auto *CStrCallee = cast(Arg->getCallee()); + diag(StrlenCall->getBeginLoc(), "redundant call to %0 and %1") + << StrlenCall->getDirectCallee() << Arg->getMethodDecl() + << FixItHint::CreateRemoval(StrlenRange) + << FixItHint::CreateRemoval(CloseParenRange) + << FixItHint::CreateReplacement(CStrCallee->getMemberLoc(), "size"); +} + +} // namespace readability +} // namespace tidy +} // namespace clang \ No newline at end of file diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -129,6 +129,12 @@ Warns when `empty()` is used on a range and the result is ignored. Suggests `clear()` if it is an existing member function. +- New :doc:`readability-strlen-string-cstr + ` check. + +Warns when the return value of `std::basic_string::c_str` or `std::basic_string::data` +is used as the argument for `strlen`, and suggests a fix. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -362,6 +362,7 @@ `readability-static-accessed-through-instance `_, "Yes" `readability-static-definition-in-anonymous-namespace `_, "Yes" `readability-string-compare `_, "Yes" + `readability-strlen-string-cstr `_, "Yes" `readability-suspicious-call-argument `_, `readability-uniqueptr-delete-release `_, "Yes" `readability-uppercase-literal-suffix `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/strlen-string-cstr.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/strlen-string-cstr.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/strlen-string-cstr.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - readability-strlen-string-cstr + +readability-strlen-string-cstr +============================== + +Finds calls to ``strlen`` and similar functions where the result +of ``std::basic_string::c_str`` or ``std::basic_string::data`` is used +as an argument. + +Example +------- + +.. code-block:: c++ + + std::string str1{"foo"} + + // Use str1.size() + size_t length = strlen(str1.c_str()); + + +Options +------- + +.. option:: EnableForDataMethod + + Default is `true`. + + When `false`, the check will not warn then the result of + ``std::basic_string::data`` is used as an argument for ``strlen``. \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp @@ -0,0 +1,60 @@ +// RUN: %check_clang_tidy -check-suffix=0 %s readability-strlen-string-cstr %t -- -config="{CheckOptions: [{key: readability-strlen-string-cstr.EnableForDataMethod, value: false}]}" +// RUN: %check_clang_tidy -check-suffix=1 %s readability-strlen-string-cstr %t -- -config="{CheckOptions: [{key: readability-strlen-string-cstr.EnableForDataMethod, value: true}]}" +int strlen(const char *); +int strnlen(const char *, int); +int strnlen_s(const char *, int); + +int wcslen(const wchar_t *); +int wcsnlen_s(const wchar_t *, int); + +namespace std { +template class allocator {}; +template class char_traits {}; + +template , + typename A = std::allocator> +class basic_string { +public: + basic_string(); + basic_string(const C *, unsigned int size); + const C *c_str() const; + const C *data() const; + int size() const; +}; + +using wstring = basic_string; +using string = basic_string; + +int strlen(const char *); +int wcslen(const wchar_t *); +} // namespace std + +void f1() { + std::string str1("a", 1); + int length = strlen(str1.c_str()); + // CHECK-MESSAGES-0: [[@LINE-1]]:16: warning: redundant call to 'strlen' {{.*}} + // CHECK-FIXES-0: {{^ }}int length = str1.size();{{$}} + // CHECK-MESSAGES-1: [[@LINE-3]]:16: warning: redundant call to 'strlen' {{.*}} + // CHECK-FIXES-1: {{^ }}int length = str1.size();{{$}} + + length = strnlen(str1.data(), 30); + // CHECK-MESSAGES-1: [[@LINE-1]]:12: warning: redundant call to 'strnlen' {{.*}} + // CHECK-FIXES-1: {{^ }}length = str1.size();{{$}} + + const std::string *p1 = &str1; + length = std::strlen(p1->c_str()) + 30; + // CHECK-MESSAGES-0: [[@LINE-1]]:12: warning: redundant call {{.*}} + // CHECK-FIXES-0: {{^ }}length = p1->size() + 30;{{$}} + // CHECK-MESSAGES-1: [[@LINE-3]]:12: warning: redundant call {{.*}} + // CHECK-FIXES-1: {{^ }}length = p1->size() + 30;{{$}} +} + +void f2() { + std::wstring wstr1; + + int length = wcslen(wstr1.c_str()); + // CHECK-MESSAGES-0: [[@LINE-1]]:16: warning: redundant call to 'wcslen' {{.*}} + // CHECK-FIXES-0: {{^ }}int length = wstr1.size();{{$}} + // CHECK-MESSAGES-1: [[@LINE-3]]:16: warning: redundant call to 'wcslen' {{.*}} + // CHECK-FIXES-1: {{^ }}int length = wstr1.size();{{$}} +} \ No newline at end of file