Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -25,6 +25,7 @@ RedundantSmartptrGetCheck.cpp RedundantStringInitCheck.cpp SimplifyBooleanExprCheck.cpp + SimplifySubscriptExprCheck.cpp StaticAccessedThroughInstanceCheck.cpp StaticDefinitionInAnonymousNamespaceCheck.cpp StringCompareCheck.cpp Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -32,6 +32,7 @@ #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" #include "SimplifyBooleanExprCheck.h" +#include "SimplifySubscriptExprCheck.h" #include "StaticAccessedThroughInstanceCheck.h" #include "StaticDefinitionInAnonymousNamespaceCheck.h" #include "StringCompareCheck.h" @@ -72,6 +73,8 @@ "readability-redundant-function-ptr-dereference"); CheckFactories.registerCheck( "readability-redundant-member-init"); + CheckFactories.registerCheck( + "readability-simplify-subscript-expr"); CheckFactories.registerCheck( "readability-static-accessed-through-instance"); CheckFactories.registerCheck( Index: clang-tidy/readability/SimplifySubscriptExprCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/SimplifySubscriptExprCheck.h @@ -0,0 +1,38 @@ +//===--- SimplifySubscriptExprCheck.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_READABILITY_SIMPLIFYSUBSCRIPTEXPRCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFYSUBSCRIPTEXPRCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Simplifies subscript expressions. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-simplify-subscript-expr.html +class SimplifySubscriptExprCheck : public ClangTidyCheck { +public: + SimplifySubscriptExprCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap& Opts) override; + +private: + const std::vector Types; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFYSUBSCRIPTEXPRCHECK_H Index: clang-tidy/readability/SimplifySubscriptExprCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/SimplifySubscriptExprCheck.cpp @@ -0,0 +1,76 @@ +//===--- SimplifySubscriptExprCheck.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 "SimplifySubscriptExprCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +static const char kDefaultTypes[] = + "::std::basic_string;::std::basic_string_view;::std::vector;::std::array"; + +SimplifySubscriptExprCheck::SimplifySubscriptExprCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), Types(utils::options::parseStringList( + Options.get("Types", kDefaultTypes))) { +} + +void SimplifySubscriptExprCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + const auto TypesMatcher = hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl(hasAnyName( + llvm::SmallVector(Types.begin(), Types.end())))))); + + Finder->addMatcher( + arraySubscriptExpr(hasBase(ignoringParenImpCasts( + cxxMemberCallExpr( + has(memberExpr().bind("member")), + on(hasType(qualType( + unless(anyOf(substTemplateTypeParmType(), + hasDescendant(substTemplateTypeParmType()))), + anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)))))), + callee(namedDecl(hasName("data")))) + .bind("call")))), + this); +} + +void SimplifySubscriptExprCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs("call"); + if (Result.Context->getSourceManager().isMacroBodyExpansion( + Call->getExprLoc())) + return; + + const auto *Member = Result.Nodes.getNodeAs("member"); + auto DiagBuilder = + diag(Member->getMemberLoc(), + "accessing an element of the container does not require a call to " + "'data()'; did you mean to use 'operator[]'?"); + if (Member->isArrow()) + DiagBuilder << FixItHint::CreateInsertion(Member->getLocStart(), "(*") + << FixItHint::CreateInsertion(Member->getOperatorLoc(), ")"); + DiagBuilder << FixItHint::CreateRemoval( + {Member->getOperatorLoc(), Call->getLocEnd()}); +} + +void SimplifySubscriptExprCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Types", utils::options::serializeStringList(Types)); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -146,6 +146,11 @@ Warns or suggests alternatives if SIMD intrinsics are used which can be replaced by ``std::experimental::simd`` operations. +- New :doc:`readability-simplify-subscript-expr + ` check. + + Simplifies subscript expressions like ``s.data()[i]`` into ``s[i]``. + - New :doc:`zircon-temporary-objects ` check. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -226,6 +226,7 @@ readability-redundant-string-cstr readability-redundant-string-init readability-simplify-boolean-expr + readability-simplify-subscript-expr readability-static-accessed-through-instance readability-static-definition-in-anonymous-namespace readability-string-compare Index: docs/clang-tidy/checks/readability-simplify-subscript-expr.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-simplify-subscript-expr.rst @@ -0,0 +1,23 @@ +.. title:: clang-tidy - readability-simplify-subscript-expr + +readability-simplify-subscript-expr +=================================== + +This check simplifies subscript expressions. Currently this covers calling +``.data()`` and immediately doing an array subscript operation to obtain a +single element, in which case simply calling ``operator[]`` suffice. + +Examples: + +.. code-block:: c++ + + std::string s = ...; + char c = s.data()[i]; // char c = s[i]; + +Options +------- + +.. option:: Types + + The list of type(s) that triggers this check. Default is + `::std::basic_string;::std::basic_string_view;::std::vector;::std::array` Index: test/clang-tidy/readability-simplify-subscript-expr.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-simplify-subscript-expr.cpp @@ -0,0 +1,108 @@ +// RUN: %check_clang_tidy %s readability-simplify-subscript-expr %t \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: readability-simplify-subscript-expr.Types, \ +// RUN: value: '::std::basic_string;::std::basic_string_view;MyVector'}]}" -- + +namespace std { + +template +class basic_string { + public: + using size_type = unsigned; + using value_type = T; + using reference = value_type&; + using const_reference = const value_type&; + + reference operator[](size_type); + const_reference operator[](size_type) const; + T* data(); + const T* data() const; +}; + +using string = basic_string; + +template +class basic_string_view { + public: + using size_type = unsigned; + using const_reference = const T&; + using const_pointer = const T*; + + constexpr const_reference operator[](size_type) const; + constexpr const_pointer data() const noexcept; +}; + +using string_view = basic_string_view; + +} + +template +class MyVector { + public: + using size_type = unsigned; + using const_reference = const T&; + using const_pointer = const T*; + + const_reference operator[](size_type) const; + const T* data() const noexcept; +}; + +#define DO(x) do { x; } while (false) +#define ACCESS(x) (x) +#define GET(x, i) (x).data()[i] + +template +class Foo { + public: + char bar(int i) { + return x.data()[i]; + } + private: + T x; +}; + +void f(int i) { + MyVector v; + int x = v.data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: accessing an element of the container does not require a call to 'data()'; did you mean to use 'operator[]'? [readability-simplify-subscript-expr] + // CHECK-FIXES: int x = v[i]; + + std::string s; + char c1 = s.data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: accessing an element + // CHECK-FIXES: char c1 = s[i]; + + std::string_view sv; + char c2 = sv.data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: accessing an element + // CHECK-FIXES: char c2 = sv[i]; + + std::string* ps = &s; + char c3 = ps->data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: accessing an element + // CHECK-FIXES: char c3 = (*ps)[i]; + + char c4 = (*ps).data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: accessing an element + // CHECK-FIXES: char c4 = (*ps)[i]; + + DO(char c5 = s.data()[i]); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: accessing an element + // CHECK-FIXES: DO(char c5 = s[i]); + + char c6 = ACCESS(s).data()[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: accessing an element + // CHECK-FIXES: char c6 = ACCESS(s)[i]; + + char c7 = ACCESS(s.data())[i]; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: accessing an element + // CHECK-FIXES: char c7 = ACCESS(s)[i]; + + char c8 = ACCESS(s.data()[i]); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: accessing an element + // CHECK-FIXES: char c8 = ACCESS(s[i]); + + char c9 = GET(s, i); + + char c10 = Foo{}.bar(i); +}