Author: Nathan James Date: 2022-08-28T00:21:40+01:00 New Revision: 32d88239ae654239f16b516ee81ee9ff88b0ce07
URL: https://github.com/llvm/llvm-project/commit/32d88239ae654239f16b516ee81ee9ff88b0ce07 DIFF: https://github.com/llvm/llvm-project/commit/32d88239ae654239f16b516ee81ee9ff88b0ce07.diff LOG: [clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition 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. Reviewed By: gribozavr2 Differential Revision: https://reviews.llvm.org/D132795 Added: Modified: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h 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 05f0ae74282e..47a82e3c09e2 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp @@ -61,14 +61,18 @@ void AssignmentInIfConditionCheck::check( 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); } diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h index f84ae93ed2eb..f49dda24c9a9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h @@ -25,7 +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); + void report(const Expr *AssignmentExpr); }; } // namespace bugprone 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 20c30d73516b..ad0d360d99fe 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 @@ -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 f(int arg) { 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 f1(int arg) { 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 @@ 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; } @@ -41,11 +41,11 @@ void f3(int arg) { 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 @@ void f5(int arg) { 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; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
