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

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

@@ -24,6 +24,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
   IdentifierLengthCheck.cpp
   IdentifierNamingCheck.cpp
   ImplicitBoolConversionCheck.cpp
+  InconsistentIfElseBracesCheck.cpp
   RedundantInlineSpecifierCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   IsolateDeclarationCheck.cpp

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

@@ -0,0 +1,89 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+/// 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;
+
+  if (const Stmt *const Else = If->getElse()) {
+    if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
+      return shouldHaveBraces(NestedIf);
+
+    return isa<CompoundStmt>(Else);
+  }
+
+  return false;
+}
+
+void InconsistentIfElseBracesCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      ifStmt(hasElse(anything()),
+             unless(isConsteval()), // 'if consteval' always has braces
+             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 MatchFinder::MatchResult &Result, const IfStmt *If) {
+  const Stmt *Then = If->getThen();
+  if (const auto *NestedIf = dyn_cast<const IfStmt>(Then)) {
+    // If the then-branch is a nested IfStmt, first we need to add braces to
+    // it, then we need to check the inner IfStmt.
+    emitDiagnostic(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
+    if (shouldHaveBraces(NestedIf))
+      checkIfStmt(Result, NestedIf);
+  } else if (!isa<CompoundStmt>(Then)) {
+    emitDiagnostic(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
+  }
+
+  if (const Stmt *const Else = If->getElse()) {
+    if (const auto *NestedIf = dyn_cast<const IfStmt>(Else))
+      checkIfStmt(Result, NestedIf);
+    else if (!isa<CompoundStmt>(Else))
+      emitDiagnostic(Result, If->getElse(), If->getElseLoc());
+  }
+}
+
+void InconsistentIfElseBracesCheck::emitDiagnostic(
+    const MatchFinder::MatchResult &Result, const Stmt *S,
+    SourceLocation StartLoc, SourceLocation EndLocHint) {
+  const SourceManager &SM = *Result.SourceManager;
+  const LangOptions &LangOpts = Result.Context->getLangOpts();
+
+  if (!StartLoc.isMacroID()) {
+    const utils::BraceInsertionHints Hints =
+        utils::getBraceInsertionsHints(S, LangOpts, SM, StartLoc, EndLocHint);
+    assert(Hints && Hints.offersFixIts() && "Expected hints or fix-its");
+    diag(Hints.DiagnosticPos, "<message>")
+        << Hints.openingBraceFixIt() << Hints.closingBraceFixIt();
+  } else {
+    diag(StartLoc, "<message-for-macro-expansions>") << StartLoc.isMacroID();
+  }
+}
+
+} // namespace clang::tidy::readability

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

@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_READABILITY_INCONSISTENTIFELSEBRACESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENTIFELSEBRACESCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include <optional>
+
+namespace clang::tidy::readability {
+
+/// Detects `if`/`else` statements where one branch uses braces and the other
+/// does not.
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/readability/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;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+private:
+  void checkIfStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+                   const IfStmt *If);
+  void emitDiagnostic(const ast_matchers::MatchFinder::MatchResult &Result,
+                      const Stmt *S, SourceLocation StartLoc,
+                      SourceLocation EndLocHint = {});
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENTIFELSEBRACESCHECK_H

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

@@ -30,6 +30,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolConversionCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "InconsistentIfElseBracesCheck.h"
 #include "IsolateDeclarationCheck.h"
 #include "MagicNumbersCheck.h"
 #include "MakeMemberFunctionConstCheck.h"
@@ -110,6 +111,8 @@ class ReadabilityModule : public ClangTidyModule {
         "readability-identifier-naming");
     CheckFactories.registerCheck<ImplicitBoolConversionCheck>(
         "readability-implicit-bool-conversion");
+    CheckFactories.registerCheck<InconsistentIfElseBracesCheck>(
+        "readability-inconsistent-ifelse-braces");
     CheckFactories.registerCheck<MathMissingParenthesesCheck>(
         "readability-math-missing-parentheses");
     CheckFactories.registerCheck<RedundantInlineSpecifierCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -157,6 +157,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`readability-inconsistent-ifelse-braces
+  <clang-tidy/checks/readability/inconsistent-ifelse-braces>` check.
+
+  Detects ``if``/``else`` statements where one branch uses braces and the other
+  does not.
+
 - New :doc:`bugprone-invalid-enum-default-initialization
   <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
 

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

@@ -386,6 +386,7 @@ Clang-Tidy Checks
    :doc:`readability-identifier-naming <readability/identifier-naming>`, "Yes"
    :doc:`readability-implicit-bool-conversion <readability/implicit-bool-conversion>`, "Yes"
    :doc:`readability-inconsistent-declaration-parameter-name <readability/inconsistent-declaration-parameter-name>`, "Yes"
+   :doc:`readability-inconsistent-ifelse-braces <readability/inconsistent-ifelse-braces>`, "Yes"
    :doc:`readability-isolate-declaration <readability/isolate-declaration>`, "Yes"
    :doc:`readability-magic-numbers <readability/magic-numbers>`,
    :doc:`readability-make-member-function-const <readability/make-member-function-const>`, "Yes"

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

@@ -0,0 +1,42 @@
+.. title:: clang-tidy - readability-inconsistent-ifelse-braces
+
+readability-inconsistent-ifelse-braces
+======================================
+
+Detects ``if``/``else`` statements where one branch uses braces and the other
+does not.
+
+Before:
+
+.. code-block:: c++
+
+  if (condition) {
+    statement;
+  } else
+    statement;
+
+  if (condition)
+    statement;
+
+  if (condition)
+    statement;
+  else
+    statement;
+
+After:
+
+.. code-block:: c++
+
+  if (condition) {
+    statement;
+  } else {
+    statement;
+  }
+
+  if (condition)
+    statement;
+
+  if (condition)
+    statement;
+  else
+    statement;

clang-tools-extra/test/clang-tidy/checkers/readability/inconsistent-ifelse-braces-consteval-if.cpp

@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy -std=c++23-or-later %s readability-inconsistent-ifelse-braces %t
+
+bool cond(const char *) { return false; }
+void do_something(const char *) {}
+
+// Positive tests.
+void f() {
+  if consteval {
+    if (cond("if1"))
+      do_something("if-single-line");
+    else {
+    }
+    // CHECK-MESSAGES: :[[@LINE-4]]:21: warning: <message> [readability-inconsistent-ifelse-braces]
+    // CHECK-FIXES: if (cond("if1")) {
+    // CHECK-FIXES: } else {
+  }
+
+  if consteval {
+    if (cond("if2"))
+      do_something("if-single-line");
+    else {
+    }
+    // CHECK-MESSAGES: :[[@LINE-4]]:21: warning: <message> [readability-inconsistent-ifelse-braces]
+    // CHECK-FIXES: if (cond("if2")) {
+    // CHECK-FIXES: } else {
+  } else {
+    if (cond("if2.1")) {
+    } else
+      do_something("else-single-line");
+    // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: <message> [readability-inconsistent-ifelse-braces]
+    // CHECK-FIXES: } else {
+    // CHECK-FIXES: }
+  }
+}
+
+// Negative tests.
+void g() {
+  if consteval {
+    if (cond("if0")) {
+      do_something("if-single-line");
+    } else if (cond("if0")) {
+      do_something("elseif-single-line");
+    } else {
+      do_something("else-single-line");
+    }
+  } else {
+    if (cond("if0.1"))
+      do_something("if-single-line");
+    else if (cond("if0.1"))
+      do_something("elseif-single-line");
+    else
+      do_something("else-single-line");
+  }
+}