[clang-tools-extra] 05130a6 - new clang-tidy checker for assignments within condition clause of if statement
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 5 14:04:19 PDT 2022
Author: Dmitri Gribenko
Date: 2022-07-05T23:04:12+02:00
New Revision: 05130a6ba7d9974136388c1fbe63125596325d2e
URL: https://github.com/llvm/llvm-project/commit/05130a6ba7d9974136388c1fbe63125596325d2e
DIFF: https://github.com/llvm/llvm-project/commit/05130a6ba7d9974136388c1fbe63125596325d2e.diff
LOG: new clang-tidy checker for assignments within condition clause of if statement
new clang-tidy checker for assignments within the condition clause of an 'if' statement.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D127114
Added:
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
new file mode 100644
index 0000000000000..e4f52191e0b00
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -0,0 +1,49 @@
+//===--- AssignmentInIfConditionCheck.cpp - clang-tidy --------------------===//
+//
+// 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 "AssignmentInIfConditionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void AssignmentInIfConditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(ifStmt(hasCondition(forEachDescendant(
+ binaryOperator(isAssignmentOperator())
+ .bind("assignment_in_if_statement")))),
+ this);
+ Finder->addMatcher(ifStmt(hasCondition(forEachDescendant(
+ cxxOperatorCallExpr(isAssignmentOperator())
+ .bind("assignment_in_if_statement")))),
+ this);
+}
+
+void AssignmentInIfConditionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<clang::Stmt>("assignment_in_if_statement");
+ if (!MatchedDecl) {
+ return;
+ }
+ diag(MatchedDecl->getBeginLoc(),
+ "an assignment within an 'if' condition is bug-prone");
+ diag(MatchedDecl->getBeginLoc(),
+ "if it should be an assignment, move it out of the 'if' condition",
+ DiagnosticIDs::Note);
+ diag(MatchedDecl->getBeginLoc(),
+ "if it is meant to be an equality check, change '=' to '=='",
+ DiagnosticIDs::Note);
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
new file mode 100644
index 0000000000000..8e57f1a7ca80d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
@@ -0,0 +1,34 @@
+//===--- AssignmentInIfConditionCheck.h - clang-tidy ------------*- C++ -*-===//
+//
+// 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_ASSIGNMENTINIFCONDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSIGNMENTINIFCONDITIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Catches assignments within the condition clause of an if statement.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-assignment-in-if-condition.html
+class AssignmentInIfConditionCheck : public ClangTidyCheck {
+public:
+ AssignmentInIfConditionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSIGNMENTINIFCONDITIONCHECK_H
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 2d8bfffc01268..961480a23f8f1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
+#include "AssignmentInIfConditionCheck.h"
#include "BadSignalToKillThreadCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "BranchCloneCheck.h"
@@ -84,12 +85,13 @@ class BugproneModule : public ClangTidyModule {
"bugprone-argument-comment");
CheckFactories.registerCheck<AssertSideEffectCheck>(
"bugprone-assert-side-effect");
+ CheckFactories.registerCheck<AssignmentInIfConditionCheck>(
+ "bugprone-assignment-in-if-condition");
CheckFactories.registerCheck<BadSignalToKillThreadCheck>(
"bugprone-bad-signal-to-kill-thread");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"bugprone-bool-pointer-implicit-conversion");
- CheckFactories.registerCheck<BranchCloneCheck>(
- "bugprone-branch-clone");
+ CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");
CheckFactories.registerCheck<CopyConstructorInitCheck>(
"bugprone-copy-constructor-init");
CheckFactories.registerCheck<DanglingHandleCheck>(
@@ -100,8 +102,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-easily-swappable-parameters");
CheckFactories.registerCheck<ExceptionEscapeCheck>(
"bugprone-exception-escape");
- CheckFactories.registerCheck<FoldInitTypeCheck>(
- "bugprone-fold-init-type");
+ CheckFactories.registerCheck<FoldInitTypeCheck>("bugprone-fold-init-type");
CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
"bugprone-forward-declaration-namespace");
CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
@@ -112,8 +113,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-inaccurate-erase");
CheckFactories.registerCheck<IncorrectRoundingsCheck>(
"bugprone-incorrect-roundings");
- CheckFactories.registerCheck<InfiniteLoopCheck>(
- "bugprone-infinite-loop");
+ CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop");
CheckFactories.registerCheck<IntegerDivisionCheck>(
"bugprone-integer-division");
CheckFactories.registerCheck<LambdaFunctionNameCheck>(
@@ -141,8 +141,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-not-null-terminated-result");
CheckFactories.registerCheck<ParentVirtualCallCheck>(
"bugprone-parent-virtual-call");
- CheckFactories.registerCheck<PosixReturnCheck>(
- "bugprone-posix-return");
+ CheckFactories.registerCheck<PosixReturnCheck>("bugprone-posix-return");
CheckFactories.registerCheck<ReservedIdentifierCheck>(
"bugprone-reserved-identifier");
CheckFactories.registerCheck<SharedPtrArrayMismatchCheck>(
@@ -196,12 +195,10 @@ class BugproneModule : public ClangTidyModule {
"bugprone-unhandled-self-assignment");
CheckFactories.registerCheck<UnhandledExceptionAtNewCheck>(
"bugprone-unhandled-exception-at-new");
- CheckFactories.registerCheck<UnusedRaiiCheck>(
- "bugprone-unused-raii");
+ CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii");
CheckFactories.registerCheck<UnusedReturnValueCheck>(
"bugprone-unused-return-value");
- CheckFactories.registerCheck<UseAfterMoveCheck>(
- "bugprone-use-after-move");
+ CheckFactories.registerCheck<UseAfterMoveCheck>("bugprone-use-after-move");
CheckFactories.registerCheck<VirtualNearMissCheck>(
"bugprone-virtual-near-miss");
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 50f6862bc2f44..1c7b0c2519f77 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyBugproneModule
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
+ AssignmentInIfConditionCheck.cpp
BadSignalToKillThreadCheck.cpp
BoolPointerImplicitConversionCheck.cpp
BranchCloneCheck.cpp
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e6d9a57dccea6..ff444a0934f6f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,6 +132,11 @@ New checks
Detects confusable Unicode identifiers.
+- New :doc:`bugprone-assignment-in-if-condition
+ <clang-tidy/checks/bugprone-assignment-in-if-condition>` check.
+
+ Warns when there is an assignment within an if statement condition expression.
+
- New :doc:`modernize-macro-to-enum
<clang-tidy/checks/modernize/macro-to-enum>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
new file mode 100644
index 0000000000000..9fa37c0593815
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - bugprone-assignment-in-if-condition
+
+bugprone-assignment-in-if-condition
+===================================
+
+Finds assignments within conditions of `if` statements.
+Such assignments are bug-prone because they may have been intended as equality tests.
+
+This check finds all assignments within `if` conditions, including ones that are not flagged
+by `-Wparentheses` due to an extra set of parentheses, and including assignments that call
+an overloaded `operator=()`. The identified assignments violate
+`BARR group "Rule 8.2.c" <https://barrgroup.com/embedded-systems/books/embedded-c-coding-standard/statement-rules/if-else-statements>`_.
+
+.. code-block:: c++
+
+ int f = 3;
+ if(f = 4) { // This is identified by both `Wparentheses` and this check - should it have been: `if (f == 4)` ?
+ f = f + 1;
+ }
+
+ if((f == 5) || (f = 6)) { // the assignment here `(f = 6)` is identified by this check, but not by `-Wparentheses`. Should it have been `(f == 6)` ?
+ f = f + 2;
+ }
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index d967a9b059bf5..92585540651d0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@ Clang-Tidy Checks
`boost-use-to-string <boost/use-to-string.html>`_, "Yes"
`bugprone-argument-comment <bugprone/argument-comment.html>`_, "Yes"
`bugprone-assert-side-effect <bugprone/assert-side-effect.html>`_,
+ `bugprone-assignment-in-if-condition <bugprone/assignment-in-if-condition.html>`_,
`bugprone-bad-signal-to-kill-thread <bugprone/bad-signal-to-kill-thread.html>`_,
`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion.html>`_, "Yes"
`bugprone-branch-clone <bugprone/branch-clone.html>`_,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
new file mode 100644
index 0000000000000..49e52edb3f3d7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-assignment-in-if-condition %t
+
+void f(int arg) {
+ int f = 3;
+ if ((f = arg) || (f == (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 5;
+ }
+}
+
+void f1(int arg) {
+ int f = 3;
+ if ((f == arg) || (f = (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 5;
+ }
+}
+
+void f2(int arg) {
+ int f = 3;
+ if (f = arg)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 5;
+ }
+}
+
+volatile int v = 32;
+
+void f3(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && (f = v)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 5;
+ }
+}
+
+void f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f = v) || (f < 8))))
+ // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 5;
+ } else if ((arg + 8 < f) && ((f = v) || (f < 8)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 6;
+ }
+}
+
+class BrokenOperator {
+public:
+ int d = 0;
+ int operator=(const int &val) {
+ d = val + 1;
+ return d;
+ }
+};
+
+void f5(int arg) {
+ BrokenOperator bo;
+ int f = 3;
+ bo = f;
+ if (bo.d == 3) {
+ f = 6;
+ }
+ if (bo = 3)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 7;
+ }
+ if ((arg == 3) || (bo = 6))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+ f = 8;
+ }
+ v = f;
+}
+
+// Tests that shouldn't trigger warnings.
+void awesome_f2(int arg) {
+ int f = 3;
+ if ((f == arg) || (f == (arg + 1))) {
+ f = 5;
+ }
+}
+
+void awesome_f3(int arg) {
+ int f = 3;
+ if (f == arg) {
+ f = 5;
+ }
+}
+
+void awesome_f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f == v) || (f < 8)))) {
+ f = 5;
+ }
+}
More information about the cfe-commits
mailing list