[clang-tidy] Add readability-misleading-indentation check.

Differential Revision: https://reviews.llvm.org/D19586

llvm-svn: 295041

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityModule
   IdentifierNamingCheck.cpp
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
+  MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp

clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp

@@ -0,0 +1,103 @@
+//===--- MisleadingIndentationCheck.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 "MisleadingIndentationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void MisleadingIndentationCheck::danglingElseCheck(const SourceManager &SM,
+                                                   const IfStmt *If) {
+  SourceLocation IfLoc = If->getIfLoc();
+  SourceLocation ElseLoc = If->getElseLoc();
+
+  if (IfLoc.isMacroID() || ElseLoc.isMacroID())
+    return;
+
+  if (SM.getExpansionLineNumber(If->getThen()->getLocEnd()) ==
+      SM.getExpansionLineNumber(ElseLoc))
+    return;
+
+  if (SM.getExpansionColumnNumber(IfLoc) !=
+      SM.getExpansionColumnNumber(ElseLoc))
+    diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
+}
+
+void MisleadingIndentationCheck::missingBracesCheck(const SourceManager &SM,
+                                                    const CompoundStmt *CStmt) {
+  const static StringRef StmtNames[] = {"if", "for", "while"};
+  for (unsigned int i = 0; i < CStmt->size() - 1; i++) {
+    const Stmt *CurrentStmt = CStmt->body_begin()[i];
+    const Stmt *Inner = nullptr;
+    int StmtKind = 0;
+
+    if (const auto *CurrentIf = dyn_cast<IfStmt>(CurrentStmt)) {
+      StmtKind = 0;
+      Inner =
+          CurrentIf->getElse() ? CurrentIf->getElse() : CurrentIf->getThen();
+    } else if (const auto *CurrentFor = dyn_cast<ForStmt>(CurrentStmt)) {
+      StmtKind = 1;
+      Inner = CurrentFor->getBody();
+    } else if (const auto *CurrentWhile = dyn_cast<WhileStmt>(CurrentStmt)) {
+      StmtKind = 2;
+      Inner = CurrentWhile->getBody();
+    } else {
+      continue;
+    }
+
+    if (isa<CompoundStmt>(Inner))
+      continue;
+
+    SourceLocation InnerLoc = Inner->getLocStart();
+    SourceLocation OuterLoc = CurrentStmt->getLocStart();
+
+    if (SM.getExpansionLineNumber(InnerLoc) ==
+        SM.getExpansionLineNumber(OuterLoc))
+      continue;
+
+    const Stmt *NextStmt = CStmt->body_begin()[i + 1];
+    SourceLocation NextLoc = NextStmt->getLocStart();
+
+    if (InnerLoc.isMacroID() || OuterLoc.isMacroID() || NextLoc.isMacroID())
+      continue;
+
+    if (SM.getExpansionColumnNumber(InnerLoc) ==
+        SM.getExpansionColumnNumber(NextLoc)) {
+      diag(NextLoc, "misleading indentation: statement is indented too deeply");
+      diag(OuterLoc, "did you mean this line to be inside this '%0'",
+           DiagnosticIDs::Note)
+          << StmtNames[StmtKind];
+    }
+  }
+}
+
+void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this);
+  Finder->addMatcher(
+      compoundStmt(has(stmt(anyOf(ifStmt(), forStmt(), whileStmt()))))
+          .bind("compound"),
+      this);
+}
+
+void MisleadingIndentationCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *If = Result.Nodes.getNodeAs<IfStmt>("if"))
+    danglingElseCheck(*Result.SourceManager, If);
+
+  if (const auto *CStmt = Result.Nodes.getNodeAs<CompoundStmt>("compound"))
+    missingBracesCheck(*Result.SourceManager, CStmt);
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.h

@@ -0,0 +1,41 @@
+//===--- MisleadingIndentationCheck.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_MISLEADING_INDENTATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Checks the code for dangling else, and possible misleading indentations due
+/// to missing braces. Note that this check only works as expected when the tabs
+/// or spaces are used consistently and not mixed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html
+class MisleadingIndentationCheck : public ClangTidyCheck {
+public:
+  MisleadingIndentationCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
+  void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

@@ -20,6 +20,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
@@ -61,6 +62,8 @@ class ReadabilityModule : public ClangTidyModule {
         "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    CheckFactories.registerCheck<MisleadingIndentationCheck>(
+        "readability-misleading-indentation");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -57,6 +57,11 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- New `readability-misleading-indentation
+  <http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html>`_ check
+
+  Finds misleading indentation where braces should be introduced or the code should be reformatted.
+
 - New `safety-no-assembler
   <http://clang.llvm.org/extra/clang-tidy/checks/safety-no-assembler.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -139,6 +139,7 @@ Clang-Tidy Checks
    readability-identifier-naming
    readability-implicit-bool-cast
    readability-inconsistent-declaration-parameter-name
+   readability-misleading-indentation
    readability-misplaced-array-index
    readability-named-parameter
    readability-non-const-parameter

clang-tools-extra/docs/clang-tidy/checks/readability-misleading-indentation.rst

@@ -0,0 +1,38 @@
+.. title:: clang-tidy - readability-misleading-indentation
+
+readability-misleading-indentation
+==================================
+
+Correct indentation helps to understand code. Mismatch of the syntactical
+structure and the indentation of the code may hide serious problems.
+Missing braces can also make it significantly harder to read the code,
+therefore it is important to use braces. 
+
+The way to avoid dangling else is to always check that an ``else`` belongs
+to the ``if`` that begins in the same column.
+
+You can omit braces when your inner part of e.g. an ``if`` statement has only
+one statement in it. Although in that case you should begin the next statement
+in the same column with the ``if``.
+
+Examples:
+
+.. code-block:: c++
+
+  // Dangling else:
+  if (cond1)
+    if (cond2)
+      foo1();
+  else
+    foo2();  // Wrong indentation: else belongs to if(cond2) statement.
+
+  // Missing braces:
+  if (cond1)
+    foo1();
+    foo2();  // Not guarded by if(cond1).
+
+Limitations
+===========
+
+Note that this check only works as expected when the tabs or spaces are used
+consistently and not mixed.

clang-tools-extra/test/clang-tidy/readability-misleading-indentation.cpp

@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s readability-misleading-indentation %t
+
+void foo1();
+void foo2();
+
+#define BLOCK \
+  if (cond1)  \
+    foo1();   \
+    foo2();
+
+int main()
+{
+  bool cond1 = true;
+  bool cond2 = true;
+
+  if (cond1)
+    if (cond2)
+      foo1();
+  else
+    foo2();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+
+  if (cond1) {
+    if (cond2)
+      foo1();
+  }
+  else
+    foo2();
+
+  if (cond1)
+    if (cond2)
+      foo1();
+    else
+      foo2();
+
+  if (cond2)
+    foo1();
+    foo2();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: misleading indentation: statement is indented too deeply [readability-misleading-indentation]
+    // CHECK-MESSAGES: :[[@LINE-4]]:3: note: did you mean this line to be inside this 'if'
+    foo2(); // No redundant warning.
+
+  if (cond1)
+  {
+    foo1();
+  }
+    foo2();
+
+  if (cond1)
+    foo1();
+  foo2();
+
+  if (cond2)
+    if (cond1) foo1(); else foo2();
+
+  if (cond1) {
+  } else {
+  }
+
+  if (cond1) {
+  }
+  else {
+  }
+
+  if (cond1)
+  {
+  }
+  else
+  {
+  }
+
+  if (cond1)
+    {
+    }
+  else
+    {
+    }
+
+  BLOCK
+}