diff --git a/clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.h b/clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.h new file mode 100644 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp b/clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp new file mode 100644 --- /dev/null +++ b/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("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 diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -4,6 +4,7 @@ ) add_clang_library(clangTidyMiscModule + AssignmentInIfClauseCheck.cpp DefinitionsInHeadersCheck.cpp MiscTidyModule.cpp MisleadingBidirectional.cpp diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/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( + "misc-assignment-in-if-clause"); CheckFactories.registerCheck( "misc-definitions-in-headers"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -123,6 +123,11 @@ Warns when the code is unwrapping a `std::optional`, `absl::optional`, or `base::Optional` object without assuring that it contains a value. +- New :doc:`misc-assignment-in-if-clause + ` check. + + Warns when there is an assignment within an if statement condition expression. + - New :doc:`modernize-macro-to-enum ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -213,6 +213,7 @@ `llvmlibc-callee-namespace `_, `llvmlibc-implementation-in-namespace `_, `llvmlibc-restrict-system-libc-headers `_, "Yes" + `misc-assignment-in-if-clause `_, "Yes" `misc-definitions-in-headers `_, "Yes" `misc-misleading-bidirectional `_, `misc-misleading-identifier `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst new file mode 100644 --- /dev/null +++ b/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; + } + diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp new file mode 100644 --- /dev/null +++ b/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; + } +} + +