[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

63–64

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

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

 /// 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;
-  void report(const Expr *E);
+  void report(const Expr *AssignmentExpr);
 };
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSIGNMENTINIFCONDITIONCHECK_H

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

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(),
+void AssignmentInIfConditionCheck::report(const Expr *AssignmentExpr) {
+  SourceLocation OpLoc =
+      isa<BinaryOperator>(AssignmentExpr)
+          ? cast<BinaryOperator>(AssignmentExpr)->getOperatorLoc()
+          : cast<CXXOperatorCallExpr>(AssignmentExpr)->getOperatorLoc();
+
+  diag(OpLoc, "an assignment within an 'if' condition is bug-prone")
+      << AssignmentExpr->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) {