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 NonConstParameterCheck.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,57 @@ +//===--- 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" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void MisplacedArrayIndexCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(arraySubscriptExpr(hasLHS(hasType(isInteger())), + hasRHS(hasType(isAnyPointer()))) + .bind("expr"), + this); +} + +void MisplacedArrayIndexCheck::check(const MatchFinder::MatchResult &Result) { + const auto *ArraySubscriptE = + Result.Nodes.getNodeAs("expr"); + + auto Diag = diag(ArraySubscriptE->getLocStart(), "confusing array subscript " + "expression, usually the " + "index is inside the []"); + + // Only try to fixit when LHS and RHS can be swapped directly without changing + // the logic. + const Expr *RHSE = ArraySubscriptE->getRHS()->IgnoreParenImpCasts(); + if (!isa(RHSE) && !isa(RHSE) && + !isa(RHSE)) + return; + + const StringRef LText = tooling::fixit::getText( + ArraySubscriptE->getLHS()->getSourceRange(), *Result.Context); + const StringRef RText = tooling::fixit::getText( + ArraySubscriptE->getRHS()->getSourceRange(), *Result.Context); + + Diag << FixItHint::CreateReplacement( + ArraySubscriptE->getLHS()->getSourceRange(), RText); + Diag << FixItHint::CreateReplacement( + ArraySubscriptE->getRHS()->getSourceRange(), LText); +} + +} // 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 "NonConstParameterCheck.h" #include "RedundantControlFlowCheck.h" @@ -54,6 +55,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 @@ -85,6 +85,11 @@ Warns about the performance overhead arising from concatenating strings using the ``operator+``, instead of ``operator+=``. +- New `readability-misplaced-array-index + `_ check + + Warns when there is array index before the [] instead of inside it. + - New `readability-non-const-parameter `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -127,6 +127,7 @@ readability-identifier-naming readability-implicit-bool-cast readability-inconsistent-declaration-parameter-name + readability-misplaced-array-index readability-named-parameter readability-non-const-parameter readability-redundant-control-flow 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,27 @@ +.. 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-block:: c++ + + void f(int *X, int Y) { + Y[X] = 0; + } + +becomes + +.. code-block:: 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,34 @@ +// 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 *P1, struct XY *P2) { + 10[P1] = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the [] + // CHECK-FIXES: P1[10] = 0; + + 10[P2->X] = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression + // CHECK-FIXES: P2->X[10] = 0; + + dostuff(1["abc"]); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression + // CHECK-FIXES: dostuff("abc"[1]); + + dostuff(1[ABC]); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression + // CHECK-FIXES: dostuff(ABC[1]); + + dostuff(0[0 + ABC]); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression + // CHECK-FIXES: dostuff(0[0 + ABC]); + // No fixit. Probably the code should be ABC[0] +} + +void normalSyntax(int *X) { + X[10] = 0; +}