Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -10,6 +10,7 @@ IdentifierNamingCheck.cpp ImplicitBoolCastCheck.cpp InconsistentDeclarationParameterNameCheck.cpp + MisplacedArrayIndexCheck.cpp NamedParameterCheck.cpp NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp Index: clang-tidy/readability/MisplacedArrayIndexCheck.h =================================================================== --- clang-tidy/readability/MisplacedArrayIndexCheck.h +++ clang-tidy/readability/MisplacedArrayIndexCheck.h @@ -0,0 +1,36 @@ +//===--- MisplacedArrayIndexCheck.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_MISPLACED_ARRAY_INDEX_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Warn about unusual array index syntax (index[array] instead of +/// array[index]). +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-misplaced-array-index.html +class MisplacedArrayIndexCheck : public ClangTidyCheck { +public: + MisplacedArrayIndexCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H Index: clang-tidy/readability/MisplacedArrayIndexCheck.cpp =================================================================== --- clang-tidy/readability/MisplacedArrayIndexCheck.cpp +++ clang-tidy/readability/MisplacedArrayIndexCheck.cpp @@ -0,0 +1,81 @@ +//===--- MisplacedArrayIndexCheck.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 "MisplacedArrayIndexCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void MisplacedArrayIndexCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + arraySubscriptExpr( + hasLHS(hasType(isInteger())), + hasRHS(ignoringParenImpCasts( + anyOf(stringLiteral(), declRefExpr(hasType(pointsTo(qualType()))), + memberExpr(hasType(pointsTo(qualType()))))))) + .bind("expr"), + this); +} + +static bool hasMacroID(const Expr *E) { + if (E->getExprLoc().isMacroID()) + return true; + for (const Stmt *Child : E->children()) { + const Expr *ChildExpr = dyn_cast(Child); + if (ChildExpr && hasMacroID(ChildExpr)) + return true; + } + return false; +} + +static StringRef getAsString(const MatchFinder::MatchResult &Result, + const Expr *E) { + const SourceManager &SM = *Result.SourceManager; + const SourceRange &R = E->getSourceRange(); + + // Don't even try to resolve macro or include contraptions. Not worth emitting + // a fixit for. + if (hasMacroID(E) || !SM.isWrittenInSameFile(R.getBegin(), R.getEnd())) + return StringRef(); + + const char *Begin = SM.getCharacterData(R.getBegin()); + const char *End = SM.getCharacterData(Lexer::getLocForEndOfToken( + R.getEnd(), 0, SM, Result.Context->getLangOpts())); + + return StringRef(Begin, End - Begin); +} + +void MisplacedArrayIndexCheck::check(const MatchFinder::MatchResult &Result) { + const auto *ArraySubscriptE = + Result.Nodes.getNodeAs("expr"); + + auto D = + diag(ArraySubscriptE->getLocStart(), + "unusual array index syntax, usually the index is inside the []"); + + StringRef LHSString = getAsString(Result, ArraySubscriptE->getLHS()); + StringRef RHSString = getAsString(Result, ArraySubscriptE->getRHS()); + if (LHSString.empty() || RHSString.empty()) + return; + + D << FixItHint::CreateReplacement(ArraySubscriptE->getLHS()->getSourceRange(), + RHSString); + D << FixItHint::CreateReplacement(ArraySubscriptE->getRHS()->getSourceRange(), + LHSString); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -19,6 +19,7 @@ #include "IdentifierNamingCheck.h" #include "ImplicitBoolCastCheck.h" #include "InconsistentDeclarationParameterNameCheck.h" +#include "MisplacedArrayIndexCheck.h" #include "NamedParameterCheck.h" #include "RedundantControlFlowCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -53,6 +54,8 @@ "readability-implicit-bool-cast"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-misplaced-array-index"); CheckFactories.registerCheck( "readability-static-definition-in-anonymous-namespace"); CheckFactories.registerCheck( Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -249,6 +249,11 @@ Warns about defaulted constructors and assignment operators that are actually deleted. +- New `misplaced-array-index + `_ check + + Warns when there is array index before the [] instead of inside it. + - New `readability-redundant-control-flow `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -120,6 +120,7 @@ readability-identifier-naming readability-implicit-bool-cast readability-inconsistent-declaration-parameter-name + readability-misplaced-array-index readability-named-parameter readability-redundant-control-flow readability-redundant-smartptr-get Index: docs/clang-tidy/checks/readability-misplaced-array-index.rst =================================================================== --- docs/clang-tidy/checks/readability-misplaced-array-index.rst +++ docs/clang-tidy/checks/readability-misplaced-array-index.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - readability-misplaced-array-index + +readability-misplaced-array-index +================================= + +This check warns for unusual array index syntax. + +The following code has unusual array index syntax: + +.. code:: c++ + + void f(int *x, int y) + { + y[x] = 0; + } + +becomes + +.. code:: c++ + + void f(int *x, int y) + { + x[y] = 0; + } + +The check warns about such unusual syntax for readability reasons: + * There are programmers that are not familiar with this unusual syntax. + * It is possible that variables are mixed up. + Index: test/clang-tidy/readability-misplaced-array-index.cpp =================================================================== --- test/clang-tidy/readability-misplaced-array-index.cpp +++ test/clang-tidy/readability-misplaced-array-index.cpp @@ -0,0 +1,32 @@ +// RUN: %check_clang_tidy %s readability-misplaced-array-index %t + +#define ABC "abc" + +struct XY { int *x; int *y; }; + +void dostuff(int); + +void unusualSyntax(int *x, struct XY *xy) +{ + 10[x] = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the [] + // CHECK-FIXES: x[10] = 0; + + 10[xy->x] = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unusual array index syntax, usually the index is inside the [] + // CHECK-FIXES: xy->x[10] = 0; + + dostuff(1["abc"]); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the [] + // CHECK-FIXES: dostuff("abc"[1]); + + dostuff(1[ABC]); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: unusual array index syntax, usually the index is inside the [] + // No fixit, we don't want to replace with "abc"[1] +} + +void normalSyntax(int *x) +{ + x[10] = 0; + dostuff(0[0 + ABC]); +}