Address review comments

Author: hvitved

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

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);