[clang-tools-extra] [clang-tidy] Add new check 'bugprone-inconsistent-ifelse-braces' (PR #162361)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 25 00:09:02 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra
Author: Davide Cunial (capitan-davide)
<details>
<summary>Changes</summary>
Closes https://github.com/llvm/llvm-project/issues/162140
---
Full diff: https://github.com/llvm/llvm-project/pull/162361.diff
9 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
- (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
- (added) clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp (+89)
- (added) clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h (+41)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst (+42)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+2-1)
- (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp (+55)
- (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp (+123)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index fe261e729539c..e1e42d22c520e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/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>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 46bc8efd44bc5..d19fd5017d2e0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.cpp
new file mode 100644
index 0000000000000..0c6b3e3e3ff9c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/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::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;
+
+ 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(
+ traverse(TK_IgnoreUnlessSpelledInSource,
+ 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 ast_matchers::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.
+ checkStmt(Result, If->getThen(), If->getRParenLoc(), If->getElseLoc());
+ if (shouldHaveBraces(NestedIf))
+ checkIfStmt(Result, NestedIf);
+ } else if (!isa<CompoundStmt>(Then)) {
+ checkStmt(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))
+ 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) {
+ DiagnosticBuilder Diag = diag(Hints.DiagnosticPos, "<message>");
+ if (Hints.offersFixIts()) {
+ Diag << Hints.openingBraceFixIt() << Hints.closingBraceFixIt();
+ }
+ }
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h b/clang-tools-extra/clang-tidy/bugprone/InconsistentIfelseBracesCheck.h
new file mode 100644
index 0000000000000..c818f46fea281
--- /dev/null
+++ b/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 {
+
+/// Detects `if`/`else` statements where one branch uses braces and the other
+/// does not.
+///
+/// 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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7cdff86beeec6..28bebceef1006 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -157,6 +157,12 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-inconsistent-ifelse-braces
+ <clang-tidy/checks/bugprone/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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst
new file mode 100644
index 0000000000000..44f2d64452393
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/inconsistent-ifelse-braces.rst
@@ -0,0 +1,42 @@
+.. title:: clang-tidy - bugprone-inconsistent-ifelse-braces
+
+bugprone-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;
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c490d2ece2e0a..4c5b70f6c47e5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/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>`, "Yes"
: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"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp
new file mode 100644
index 0000000000000..3eb51c39e2921
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces-consteval-if.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy -std=c++23-or-later %s bugprone-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> [bugprone-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> [bugprone-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> [bugprone-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");
+ }
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp
new file mode 100644
index 0000000000000..3d4af258137ce
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inconsistent-ifelse-braces.cpp
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s bugprone-inconsistent-ifelse-braces %t
+
+bool cond(const char *) { return false; }
+
+void do_something(const char *) {}
+
+// Positive tests.
+void f() {
+ if (cond("if0") /*comment*/) do_something("if-same-line");
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:31: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (cond("if0") /*comment*/) { do_something("if-same-line");
+ // CHECK-FIXES: } else {
+
+ if (cond("if0.1") /*comment*/) {
+ } else do_something("else-same-line");
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else { do_something("else-same-line");
+ // CHECK-FIXES: }
+
+ if (cond("if1"))
+ do_something("if-single-line");
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:19: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (cond("if1")) {
+ // CHECK-FIXES: } else {
+
+ if (cond("if1.1")) {
+ } else
+ do_something("else-single-line");
+ // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else {
+ // CHECK-FIXES: }
+
+ if (cond("if2") /*comment*/)
+ // some comment
+ do_something("if-multi-line");
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-5]]:31: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (cond("if2") /*comment*/) {
+ // CHECK-FIXES: } else {
+
+ if (cond("if2.1") /*comment*/) {
+ } else
+ // some comment
+ do_something("else-multi-line");
+ // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else {
+ // CHECK-FIXES: }
+
+ if (cond("if3")) do_something("elseif-same-line");
+ else if (cond("if3")) {
+ } else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:19: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (cond("if3")) { do_something("elseif-same-line");
+ // CHECK-FIXES: } else if (cond("if3")) {
+
+ if (cond("if3.1")) {
+ } else if (cond("if3.1")) do_something("elseif-same-line");
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:28: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else if (cond("if3.1")) { do_something("elseif-same-line");
+ // CHECK-FIXES: } else {
+
+ if (cond("if3.2")) {
+ } else if (cond("if3.2")) {
+ } else do_something("else-same-line");
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else { do_something("else-same-line");
+ // CHECK-FIXES: }
+
+ if (cond("if4-outer"))
+ if (cond("if4-inner"))
+ do_something("if-single-line");
+ else {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-7]]:25: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-MESSAGES: :[[@LINE-7]]:27: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (cond("if4-outer")) {
+ // CHECK-FIXES: if (cond("if4-inner")) {
+ // CHECK-FIXES: } else {
+ // CHECK-FIXES: } else {
+
+ if (cond("if5"))
+ do_something("if-single-line");
+ else if (cond("if5")) {
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:19: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: if (cond("if5")) {
+ // CHECK-FIXES: } else if (cond("if5")) {
+
+ if (cond("if5.1")) {
+ } else if (cond("if5.1"))
+ do_something("elseif-single-line");
+ // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: <message> [bugprone-inconsistent-ifelse-braces]
+ // CHECK-FIXES: } else if (cond("if5.1")) {
+ // CHECK-FIXES: }
+}
+
+// Negative tests.
+void g() {
+ if (cond("if0")) {
+ do_something("if-single-line");
+ } else if (cond("if0")) {
+ do_something("elseif-single-line");
+ } else {
+ do_something("else-single-line");
+ }
+
+ if (cond("if1"))
+ do_something("if-single-line");
+ else if (cond("if1"))
+ do_something("elseif-single-line");
+ else
+ do_something("else-single-line");
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/162361
More information about the cfe-commits
mailing list