dodohand created this revision.
Herald added subscribers: carlosgalvezp, mgorny.
Herald added a project: All.
dodohand requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
new clang-tidy checker for assignments with the condition clause of an 'if'
statement.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127114
Files:
clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp
clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.h
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst
clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s misc-assignment-in-if-clause %t
+
+// Add something that triggers the check here.
+void f(int arg)
+{
+ int f = 3;
+ if(( f = arg ) || ( f == (arg + 1)))
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ f = 5;
+ }
+}
+
+void f1(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || ( f = (arg + 1)))
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ f = 5;
+ }
+}
+
+void f2(int arg)
+{
+ int f = 3;
+ if( f = arg )
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ 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]]:44: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ f = 5;
+ }
+}
+
+void f4(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || (( arg + 6 < f ) && (( f = v ) || (f < 8))))
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ f = 5;
+ }
+ else if(( arg + 8 < f ) && ((f = v) || (f < 8)))
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ 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]]:8: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ f = 7;
+ }
+ if((arg == 3) || (bo = 6))
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+ f = 8;
+ }
+ v = f;
+}
+
+// Add something that doesn't trigger the check here.
+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;
+ }
+}
+
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - misc-assignment-in-if-clause
+
+misc-assignment-in-if-clause
+============================
+
+Finds assignments within the condition of `if` statements.
+Allows automatic identification of assignments which may have been intended as equality tests.
+Finds these assignments even within multiple sets of parentheses which is often appropriate to structure multi-part condition statements.
+Finds these assignments even within multiple sets of paretheses which disables the compiler -Wparentheses check which one would otherwise like to rely on to find accidental assignment.
+The identified assignments violate BARR group "Rule 8.2.c". See: https://barrgroup.com/embedded-systems/books/embedded-c-coding-standard/statement-rules/if-else-statements
+
+Also finds these assignments when they're actually calling an overridden `=` operator rather than a plain-old assign.
+
+
+.. code-block:: c++
+
+ static volatile int f = 3;
+ if( f = 4 ) // This is identified - should it have been: `if ( f == 4 )` ?
+ {
+ f = f + 1;
+ }
+
+ if(( f == 5 ) || ( f = 6 )) // the assignment here `( f = 6 )` is identified - should it have been `( f == 6 )' ?
+ {
+ f = f + 2;
+ }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -213,6 +213,7 @@
`llvmlibc-callee-namespace <llvmlibc-callee-namespace.html>`_,
`llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
`llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
+ `misc-assignment-in-if-clause <misc-assignment-in-if-clause.html>`_, "Yes"
`misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
`misc-misleading-bidirectional <misc-misleading-bidirectional.html>`_,
`misc-misleading-identifier <misc-misleading-identifier.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,6 +123,11 @@
Warns when the code is unwrapping a `std::optional<T>`, `absl::optional<T>`,
or `base::Optional<T>` object without assuring that it contains a value.
+- New :doc:`misc-assignment-in-if-clause
+ <clang-tidy/checks/misc-assignment-in-if-clause>` 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.
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "AssignmentInIfClauseCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "MisleadingBidirectional.h"
#include "MisleadingIdentifier.h"
@@ -33,6 +34,8 @@
class MiscModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<AssignmentInIfClauseCheck>(
+ "misc-assignment-in-if-clause");
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
CheckFactories.registerCheck<MisleadingBidirectionalCheck>(
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
)
add_clang_library(clangTidyMiscModule
+ AssignmentInIfClauseCheck.cpp
DefinitionsInHeadersCheck.cpp
MiscTidyModule.cpp
MisleadingBidirectional.cpp
Index: clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.h
@@ -0,0 +1,34 @@
+//===--- AssignmentInIfClauseCheck.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_MISC_ASSIGNMENTINIFCLAUSECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSIGNMENTINIFCLAUSECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Catches assignments within the condition clause of an if statement.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-assignment-in-if-clause.html
+class AssignmentInIfClauseCheck : public ClangTidyCheck {
+public:
+ AssignmentInIfClauseCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSIGNMENTINIFCLAUSECHECK_H
Index: clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp
@@ -0,0 +1,45 @@
+//===--- AssignmentInIfClauseCheck.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 "AssignmentInIfClauseCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void AssignmentInIfClauseCheck::registerMatchers(MatchFinder *Finder) {
+ // Scott Added this one
+ 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 AssignmentInIfClauseCheck::check(const MatchFinder::MatchResult &Result) {
+ // Add callback implementation.
+ const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<clang::Stmt>("assignment_in_if_statement");
+ if (!MatchedDecl) {
+ return;
+ }
+ diag(MatchedDecl->getBeginLoc(),
+ "Assignment detected within if statement. Fix to equality check if this "
+ "was accidental. Consider moving out of if statement if intentional.");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
