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

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, LegalizeAdulthood, 
dodohand.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132795

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


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
@@ -3,7 +3,7 @@
 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;
   }
@@ -12,7 +12,7 @@
 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;
   }
@@ -21,7 +21,7 @@
 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;
   }
@@ -32,7 +32,7 @@
 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;
   }
@@ -41,11 +41,11 @@
 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;
   }
@@ -68,12 +68,12 @@
     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;
   }
Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -8,6 +8,8 @@
 
 #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"
 
@@ -62,13 +64,17 @@
 }
 
 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);
 }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits