Create a new check to look for mis-use in calls that take iterators #99917
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Member
| @llvm/pr-subscribers-clang-tools-extra Author: Nathan James (njames93) ChangesLooks for various patterns of functions that take arguments calling begin/end or similar for common mistakes. Still needs a bit of work, but open to suggestions of how this check could be improved Patch is 35.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99917.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 689eb92a3d8d1..cea040b81878a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -33,6 +33,7 @@ #include "InaccurateEraseCheck.h" #include "IncDecInConditionsCheck.h" #include "IncorrectEnableIfCheck.h" +#include "IncorrectIteratorsCheck.h" #include "IncorrectRoundingsCheck.h" #include "InfiniteLoopCheck.h" #include "IntegerDivisionCheck.h" @@ -139,6 +140,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); + CheckFactories.registerCheck<IncorrectIteratorsCheck>( + "bugprone-incorrect-iterators"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index cb0d8ae98bac5..8425dbed0505a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule ImplicitWideningOfMultiplicationResultCheck.cpp InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp + IncorrectIteratorsCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp new file mode 100644 index 0000000000000..56429b9100146 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp @@ -0,0 +1,362 @@ +//===--- IncorrectIteratorsCheck.cpp - clang-tidy -------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#include "IncorrectIteratorsCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/iterator_range.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { +using SVU = llvm::SmallVector<unsigned, 3>; +/// Checks to see if a function a +AST_MATCHER_P(FunctionDecl, areParametersSameTemplateType, SVU, Indexes) { + const auto *TemplateDecl = Node.getPrimaryTemplate(); + if (!TemplateDecl) + return false; + const auto *FuncDecl = TemplateDecl->getTemplatedDecl(); + if (!FuncDecl) + return false; + assert(!Indexes.empty()); + const auto *FirstParam = FuncDecl->getParamDecl(Indexes.front()); + if (!FirstParam) + return false; + auto Type = FirstParam->getOriginalType(); + for (auto Item : llvm::drop_begin(Indexes)) { + const auto *Param = FuncDecl->getParamDecl(Item); + if (!Param) + return false; + if (Param->getOriginalType() != Type) + return false; + } + return true; +} +struct NameMatchers { + ArrayRef<StringRef> FreeNames; + ArrayRef<StringRef> MethodNames; +}; + +struct NamePairs { + NameMatchers BeginNames; + NameMatchers EndNames; +}; + +struct FullState { + NamePairs Forward; + NamePairs Reversed; + NamePairs Combined; + NameMatchers All; + ArrayRef<StringRef> MakeReverseIterator; +}; + +} // namespace + +static constexpr char FirstRangeArg[] = "FirstRangeArg"; +static constexpr char FirstRangeArgExpr[] = "FirstRangeArgExpr"; +static constexpr char ReverseBeginBind[] = "ReverseBeginBind"; +static constexpr char ReverseEndBind[] = "ReverseEndBind"; +static constexpr char SecondRangeArg[] = "SecondRangeArg"; +static constexpr char SecondRangeArgExpr[] = "SecondRangeArgExpr"; +static constexpr char ArgMismatchBegin[] = "ArgMismatchBegin"; +static constexpr char ArgMismatchEnd[] = "ArgMismatchEnd"; +static constexpr char ArgMismatchExpr[] = "ArgMismatchExpr"; +static constexpr char ArgMismatchRevBind[] = "ArgMismatchRevBind"; +static constexpr char Callee[] = "Callee"; +static constexpr char Internal[] = "Internal"; +static constexpr char InternalOther[] = "InternalOther"; +static constexpr char InternalArgument[] = "InternalArgument"; + +static auto +makeExprMatcher(ast_matchers::internal::Matcher<Expr> ArgumentMatcher, + const NameMatchers &Names, ArrayRef<StringRef> MakeReverse, + const NameMatchers &RevNames, StringRef RevBind) { + return expr(anyOf( + cxxMemberCallExpr(argumentCountIs(0), + callee(cxxMethodDecl(hasAnyName(Names.MethodNames))), + on(ArgumentMatcher)), + callExpr(argumentCountIs(1), + hasDeclaration(functionDecl(hasAnyName(Names.FreeNames))), + hasArgument(0, ArgumentMatcher)), + callExpr( + callee(functionDecl(hasAnyName(MakeReverse))), argumentCountIs(1), + hasArgument( + 0, expr(anyOf(cxxMemberCallExpr(argumentCountIs(0), + callee(cxxMethodDecl(hasAnyName( + RevNames.MethodNames))), + on(ArgumentMatcher)), + callExpr(argumentCountIs(1), + hasDeclaration(functionDecl( + hasAnyName(RevNames.FreeNames))), + hasArgument(0, ArgumentMatcher)))))) + .bind(RevBind))); +} + +// Detects a range function where the argument for the begin call differs from +// the end call std::find(I.begin(), J.end()) +static auto makeRangeArgMismatch(unsigned BeginExpected, unsigned EndExpected, + NamePairs Forward, NamePairs Reverse, + ArrayRef<StringRef> MakeReverse) { + std::vector<ast_matchers::internal::DynTypedMatcher> Matchers; + + for (const auto &[Primary, Backwards] : + {std::pair{Forward, Reverse}, std::pair{Reverse, Forward}}) { + Matchers.push_back(callExpr( + hasArgument(BeginExpected, + makeExprMatcher(expr().bind(FirstRangeArg), + Primary.BeginNames, MakeReverse, + Backwards.EndNames, ReverseBeginBind) + .bind(FirstRangeArgExpr)), + hasArgument(EndExpected, + makeExprMatcher( + expr(unless(matchers::isStatementIdenticalToBoundNode( + FirstRangeArg))) + .bind(SecondRangeArg), + Primary.EndNames, MakeReverse, Backwards.BeginNames, + ReverseEndBind) + .bind(SecondRangeArgExpr)))); + } + + return callExpr( + argumentCountAtLeast(std::max(BeginExpected, EndExpected) + 1), + ast_matchers::internal::DynTypedMatcher::constructVariadic( + ast_matchers::internal::DynTypedMatcher::VO_AnyOf, + ASTNodeKind::getFromNodeKind<CallExpr>(), std::move(Matchers)) + .convertTo<CallExpr>()); +} + +// Detects a range function where we expect a call to begin but get a call to +// end or vice versa std::find(X.end(), X.end()) // Here we would warn on the +// first argument that it should be begin +static auto makeUnexpectedBeginEndMatcher(unsigned Index, StringRef BindTo, + NameMatchers Names, + ArrayRef<StringRef> MakeReverse, + const NameMatchers &RevNames) { + return callExpr(hasArgument(Index, makeExprMatcher(expr().bind(BindTo), Names, + MakeReverse, RevNames, + ArgMismatchRevBind) + .bind(ArgMismatchExpr))); +} + +static auto makeUnexpectedBeginEndPair(unsigned BeginExpected, + unsigned EndExpected, + NamePairs BeginEndPairs, + ArrayRef<StringRef> MakeReverse) { + // return expr(unless(anything())); + return eachOf(makeUnexpectedBeginEndMatcher( + BeginExpected, ArgMismatchBegin, BeginEndPairs.EndNames, + MakeReverse, BeginEndPairs.BeginNames), + makeUnexpectedBeginEndMatcher( + EndExpected, ArgMismatchEnd, BeginEndPairs.BeginNames, + MakeReverse, BeginEndPairs.EndNames)); +} + +/// The full matcher for functions that take a range with 2 arguments +static auto makePairRangeMatcher(std::vector<StringRef> FuncNames, + unsigned BeginExpected, unsigned EndExpected, + const FullState &State) { + + return callExpr(callee(functionDecl(hasAnyName(std::move(FuncNames)), + areParametersSameTemplateType( + {BeginExpected, EndExpected}))), + anyOf(makeRangeArgMismatch(BeginExpected, EndExpected, + State.Forward, State.Reversed, + State.MakeReverseIterator), + makeUnexpectedBeginEndPair(BeginExpected, EndExpected, + State.Combined, + State.MakeReverseIterator))) + .bind(Callee); +} + +static auto makeContainerInternalMatcher(std::vector<StringRef> ClassNames, + std::vector<StringRef> ClassMethods, + unsigned InternalExpected, + const FullState &State) { + return cxxMemberCallExpr( + thisPointerType(cxxRecordDecl(hasAnyName(std::move(ClassNames)))), + callee(cxxMethodDecl(hasAnyName(std::move(ClassMethods)))), + on(expr().bind(Internal)), + hasArgument( + InternalExpected, + makeExprMatcher( + expr(unless(matchers::isStatementIdenticalToBoundNode(Internal))) + .bind(InternalOther), + State.All, State.MakeReverseIterator, State.All, + ArgMismatchRevBind) + .bind(InternalArgument))).bind(Callee); +} + +/// Full matcher for class methods that take a range with 2 arguments +static auto makeContainerPairRangeMatcher(std::vector<StringRef> ClassNames, + std::vector<StringRef> ClassMethods, + unsigned BeginExpected, + unsigned EndExpected, + const FullState &State) { + return cxxMemberCallExpr( + thisPointerType(cxxRecordDecl(hasAnyName(std::move(ClassNames)))), + callee(cxxMethodDecl( + hasAnyName(std::move(ClassMethods)), + areParametersSameTemplateType({BeginExpected, EndExpected}))), + anyOf(makeRangeArgMismatch(BeginExpected, EndExpected, + State.Forward, State.Reversed, + State.MakeReverseIterator), + makeUnexpectedBeginEndPair(BeginExpected, EndExpected, + State.Combined, + State.MakeReverseIterator))) + .bind(Callee); +} + +void IncorrectIteratorsCheck::registerMatchers(MatchFinder *Finder) { + NamePairs Forward{NameMatchers{BeginFree, BeginMethod}, + NameMatchers{EndFree, EndMethod}}; + NamePairs Reverse{NameMatchers{RBeginFree, RBeginMethod}, + NameMatchers{REndFree, REndMethod}}; + llvm::SmallVector<StringRef, 8> CombinedFreeBegin{ + llvm::iterator_range{llvm::concat<StringRef>(BeginFree, RBeginFree)}}; + llvm::SmallVector<StringRef, 8> CombinedFreeEnd{ + llvm::iterator_range{llvm::concat<StringRef>(EndFree, REndFree)}}; + llvm::SmallVector<StringRef, 8> CombinedMethodBegin{ + llvm::iterator_range{llvm::concat<StringRef>(BeginMethod, RBeginMethod)}}; + llvm::SmallVector<StringRef, 8> CombinedMethodEnd{ + llvm::iterator_range{llvm::concat<StringRef>(EndMethod, REndMethod)}}; + llvm::SmallVector<StringRef, 16> AllFree{llvm::iterator_range{ + llvm::concat<StringRef>(CombinedFreeBegin, CombinedFreeEnd)}}; + llvm::SmallVector<StringRef, 16> AllMethod{llvm::iterator_range{ + llvm::concat<StringRef>(CombinedMethodBegin, CombinedMethodEnd)}}; + NamePairs Combined{NameMatchers{CombinedFreeBegin, CombinedMethodBegin}, + NameMatchers{CombinedFreeEnd, CombinedMethodEnd}}; + FullState State{ + Forward, Reverse, Combined, {AllFree, AllMethod}, MakeReverseIterator}; + Finder->addMatcher(makePairRangeMatcher({"::std::find"}, 1, 2, State), this); + Finder->addMatcher(makePairRangeMatcher({"::std::find"}, 0, 1, State), this); + Finder->addMatcher( + makeContainerPairRangeMatcher({"::std::vector", "::std::list"}, + {"insert"}, 1, 2, State), + this); + Finder->addMatcher( + makeContainerInternalMatcher({"::std::vector", "::std::list"}, + {"insert", "erase", "emplace"}, 0, State), + this); +} + +void IncorrectIteratorsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CallExpr>(Callee); + if (const auto *BeginMismatch = + Result.Nodes.getNodeAs<Expr>(ArgMismatchBegin)) { + diag(BeginMismatch->getBeginLoc(), + "'end' iterator supplied where a 'begin' iterator is expected") + << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange(); + if (const auto *Rev = + Result.Nodes.getNodeAs<CallExpr>(ArgMismatchRevBind)) { + diag(Rev->getBeginLoc(), "%0 changes 'begin' into a 'end' iterator", + DiagnosticIDs::Note) + << Rev->getSourceRange() << Rev->getDirectCallee(); + } + } else if (const auto *EndMismatch = + Result.Nodes.getNodeAs<Expr>(ArgMismatchEnd)) { + diag(EndMismatch->getBeginLoc(), + "'begin' iterator supplied where an 'end' iterator is expected") + << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange(); + if (const auto *Rev = + Result.Nodes.getNodeAs<CallExpr>(ArgMismatchRevBind)) { + diag(Rev->getBeginLoc(), "%0 changes 'end' into a 'begin' iterator", + DiagnosticIDs::Note) + << Rev->getSourceRange() << Rev->getDirectCallee(); + } + } else if (const auto *InternalArg = + Result.Nodes.getNodeAs<Expr>(InternalArgument)) { + diag(InternalArg->getBeginLoc(), + "%0 called with an iterator for a different container") + << Call->getDirectCallee(); + const auto *Object = Result.Nodes.getNodeAs<Expr>(Internal); + diag(Object->getBeginLoc(), "container is specified here", + DiagnosticIDs::Note) + << Object->getSourceRange(); + const auto *Other = Result.Nodes.getNodeAs<Expr>(InternalOther); + diag(Other->getBeginLoc(), "different container provided here", + DiagnosticIDs::Note) + << Other->getSourceRange(); + } else { + const auto *Range1 = Result.Nodes.getNodeAs<Expr>(FirstRangeArg); + const auto *Range2 = Result.Nodes.getNodeAs<Expr>(SecondRangeArg); + const auto *FullRange1 = Result.Nodes.getNodeAs<Expr>(FirstRangeArgExpr); + const auto *FullRange2 = Result.Nodes.getNodeAs<Expr>(SecondRangeArgExpr); + assert(Range1 && Range2 && FullRange1 && FullRange2 && "Unexpected match"); + diag(Call->getBeginLoc(), "mismatched ranges pass to function") + << FullRange1->getSourceRange() << FullRange2->getSourceRange(); + diag(Range1->getBeginLoc(), "first range passed here to begin", + DiagnosticIDs::Note) + << FullRange1->getSourceRange(); + diag(Range2->getBeginLoc(), "different range passed here to end", + DiagnosticIDs::Note) + << FullRange2->getSourceRange(); + } +} + +IncorrectIteratorsCheck::IncorrectIteratorsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + BeginFree(utils::options::parseStringList( + Options.get("BeginFree", "::std::begin;::std::cbegin"))), + EndFree(utils::options::parseStringList( + Options.get("EndFree", "::std::end;::std::cend"))), + BeginMethod(utils::options::parseStringList( + Options.get("BeginMethod", "begin;cbegin"))), + EndMethod(utils::options::parseStringList( + Options.get("EndMethod", "end;cend"))), + RBeginFree(utils::options::parseStringList( + Options.get("RBeginFree", "::std::rbegin;::std::crbegin"))), + REndFree(utils::options::parseStringList( + Options.get("REndFree", "::std::rend;::std::crend"))), + RBeginMethod(utils::options::parseStringList( + Options.get("RBeginMethod", "rbegin;crbegin"))), + REndMethod(utils::options::parseStringList( + Options.get("REndMethod", "rend;crend"))), + MakeReverseIterator(utils::options::parseStringList( + Options.get("MakeReverseIterator", "::std::make_reverse_iterator"))) { +} + +void IncorrectIteratorsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "BeginFree", + utils::options::serializeStringList(BeginFree)); + Options.store(Opts, "EndFree", utils::options::serializeStringList(EndFree)); + Options.store(Opts, "BeginMethod", + utils::options::serializeStringList(BeginMethod)); + Options.store(Opts, "EndMethod", + utils::options::serializeStringList(EndMethod)); + Options.store(Opts, "RBeginFree", + utils::options::serializeStringList(RBeginFree)); + Options.store(Opts, "REndFree", + utils::options::serializeStringList(REndFree)); + Options.store(Opts, "RBeginMethod", + utils::options::serializeStringList(RBeginMethod)); + Options.store(Opts, "REndMethod", + utils::options::serializeStringList(REndMethod)); + Options.store(Opts, "MakeReverseIterator", + utils::options::serializeStringList(MakeReverseIterator)); +} +std::optional<TraversalKind> +IncorrectIteratorsCheck::getCheckTraversalKind() const { + return TK_IgnoreUnlessSpelledInSource; +} +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h new file mode 100644 index 0000000000000..9f33b61c5a981 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h @@ -0,0 +1,44 @@ +//===--- IncorrectIteratorsCheck.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_BUGPRONE_INCORRECTITERATORSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTITERATORSCHECK_H + +#include "../ClangTidyCheck.h" +#include "llvm/ADT/StringRef.h" + +namespace clang::tidy::bugprone { + +/// Detects calls to iterator alg... [truncated] |
Member
| @llvm/pr-subscribers-clang-tidy Author: Nathan James (njames93) ChangesLooks for various patterns of functions that take arguments calling begin/end or similar for common mistakes. Still needs a bit of work, but open to suggestions of how this check could be improved Patch is 35.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99917.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 689eb92a3d8d1..cea040b81878a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -33,6 +33,7 @@ #include "InaccurateEraseCheck.h" #include "IncDecInConditionsCheck.h" #include "IncorrectEnableIfCheck.h" +#include "IncorrectIteratorsCheck.h" #include "IncorrectRoundingsCheck.h" #include "InfiniteLoopCheck.h" #include "IntegerDivisionCheck.h" @@ -139,6 +140,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); + CheckFactories.registerCheck<IncorrectIteratorsCheck>( + "bugprone-incorrect-iterators"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index cb0d8ae98bac5..8425dbed0505a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule ImplicitWideningOfMultiplicationResultCheck.cpp InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp + IncorrectIteratorsCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp new file mode 100644 index 0000000000000..56429b9100146 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp @@ -0,0 +1,362 @@ +//===--- IncorrectIteratorsCheck.cpp - clang-tidy -------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#include "IncorrectIteratorsCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/iterator_range.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { +using SVU = llvm::SmallVector<unsigned, 3>; +/// Checks to see if a function a +AST_MATCHER_P(FunctionDecl, areParametersSameTemplateType, SVU, Indexes) { + const auto *TemplateDecl = Node.getPrimaryTemplate(); + if (!TemplateDecl) + return false; + const auto *FuncDecl = TemplateDecl->getTemplatedDecl(); + if (!FuncDecl) + return false; + assert(!Indexes.empty()); + const auto *FirstParam = FuncDecl->getParamDecl(Indexes.front()); + if (!FirstParam) + return false; + auto Type = FirstParam->getOriginalType(); + for (auto Item : llvm::drop_begin(Indexes)) { + const auto *Param = FuncDecl->getParamDecl(Item); + if (!Param) + return false; + if (Param->getOriginalType() != Type) + return false; + } + return true; +} +struct NameMatchers { + ArrayRef<StringRef> FreeNames; + ArrayRef<StringRef> MethodNames; +}; + +struct NamePairs { + NameMatchers BeginNames; + NameMatchers EndNames; +}; + +struct FullState { + NamePairs Forward; + NamePairs Reversed; + NamePairs Combined; + NameMatchers All; + ArrayRef<StringRef> MakeReverseIterator; +}; + +} // namespace + +static constexpr char FirstRangeArg[] = "FirstRangeArg"; +static constexpr char FirstRangeArgExpr[] = "FirstRangeArgExpr"; +static constexpr char ReverseBeginBind[] = "ReverseBeginBind"; +static constexpr char ReverseEndBind[] = "ReverseEndBind"; +static constexpr char SecondRangeArg[] = "SecondRangeArg"; +static constexpr char SecondRangeArgExpr[] = "SecondRangeArgExpr"; +static constexpr char ArgMismatchBegin[] = "ArgMismatchBegin"; +static constexpr char ArgMismatchEnd[] = "ArgMismatchEnd"; +static constexpr char ArgMismatchExpr[] = "ArgMismatchExpr"; +static constexpr char ArgMismatchRevBind[] = "ArgMismatchRevBind"; +static constexpr char Callee[] = "Callee"; +static constexpr char Internal[] = "Internal"; +static constexpr char InternalOther[] = "InternalOther"; +static constexpr char InternalArgument[] = "InternalArgument"; + +static auto +makeExprMatcher(ast_matchers::internal::Matcher<Expr> ArgumentMatcher, + const NameMatchers &Names, ArrayRef<StringRef> MakeReverse, + const NameMatchers &RevNames, StringRef RevBind) { + return expr(anyOf( + cxxMemberCallExpr(argumentCountIs(0), + callee(cxxMethodDecl(hasAnyName(Names.MethodNames))), + on(ArgumentMatcher)), + callExpr(argumentCountIs(1), + hasDeclaration(functionDecl(hasAnyName(Names.FreeNames))), + hasArgument(0, ArgumentMatcher)), + callExpr( + callee(functionDecl(hasAnyName(MakeReverse))), argumentCountIs(1), + hasArgument( + 0, expr(anyOf(cxxMemberCallExpr(argumentCountIs(0), + callee(cxxMethodDecl(hasAnyName( + RevNames.MethodNames))), + on(ArgumentMatcher)), + callExpr(argumentCountIs(1), + hasDeclaration(functionDecl( + hasAnyName(RevNames.FreeNames))), + hasArgument(0, ArgumentMatcher)))))) + .bind(RevBind))); +} + +// Detects a range function where the argument for the begin call differs from +// the end call std::find(I.begin(), J.end()) +static auto makeRangeArgMismatch(unsigned BeginExpected, unsigned EndExpected, + NamePairs Forward, NamePairs Reverse, + ArrayRef<StringRef> MakeReverse) { + std::vector<ast_matchers::internal::DynTypedMatcher> Matchers; + + for (const auto &[Primary, Backwards] : + {std::pair{Forward, Reverse}, std::pair{Reverse, Forward}}) { + Matchers.push_back(callExpr( + hasArgument(BeginExpected, + makeExprMatcher(expr().bind(FirstRangeArg), + Primary.BeginNames, MakeReverse, + Backwards.EndNames, ReverseBeginBind) + .bind(FirstRangeArgExpr)), + hasArgument(EndExpected, + makeExprMatcher( + expr(unless(matchers::isStatementIdenticalToBoundNode( + FirstRangeArg))) + .bind(SecondRangeArg), + Primary.EndNames, MakeReverse, Backwards.BeginNames, + ReverseEndBind) + .bind(SecondRangeArgExpr)))); + } + + return callExpr( + argumentCountAtLeast(std::max(BeginExpected, EndExpected) + 1), + ast_matchers::internal::DynTypedMatcher::constructVariadic( + ast_matchers::internal::DynTypedMatcher::VO_AnyOf, + ASTNodeKind::getFromNodeKind<CallExpr>(), std::move(Matchers)) + .convertTo<CallExpr>()); +} + +// Detects a range function where we expect a call to begin but get a call to +// end or vice versa std::find(X.end(), X.end()) // Here we would warn on the +// first argument that it should be begin +static auto makeUnexpectedBeginEndMatcher(unsigned Index, StringRef BindTo, + NameMatchers Names, + ArrayRef<StringRef> MakeReverse, + const NameMatchers &RevNames) { + return callExpr(hasArgument(Index, makeExprMatcher(expr().bind(BindTo), Names, + MakeReverse, RevNames, + ArgMismatchRevBind) + .bind(ArgMismatchExpr))); +} + +static auto makeUnexpectedBeginEndPair(unsigned BeginExpected, + unsigned EndExpected, + NamePairs BeginEndPairs, + ArrayRef<StringRef> MakeReverse) { + // return expr(unless(anything())); + return eachOf(makeUnexpectedBeginEndMatcher( + BeginExpected, ArgMismatchBegin, BeginEndPairs.EndNames, + MakeReverse, BeginEndPairs.BeginNames), + makeUnexpectedBeginEndMatcher( + EndExpected, ArgMismatchEnd, BeginEndPairs.BeginNames, + MakeReverse, BeginEndPairs.EndNames)); +} + +/// The full matcher for functions that take a range with 2 arguments +static auto makePairRangeMatcher(std::vector<StringRef> FuncNames, + unsigned BeginExpected, unsigned EndExpected, + const FullState &State) { + + return callExpr(callee(functionDecl(hasAnyName(std::move(FuncNames)), + areParametersSameTemplateType( + {BeginExpected, EndExpected}))), + anyOf(makeRangeArgMismatch(BeginExpected, EndExpected, + State.Forward, State.Reversed, + State.MakeReverseIterator), + makeUnexpectedBeginEndPair(BeginExpected, EndExpected, + State.Combined, + State.MakeReverseIterator))) + .bind(Callee); +} + +static auto makeContainerInternalMatcher(std::vector<StringRef> ClassNames, + std::vector<StringRef> ClassMethods, + unsigned InternalExpected, + const FullState &State) { + return cxxMemberCallExpr( + thisPointerType(cxxRecordDecl(hasAnyName(std::move(ClassNames)))), + callee(cxxMethodDecl(hasAnyName(std::move(ClassMethods)))), + on(expr().bind(Internal)), + hasArgument( + InternalExpected, + makeExprMatcher( + expr(unless(matchers::isStatementIdenticalToBoundNode(Internal))) + .bind(InternalOther), + State.All, State.MakeReverseIterator, State.All, + ArgMismatchRevBind) + .bind(InternalArgument))).bind(Callee); +} + +/// Full matcher for class methods that take a range with 2 arguments +static auto makeContainerPairRangeMatcher(std::vector<StringRef> ClassNames, + std::vector<StringRef> ClassMethods, + unsigned BeginExpected, + unsigned EndExpected, + const FullState &State) { + return cxxMemberCallExpr( + thisPointerType(cxxRecordDecl(hasAnyName(std::move(ClassNames)))), + callee(cxxMethodDecl( + hasAnyName(std::move(ClassMethods)), + areParametersSameTemplateType({BeginExpected, EndExpected}))), + anyOf(makeRangeArgMismatch(BeginExpected, EndExpected, + State.Forward, State.Reversed, + State.MakeReverseIterator), + makeUnexpectedBeginEndPair(BeginExpected, EndExpected, + State.Combined, + State.MakeReverseIterator))) + .bind(Callee); +} + +void IncorrectIteratorsCheck::registerMatchers(MatchFinder *Finder) { + NamePairs Forward{NameMatchers{BeginFree, BeginMethod}, + NameMatchers{EndFree, EndMethod}}; + NamePairs Reverse{NameMatchers{RBeginFree, RBeginMethod}, + NameMatchers{REndFree, REndMethod}}; + llvm::SmallVector<StringRef, 8> CombinedFreeBegin{ + llvm::iterator_range{llvm::concat<StringRef>(BeginFree, RBeginFree)}}; + llvm::SmallVector<StringRef, 8> CombinedFreeEnd{ + llvm::iterator_range{llvm::concat<StringRef>(EndFree, REndFree)}}; + llvm::SmallVector<StringRef, 8> CombinedMethodBegin{ + llvm::iterator_range{llvm::concat<StringRef>(BeginMethod, RBeginMethod)}}; + llvm::SmallVector<StringRef, 8> CombinedMethodEnd{ + llvm::iterator_range{llvm::concat<StringRef>(EndMethod, REndMethod)}}; + llvm::SmallVector<StringRef, 16> AllFree{llvm::iterator_range{ + llvm::concat<StringRef>(CombinedFreeBegin, CombinedFreeEnd)}}; + llvm::SmallVector<StringRef, 16> AllMethod{llvm::iterator_range{ + llvm::concat<StringRef>(CombinedMethodBegin, CombinedMethodEnd)}}; + NamePairs Combined{NameMatchers{CombinedFreeBegin, CombinedMethodBegin}, + NameMatchers{CombinedFreeEnd, CombinedMethodEnd}}; + FullState State{ + Forward, Reverse, Combined, {AllFree, AllMethod}, MakeReverseIterator}; + Finder->addMatcher(makePairRangeMatcher({"::std::find"}, 1, 2, State), this); + Finder->addMatcher(makePairRangeMatcher({"::std::find"}, 0, 1, State), this); + Finder->addMatcher( + makeContainerPairRangeMatcher({"::std::vector", "::std::list"}, + {"insert"}, 1, 2, State), + this); + Finder->addMatcher( + makeContainerInternalMatcher({"::std::vector", "::std::list"}, + {"insert", "erase", "emplace"}, 0, State), + this); +} + +void IncorrectIteratorsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CallExpr>(Callee); + if (const auto *BeginMismatch = + Result.Nodes.getNodeAs<Expr>(ArgMismatchBegin)) { + diag(BeginMismatch->getBeginLoc(), + "'end' iterator supplied where a 'begin' iterator is expected") + << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange(); + if (const auto *Rev = + Result.Nodes.getNodeAs<CallExpr>(ArgMismatchRevBind)) { + diag(Rev->getBeginLoc(), "%0 changes 'begin' into a 'end' iterator", + DiagnosticIDs::Note) + << Rev->getSourceRange() << Rev->getDirectCallee(); + } + } else if (const auto *EndMismatch = + Result.Nodes.getNodeAs<Expr>(ArgMismatchEnd)) { + diag(EndMismatch->getBeginLoc(), + "'begin' iterator supplied where an 'end' iterator is expected") + << Result.Nodes.getNodeAs<Expr>(ArgMismatchExpr)->getSourceRange(); + if (const auto *Rev = + Result.Nodes.getNodeAs<CallExpr>(ArgMismatchRevBind)) { + diag(Rev->getBeginLoc(), "%0 changes 'end' into a 'begin' iterator", + DiagnosticIDs::Note) + << Rev->getSourceRange() << Rev->getDirectCallee(); + } + } else if (const auto *InternalArg = + Result.Nodes.getNodeAs<Expr>(InternalArgument)) { + diag(InternalArg->getBeginLoc(), + "%0 called with an iterator for a different container") + << Call->getDirectCallee(); + const auto *Object = Result.Nodes.getNodeAs<Expr>(Internal); + diag(Object->getBeginLoc(), "container is specified here", + DiagnosticIDs::Note) + << Object->getSourceRange(); + const auto *Other = Result.Nodes.getNodeAs<Expr>(InternalOther); + diag(Other->getBeginLoc(), "different container provided here", + DiagnosticIDs::Note) + << Other->getSourceRange(); + } else { + const auto *Range1 = Result.Nodes.getNodeAs<Expr>(FirstRangeArg); + const auto *Range2 = Result.Nodes.getNodeAs<Expr>(SecondRangeArg); + const auto *FullRange1 = Result.Nodes.getNodeAs<Expr>(FirstRangeArgExpr); + const auto *FullRange2 = Result.Nodes.getNodeAs<Expr>(SecondRangeArgExpr); + assert(Range1 && Range2 && FullRange1 && FullRange2 && "Unexpected match"); + diag(Call->getBeginLoc(), "mismatched ranges pass to function") + << FullRange1->getSourceRange() << FullRange2->getSourceRange(); + diag(Range1->getBeginLoc(), "first range passed here to begin", + DiagnosticIDs::Note) + << FullRange1->getSourceRange(); + diag(Range2->getBeginLoc(), "different range passed here to end", + DiagnosticIDs::Note) + << FullRange2->getSourceRange(); + } +} + +IncorrectIteratorsCheck::IncorrectIteratorsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + BeginFree(utils::options::parseStringList( + Options.get("BeginFree", "::std::begin;::std::cbegin"))), + EndFree(utils::options::parseStringList( + Options.get("EndFree", "::std::end;::std::cend"))), + BeginMethod(utils::options::parseStringList( + Options.get("BeginMethod", "begin;cbegin"))), + EndMethod(utils::options::parseStringList( + Options.get("EndMethod", "end;cend"))), + RBeginFree(utils::options::parseStringList( + Options.get("RBeginFree", "::std::rbegin;::std::crbegin"))), + REndFree(utils::options::parseStringList( + Options.get("REndFree", "::std::rend;::std::crend"))), + RBeginMethod(utils::options::parseStringList( + Options.get("RBeginMethod", "rbegin;crbegin"))), + REndMethod(utils::options::parseStringList( + Options.get("REndMethod", "rend;crend"))), + MakeReverseIterator(utils::options::parseStringList( + Options.get("MakeReverseIterator", "::std::make_reverse_iterator"))) { +} + +void IncorrectIteratorsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "BeginFree", + utils::options::serializeStringList(BeginFree)); + Options.store(Opts, "EndFree", utils::options::serializeStringList(EndFree)); + Options.store(Opts, "BeginMethod", + utils::options::serializeStringList(BeginMethod)); + Options.store(Opts, "EndMethod", + utils::options::serializeStringList(EndMethod)); + Options.store(Opts, "RBeginFree", + utils::options::serializeStringList(RBeginFree)); + Options.store(Opts, "REndFree", + utils::options::serializeStringList(REndFree)); + Options.store(Opts, "RBeginMethod", + utils::options::serializeStringList(RBeginMethod)); + Options.store(Opts, "REndMethod", + utils::options::serializeStringList(REndMethod)); + Options.store(Opts, "MakeReverseIterator", + utils::options::serializeStringList(MakeReverseIterator)); +} +std::optional<TraversalKind> +IncorrectIteratorsCheck::getCheckTraversalKind() const { + return TK_IgnoreUnlessSpelledInSource; +} +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h new file mode 100644 index 0000000000000..9f33b61c5a981 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.h @@ -0,0 +1,44 @@ +//===--- IncorrectIteratorsCheck.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_BUGPRONE_INCORRECTITERATORSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTITERATORSCHECK_H + +#include "../ClangTidyCheck.h" +#include "llvm/ADT/StringRef.h" + +namespace clang::tidy::bugprone { + +/// Detects calls to iterator alg... [truncated] |
| ✅ With the latest revision this PR passed the C/C++ code formatter. |
PiotrZSL reviewed Jul 22, 2024
njames93 force-pushed the ct-incorrect-iterators branch 2 times, most recently from e71d153 to 7be8e9d Compare njames93 changed the title njames93 force-pushed the ct-incorrect-iterators branch from 7be8e9d to 1b91c22 Compare 
clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
njames93 force-pushed the ct-incorrect-iterators branch 2 times, most recently from 4293365 to 0c85592 Compare 
5chmidti reviewed Jul 26, 2024
Contributor
5chmidti left a comment
5chmidti left a comment There was a problem hiding this comment.
Some initial thoughts, and at first glance, this looks nice. Because we are early in the release cycle, there will be enough time to address the fixme's.
(I haven't run the check against some code bases as I am not at my main machine for another week.)
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-iterators.rst Outdated Show resolved Hide resolved
clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp Outdated Show resolved Hide resolved
clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp Outdated Show resolved Hide resolved
clang-tools-extra/clang-tidy/bugprone/IncorrectIteratorsCheck.cpp Outdated Show resolved Hide resolved
Comment on lines 619 to 847
| Finder->addMatcher( | ||
| makeContainerPairRangeMatcher({"::std::vector", "::std::list"}, | ||
| {"insert"}, 1, 2, State), | ||
| this); | ||
| Finder->addMatcher( | ||
| makeContainerInternalMatcher({"::std::vector", "::std::list"}, | ||
| {"insert", "erase", "emplace"}, 0, State), | ||
| this); |
Contributor
There was a problem hiding this comment.
There are some more container member that expect an iterator of the container, such as std::deque::erase. Can we add them now?
Member Author
There was a problem hiding this comment.
Yeah, this is a little sore point, that there's not yet(#100349) a nice way to specify the available permutations
njames93 force-pushed the ct-incorrect-iterators branch 2 times, most recently from bbf32f3 to e1c3086 Compare 
Member Author
| Added some detection for trying to advance past the end or before the start of a range. |
njames93 force-pushed the ct-incorrect-iterators branch from e1c3086 to c79951f Compare Member Author
| @5chmidti I decided to run this version of the check on llvm and clang Seems like most of the warnings detected are when we are deliberately exceeding the bounds of a range when we know its a slice of a bigger range. Some of them are potentially cases where code could be refactored to better express it's intent, other instances already have comments to explain why we are exceeding the bounds There were also some cases where classes had begin/end pairs that didn't return iterators, instead just integral types. |
Contributor
| The Checking the log, it looks like the check works well. I know that TBB has |
Member Author
The |
5chmidti approved these changes Jul 29, 2024
Contributor
5chmidti left a comment
5chmidti left a comment There was a problem hiding this comment.
LGTM (- single statement compound statements), but wait for another review please
njames93 force-pushed the ct-incorrect-iterators branch 2 times, most recently from 4762dc0 to f32add6 Compare Member Author
|
njames93 force-pushed the ct-incorrect-iterators branch 2 times, most recently from e604ce2 to e4d6c8e Compare 
Looks for various patterns of functions that take arguments calling begin/end or similar for common mistakes
njames93 force-pushed the ct-incorrect-iterators branch from e4d6c8e to d0cf6a5 Compare 
whisperity reviewed Aug 2, 2024
Comment on lines +32 to +40
| std::vector<StringRef> BeginFree; | ||
| std::vector<StringRef> EndFree; | ||
| std::vector<StringRef> BeginMethod; | ||
| std::vector<StringRef> EndMethod; | ||
| std::vector<StringRef> RBeginFree; | ||
| std::vector<StringRef> REndFree; | ||
| std::vector<StringRef> RBeginMethod; | ||
| std::vector<StringRef> REndMethod; | ||
| std::vector<StringRef> MakeReverseIterator; |
Member
There was a problem hiding this comment.
These are usually const, as these correspond to the options obtained from the user's configuration.
Member
| What happens in the following case? std::vector V; const auto& V2 = V; std::find(V.begin(), V2.end(), 42);These are syntactically different symbols but still should be the same underlying memory area. Perhaps it could be mentioned in the documentation, that the Clang Static Analyser has (albeit only as alpha...) some iterator-related checkers as well, where range constraints are (attempted to be?) modelled path-sensitively. Maybe @baloghadamsoftware or @gamesh411 still remembers the status of those checkers. |
denzor200 suggested changes Jun 23, 2025
| // CHECK-NOTES: :[[@LINE-3]]:61: warning: 'end' iterator supplied where a 'begin' iterator is expected | ||
| // CHECK-NOTES: :[[@LINE-3]]:18: warning: 'end' iterator supplied where an output iterator is expected | ||
| | ||
| std::push_heap(std::begin(I), std::end(J)); |
There was a problem hiding this comment.
Please add test case:
std::vector<int> get_data() { return std::vector<int>{1,2,3,4}; } void test() { std::push_heap(std::begin(get_data()), std::end(get_data())); } There should also be a warning here "mismatched ranges supplied to 'std::push_heap'"
Contributor
| Please rebase from |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Looks for various patterns of functions that take arguments calling begin/end or similar for common mistakes.