[clang-tools-extra] 6bd98b4 - [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-08-27T19:55:08+01:00
New Revision: 6bd98b4f2df02955b72343038ecf20cdfd23e01e

URL: https://github.com/llvm/llvm-project/commit/6bd98b4f2df02955b72343038ecf20cdfd23e01e
DIFF: https://github.com/llvm/llvm-project/commit/6bd98b4f2df02955b72343038ecf20cdfd23e01e.diff

LOG: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

Fixed a false positive where a lambda expression in the condition which contained an assignement would trigger a warning.
Fixes #56729

Reviewed By: gribozavr2

Differential Revision: https://reviews.llvm.org/D132786

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
    clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
index e4f52191e0b00..590505c1f47ba 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -17,23 +17,50 @@ 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);
+  Finder->addMatcher(translationUnitDecl(), this);
 }
 
 void AssignmentInIfConditionCheck::check(
-    const MatchFinder::MatchResult &Result) {
-  const auto *MatchedDecl =
-      Result.Nodes.getNodeAs<clang::Stmt>("assignment_in_if_statement");
-  if (!MatchedDecl) {
-    return;
-  }
+    const ast_matchers::MatchFinder::MatchResult &Result) {
+  class Visitor : public RecursiveASTVisitor<Visitor> {
+    AssignmentInIfConditionCheck &Check;
+
+  public:
+    explicit Visitor(AssignmentInIfConditionCheck &Check) : Check(Check) {}
+    bool VisitIfStmt(IfStmt *If) {
+      class ConditionVisitor : public RecursiveASTVisitor<ConditionVisitor> {
+        AssignmentInIfConditionCheck &Check;
+
+      public:
+        explicit ConditionVisitor(AssignmentInIfConditionCheck &Check)
+            : Check(Check) {}
+
+        // Dont traverse into any lambda expressions.
+        bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+          return true;
+        }
+
+        bool VisitBinaryOperator(BinaryOperator *BO) {
+          if (BO->isAssignmentOp())
+            Check.report(BO);
+          return true;
+        }
+
+        bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *OCE) {
+          if (OCE->isAssignmentOp())
+            Check.report(OCE);
+          return true;
+        }
+      };
+
+      ConditionVisitor(Check).TraverseStmt(If->getCond());
+      return true;
+    }
+  };
+  Visitor(*this).TraverseAST(*Result.Context);
+}
+
+void AssignmentInIfConditionCheck::report(const Expr *MatchedDecl) {
   diag(MatchedDecl->getBeginLoc(),
        "an assignment within an 'if' condition is bug-prone");
   diag(MatchedDecl->getBeginLoc(),

diff  --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
index 8e57f1a7ca80d..f84ae93ed2ebf 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
@@ -25,6 +25,7 @@ class AssignmentInIfConditionCheck : public ClangTidyCheck {
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void report(const Expr *E);
 };
 
 } // namespace bugprone

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1656debda1368..1734f4acf6865 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -116,6 +116,10 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Fixed a false positive in :doc:`bugprone-assignment-in-if-condition
+  <clang-tidy/checks/bugprone/assignment-in-if-condition>` check when there
+  was an assignement in a lambda found in the condition of an ``if``.
+
 - Improved :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was

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
index 49e52edb3f3d7..20c30d73516b3 100644
--- 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
@@ -101,3 +101,20 @@ void awesome_f4(int arg) {
     f = 5;
   }
 }
+
+template <typename Func> bool exec(Func F) { return F(); }
+
+void lambda_if() {
+  int X;
+  if ([&X] {
+        X = 5;
+        return true;
+      }()) {
+  }
+
+  if (exec([&] {
+        X = 5;
+        return true;
+      })) {
+  }
+}