From 03ea468a1ccc39a1a451a12b1ce48d43678aa927 Mon Sep 17 00:00:00 2001 From: Daniel Marjamaki Date: Mon, 12 Sep 2016 12:04:13 +0000 Subject: [PATCH] [clang-tidy] readability-misplaced-array-index: add new check that warns when array index is misplaced. Reviewers: alexfh Differential Revision: https://reviews.llvm.org/D21134 llvm-svn: 281206 --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/MisplacedArrayIndexCheck.cpp | 57 +++++++++++++++++++ .../readability/MisplacedArrayIndexCheck.h | 36 ++++++++++++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../readability-misplaced-array-index.rst | 27 +++++++++ .../readability-misplaced-array-index.cpp | 34 +++++++++++ 8 files changed, 164 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/MisplacedArrayIndexCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/MisplacedArrayIndexCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability-misplaced-array-index.rst create mode 100644 clang-tools-extra/test/clang-tidy/readability-misplaced-array-index.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 1e80f9405107..c2ec45c10683 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -10,6 +10,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierNamingCheck.cpp ImplicitBoolCastCheck.cpp InconsistentDeclarationParameterNameCheck.cpp + MisplacedArrayIndexCheck.cpp NamedParameterCheck.cpp NamespaceCommentCheck.cpp NonConstParameterCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/MisplacedArrayIndexCheck.cpp b/clang-tools-extra/clang-tidy/readability/MisplacedArrayIndexCheck.cpp new file mode 100644 index 000000000000..f5e09fabb5ec --- /dev/null +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/clang-tidy/readability/MisplacedArrayIndexCheck.h b/clang-tools-extra/clang-tidy/readability/MisplacedArrayIndexCheck.h new file mode 100644 index 000000000000..e9a22314f6cf --- /dev/null +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b32bbacf22d0..6aedba202a09 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/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 @@ public: "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( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 9b9562055e31..8a7ba63e42cc 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -89,6 +89,11 @@ Improvements to clang-tidy 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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 41d0d792af6e..24922837616d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -128,6 +128,7 @@ Clang-Tidy Checks 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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-misplaced-array-index.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-misplaced-array-index.rst new file mode 100644 index 000000000000..26a5af779d7a --- /dev/null +++ b/clang-tools-extra/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. + diff --git a/clang-tools-extra/test/clang-tidy/readability-misplaced-array-index.cpp b/clang-tools-extra/test/clang-tidy/readability-misplaced-array-index.cpp new file mode 100644 index 000000000000..43dd4583d5e7 --- /dev/null +++ b/clang-tools-extra/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; +}