[clang-tidy] Add new check 'bugprone-inconsistent-ifelse-braces'

Author: capitan-davide

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -33,6 +33,7 @@
 #include "ImplicitWideningOfMultiplicationResultCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncDecInConditionsCheck.h"
+#include "InconsistentIfelseBracesCheck.h"
 #include "IncorrectEnableIfCheck.h"
 #include "IncorrectEnableSharedFromThisCheck.h"
 #include "IncorrectRoundingsCheck.h"
@@ -150,6 +151,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-implicit-widening-of-multiplication-result");
     CheckFactories.registerCheck<InaccurateEraseCheck>(
         "bugprone-inaccurate-erase");
+    CheckFactories.registerCheck<InconsistentIfelseBracesCheck>(
+        "bugprone-inconsistent-ifelse-braces");
     CheckFactories.registerCheck<IncorrectEnableIfCheck>(
         "bugprone-incorrect-enable-if");
     CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(

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

@@ -28,6 +28,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   ForwardingReferenceOverloadCheck.cpp
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
+  InconsistentIfelseBracesCheck.cpp
   IncorrectEnableIfCheck.cpp
   IncorrectEnableSharedFromThisCheck.cpp
   InvalidEnumDefaultInitializationCheck.cpp

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp

@@ -0,0 +1,86 @@
+
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "InconsistentIfelseBracesCheck.h"
+#include "../utils/BracesAroundStatement.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+/// Check that at least one branch of the \p If statement is a \c CompoundStmt.
+static bool shouldHaveBraces(const IfStmt *If) {
+  const Stmt *const Then = If->getThen();
+  if (isa<CompoundStmt>(Then))
+    return true;
+
+  const Stmt *const Else = If->getElse();
+  if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
+    return shouldHaveBraces(NestedIf);
+
+  return isa<CompoundStmt>(Else);
+}
+
+void InconsistentIfelseBracesCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      traverse(TK_IgnoreUnlessSpelledInSource,
+               ifStmt(hasElse(anything()), unless(hasParent(ifStmt())))
+                   .bind("if_stmt")),
+      this);
+}
+
+void InconsistentIfelseBracesCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedIf = Result.Nodes.getNodeAs<IfStmt>("if_stmt");
+  if (!shouldHaveBraces(MatchedIf))
+    return;
+
+  checkIfStmt(Result, MatchedIf);
+}
+
+void InconsistentIfelseBracesCheck::checkIfStmt(
+    const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If) {
+  const Stmt *Then = If->getThen();
+  if (!isa<CompoundStmt>(Then))
+    checkStmt(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
+
+  const Stmt *const Else = If->getElse();
+  if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
+    return checkIfStmt(Result, NestedIf);
+
+  if (!isa<CompoundStmt>(Else))
+    checkStmt(Result, If->getElse(), If->getElseLoc());
+}
+
+void InconsistentIfelseBracesCheck::checkStmt(
+    const ast_matchers::MatchFinder::MatchResult &Result, const Stmt *S,
+    SourceLocation StartLoc, SourceLocation EndLocHint) {
+  const SourceManager &SM = *Result.SourceManager;
+  const LangOptions &LangOpts = Result.Context->getLangOpts();
+
+  const utils::BraceInsertionHints Hints =
+      utils::getBraceInsertionsHints(S, LangOpts, SM, StartLoc, EndLocHint);
+  if (Hints) {
+    auto Diag = diag(Hints.DiagnosticPos, "braces here!");
+    if (Hints.offersFixIts()) {
+      Diag << Hints.openingBraceFixIt() << Hints.closingBraceFixIt();
+    }
+  }
+}
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h

@@ -0,0 +1,41 @@
+
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCONSISTENTIFELSEBRACESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCONSISTENTIFELSEBRACESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.html
+class InconsistentIfelseBracesCheck : public ClangTidyCheck {
+public:
+  InconsistentIfelseBracesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+
+private:
+  void checkIfStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+                   const IfStmt *If);
+  void checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+                 const Stmt *S, SourceLocation StartLoc,
+                 SourceLocation EndLocHint = SourceLocation());
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCONSISTENTIFELSEBRACESCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -157,6 +157,11 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-inconsistent-ifelse-braces
+  <clang-tidy/checks/bugprone/inconsistent-ifelse-braces>` check.
+
+  FIXME: Write a short description.
+
 - New :doc:`bugprone-invalid-enum-default-initialization
   <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst

@@ -0,0 +1,6 @@
+.. title:: clang-tidy - bugprone-inconsistent-ifelse-braces
+
+bugprone-inconsistent-ifelse-braces
+===================================
+
+FIXME: Describe what patterns does the check detect and why. Give examples.

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

@@ -101,6 +101,7 @@ Clang-Tidy Checks
    :doc:`bugprone-implicit-widening-of-multiplication-result <bugprone/implicit-widening-of-multiplication-result>`, "Yes"
    :doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
    :doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,
+   :doc:`bugprone-inconsistent-ifelse-braces <bugprone/inconsistent-ifelse-braces>`,
    :doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes"
    :doc:`bugprone-incorrect-enable-shared-from-this <bugprone/incorrect-enable-shared-from-this>`, "Yes"
    :doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`,
@@ -256,7 +257,7 @@ Clang-Tidy Checks
    :doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`,
    :doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
    :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
-   :doc:`llvm-use-ranges <llvm/use-ranges>`, "Yes"
+   :doc:`llvm-use-ranges <llvm/use-ranges>`,
    :doc:`llvmlibc-callee-namespace <llvmlibc/callee-namespace>`,
    :doc:`llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace>`,
    :doc:`llvmlibc-inline-function-decl <llvmlibc/inline-function-decl>`, "Yes"

clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp

@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s bugprone-inconsistent-ifelse-braces %t
+
+void f(bool flag) {
+  int x, y;
+
+  if (flag)
+    x = 0;
+  else if (flag)
+    y = 1;
+  else {
+    y = 2;
+  }
+
+  if (flag) {
+    x = 0;
+  } else
+    y = 1;
+
+  if (flag) {
+    x = 0;
+  } else
+    ;
+  
+  if (flag)
+    x = 0;
+  else
+    y = 1;
+}
+
+// FIXME: Add something that triggers the check here.
+// void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [bugprone-inconsistent-ifelse-braces]
+
+// FIXME: Verify the applied fix.
+//   * Make the CHECK patterns specific enough and try to make verified lines
+//     unique to avoid incorrect matches.
+//   * Use {{}} for regular expressions.
+// CHECK-FIXES: {{^}}void awesome_f();{{$}}
+
+// FIXME: Add something that doesn't trigger the check here.
+void awesome_f2();