Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -11,6 +11,7 @@ IdentifierNamingCheck.cpp ImplicitBoolCastCheck.cpp InconsistentDeclarationParameterNameCheck.cpp + MisleadingIndentationCheck.cpp MisplacedArrayIndexCheck.cpp NamedParameterCheck.cpp NamespaceCommentCheck.cpp Index: clang-tidy/readability/MisleadingIndentationCheck.h =================================================================== --- /dev/null +++ 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 Index: clang-tidy/readability/MisleadingIndentationCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/MisleadingIndentationCheck.cpp @@ -0,0 +1,94 @@ +//===--- 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, "potential dangling 'else'"); +} + +void MisleadingIndentationCheck::missingBracesCheck(const SourceManager &SM, + const CompoundStmt *CStmt) { + for (unsigned int i = 0; i < CStmt->size() - 1; i++) { + const Stmt *CurrentStmt = CStmt->body_begin()[i]; + const Stmt *Inside = nullptr; + + if (const auto *CurrentIf = dyn_cast(CurrentStmt)) { + Inside = + CurrentIf->getElse() ? CurrentIf->getElse() : CurrentIf->getThen(); + } else if (const auto *CurrentFor = dyn_cast(CurrentStmt)) { + Inside = CurrentFor->getBody(); + } else if (const auto CurrentWhile = dyn_cast(CurrentStmt)) { + Inside = CurrentWhile->getBody(); + } else { + continue; + } + + if (isa(Inside)) + continue; + + SourceLocation InnerLoc = Inside->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"); + } +} + +void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(ifStmt(hasElse(stmt())).bind("if"), this); + Finder->addMatcher( + compoundStmt(anyOf(has(ifStmt()), has(forStmt()), has(whileStmt()))) + .bind("compound"), + this); +} + +void MisleadingIndentationCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *If = Result.Nodes.getNodeAs("if")) + danglingElseCheck(*Result.SourceManager, If); + + if (const auto *CStmt = Result.Nodes.getNodeAs("compound")) + missingBracesCheck(*Result.SourceManager, CStmt); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ 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 @@ "readability-implicit-bool-cast"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-misleading-indentation"); CheckFactories.registerCheck( "readability-misplaced-array-index"); CheckFactories.registerCheck( Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,11 @@ Improvements to clang-tidy -------------------------- +- New `readability-misleading-indentation + `_ check + + Finds misleading indentation where braces should be introduced or the code should be reformatted. + - New `safety-no-assembler `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -139,6 +139,7 @@ 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 Index: docs/clang-tidy/checks/readability-misleading-indentation.rst =================================================================== --- /dev/null +++ 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. Index: test/clang-tidy/readability-misleading-indentation.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-misleading-indentation.cpp @@ -0,0 +1,79 @@ +// 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: potential dangling '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] + 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 +}