From 3a1f6efce413cb8f841ae26a919c32dd6b87a152 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Wed, 2 Oct 2024 13:37:24 +0200
Subject: [PATCH] Address review comments

---
 .../rust/elements/internal/VariableImpl.qll   | 23 +++++++++++++------
 .../test/query-tests/unusedentities/main.rs   |  2 +-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
index 0f7968079157..3f0cabc97b85 100644
--- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
+++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
@@ -387,27 +387,36 @@ module Impl {
     override string getAPrimaryQlClass() { result = "VariableAccess" }
   }
 
-  /** Holds if `e` occurs in the LHS of a (compound) assignment. */
-  private predicate assignLhs(Expr e) {
-    exists(BinaryExpr be |
-      be.getOperatorName().regexpMatch(".*=") and
+  /** Holds if `e` occurs in the LHS of an assignment or compound assignment. */
+  private predicate assignLhs(Expr e, boolean compound) {
+    exists(BinaryExpr be, string op |
+      op = be.getOperatorName().regexpCapture("(.*)=", 1) and
       e = be.getLhs()
+    |
+      op = "" and compound = false
+      or
+      op != "" and compound = true
     )
     or
     exists(Expr mid |
-      assignLhs(mid) and
+      assignLhs(mid, compound) and
       getImmediateParent(e) = mid
     )
   }
 
   /** A variable write. */
   class VariableWriteAccess extends VariableAccess {
-    VariableWriteAccess() { assignLhs(this) }
+    VariableWriteAccess() { assignLhs(this, _) }
   }
 
   /** A variable read. */
   class VariableReadAccess extends VariableAccess {
-    VariableReadAccess() { not this instanceof VariableWriteAccess }
+    VariableReadAccess() {
+      not this instanceof VariableWriteAccess
+      or
+      // consider LHS in compound assignments both reads and writes
+      assignLhs(this, true)
+    }
   }
 
   cached
diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs
index c58693ed8667..7280f6c2502b 100644
--- a/rust/ql/test/query-tests/unusedentities/main.rs
+++ b/rust/ql/test/query-tests/unusedentities/main.rs
@@ -109,7 +109,7 @@ fn arrays() {
 
     println!("lets use {:?}", js);
 
-    for k // BAD: unused variable [SPURIOUS: macros not yet supported]
+    for k // SPURIOUS: unused variable [macros not yet supported]
 	in ks
 	{
         println!("lets use {}", k);