[clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition

[njames93]

Summary

Currently the diagnostic is printed at the start of the assignment expression, This can be misleading.
Having the location for the diagnostic be the location of the assignment operator is much more intuitive.

[njames93]

Remove unnecessary includes added.

[gribozavr2]

clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp

66–67

Drive-by comment -- an Expr is not a Decl (feel free to defer to a separate patch).

clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp

 //===--- 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/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace bugprone {
bool VisitIfStmt(IfStmt *If) {
 
       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(),
+  SourceLocation OpLoc =
+      isa<BinaryOperator>(MatchedDecl)
+          ? cast<BinaryOperator>(MatchedDecl)->getOperatorLoc()
+          : cast<CXXOperatorCallExpr>(MatchedDecl)->getOperatorLoc();
+
+  diag(OpLoc, "an assignment within an 'if' condition is bug-prone")
+      << MatchedDecl->getSourceRange();
+  diag(OpLoc,
        "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 '=='",
+  diag(OpLoc, "if it is meant to be an equality check, change '=' to '=='",
        DiagnosticIDs::Note);
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp

 // 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:42: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: 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]
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: 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) {