Rust: Implement UnusedVariable.ql

github · Oct 2, 2024 · 8634dd5 · 8634dd5
1 parent 5fb61b0
commit 8634dd5
Show file tree

Hide file tree

Showing 8 changed files with 279 additions and 112 deletions.
diff --git a/rust/ql/lib/codeql/rust/elements/Variable.qll b/rust/ql/lib/codeql/rust/elements/Variable.qll
@@ -7,3 +7,7 @@ private import internal.VariableImpl
 final class Variable = Impl::Variable;
 
 final class VariableAccess = Impl::VariableAccess;
+
+final class VariableWriteAccess = Impl::VariableWriteAccess;
+
+final class VariableReadAccess = Impl::VariableReadAccess;
diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
@@ -98,6 +98,27 @@ module Impl {
 
     /** Gets an access to this variable. */
     VariableAccess getAnAccess() { result.getVariable() = this }
+
+    /**
+     * Gets the pattern that declares this variable.
+     *
+     * Normally, the pattern is unique, except when introduced in an or pattern:
+     *
+     * ```rust
+     * match either {
+     *    Either::Left(x) | Either::Right(x) => println!(x),
+     * }
+     * ```
+     */
+    IdentPat getPat() { variableDecl(definingNode, result, name) }
+
+    /** Gets the initial value of this variable, if any. */
+    Expr getInitializer() {
+      exists(LetStmt let |
+        this.getPat() = let.getPat() and
+        result = let.getInitializer()
+      )
+    }
   }
 
   /** A path expression that may access a local variable. */
@@ -366,6 +387,29 @@ 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
+      e = be.getLhs()
+    )
+    or
+    exists(Expr mid |
+      assignLhs(mid) and
+      getImmediateParent(e) = mid
+    )
+  }
+
+  /** A variable write. */
+  class VariableWriteAccess extends VariableAccess {
+    VariableWriteAccess() { assignLhs(this) }
+  }
+
+  /** A variable read. */
+  class VariableReadAccess extends VariableAccess {
+    VariableReadAccess() { not this instanceof VariableWriteAccess }
+  }
+
   cached
   private module Cached {
     cached

diff --git a/rust/ql/src/queries/unusedentities/UnusedVariable.ql b/rust/ql/src/queries/unusedentities/UnusedVariable.ql
@@ -3,13 +3,16 @@
  * @description Unused variables may be an indication that the code is incomplete or has a typo.
  * @kind problem
  * @problem.severity recommendation
- * @precision medium
+ * @precision high
  * @id rust/unused-variable
  * @tags maintainability
  */
 
 import rust
 
-from Locatable e
-where none() // TODO: implement query
-select e, "Variable is not used."
+from Variable v
+where
+  not exists(v.getAnAccess()) and
+  not exists(v.getInitializer()) and
+  not v.getName().charAt(0) = "_"
+select v, "Variable is not used."
diff --git a/rust/ql/test/library-tests/variables/variables.expected b/rust/ql/test/library-tests/variables/variables.expected
@@ -146,3 +146,109 @@ variableAccess
 | variables.rs:306:15:306:16 | n2 | variables.rs:304:9:304:10 | n2 |
 | variables.rs:313:12:313:12 | v | variables.rs:310:9:310:9 | v |
 | variables.rs:314:19:314:22 | text | variables.rs:312:9:312:12 | text |
+variableWriteAccess
+| variables.rs:17:5:17:6 | x2 | variables.rs:15:13:15:14 | x2 |
+| variables.rs:266:9:266:10 | c2 | variables.rs:259:13:259:14 | c2 |
+| variables.rs:267:9:267:10 | b4 | variables.rs:258:13:258:14 | b4 |
+| variables.rs:268:9:268:11 | a10 | variables.rs:257:13:257:15 | a10 |
+variableReadAccess
+| variables.rs:11:15:11:16 | x1 | variables.rs:10:9:10:10 | x1 |
+| variables.rs:16:15:16:16 | x2 | variables.rs:15:13:15:14 | x2 |
+| variables.rs:18:15:18:16 | x2 | variables.rs:15:13:15:14 | x2 |
+| variables.rs:23:15:23:16 | x3 | variables.rs:22:9:22:10 | x3 |
+| variables.rs:25:9:25:10 | x3 | variables.rs:22:9:22:10 | x3 |
+| variables.rs:26:15:26:16 | x3 | variables.rs:24:9:24:10 | x3 |
+| variables.rs:31:15:31:16 | x4 | variables.rs:30:9:30:10 | x4 |
+| variables.rs:34:19:34:20 | x4 | variables.rs:33:13:33:14 | x4 |
+| variables.rs:36:15:36:16 | x4 | variables.rs:30:9:30:10 | x4 |
+| variables.rs:55:15:55:16 | a1 | variables.rs:47:13:47:14 | a1 |
+| variables.rs:56:15:56:16 | b1 | variables.rs:48:13:48:14 | b1 |
+| variables.rs:57:15:57:15 | x | variables.rs:51:13:51:13 | x |
+| variables.rs:58:15:58:15 | y | variables.rs:52:13:52:13 | y |
+| variables.rs:66:9:66:10 | p1 | variables.rs:62:9:62:10 | p1 |
+| variables.rs:67:15:67:16 | a2 | variables.rs:64:12:64:13 | a2 |
+| variables.rs:68:15:68:16 | b2 | variables.rs:65:12:65:13 | b2 |
+| variables.rs:75:11:75:12 | s1 | variables.rs:72:9:72:10 | s1 |
+| variables.rs:76:19:76:20 | s2 | variables.rs:74:21:74:22 | s2 |
+| variables.rs:85:15:85:16 | x5 | variables.rs:81:14:81:15 | x5 |
+| variables.rs:92:11:92:12 | x6 | variables.rs:89:9:89:10 | x6 |
+| variables.rs:97:23:97:24 | y1 | variables.rs:94:14:94:15 | y1 |
+| variables.rs:102:15:102:16 | y1 | variables.rs:90:9:90:10 | y1 |
+| variables.rs:108:11:108:17 | numbers | variables.rs:106:9:106:15 | numbers |
+| variables.rs:114:23:114:27 | first | variables.rs:110:13:110:17 | first |
+| variables.rs:115:23:115:27 | third | variables.rs:111:13:111:17 | third |
+| variables.rs:116:23:116:27 | fifth | variables.rs:112:13:112:17 | fifth |
+| variables.rs:120:11:120:17 | numbers | variables.rs:106:9:106:15 | numbers |
+| variables.rs:126:23:126:27 | first | variables.rs:122:13:122:17 | first |
+| variables.rs:127:23:127:26 | last | variables.rs:124:13:124:16 | last |
+| variables.rs:135:11:135:12 | p2 | variables.rs:133:9:133:10 | p2 |
+| variables.rs:138:24:138:25 | x7 | variables.rs:137:16:137:17 | x7 |
+| variables.rs:149:11:149:13 | msg | variables.rs:147:9:147:11 | msg |
+| variables.rs:152:24:152:34 | id_variable | variables.rs:151:17:151:27 | id_variable |
+| variables.rs:157:23:157:24 | id | variables.rs:156:26:156:27 | id |
+| variables.rs:168:11:168:16 | either | variables.rs:167:9:167:14 | either |
+| variables.rs:170:26:170:27 | a3 | variables.rs:169:9:169:44 | a3 |
+| variables.rs:182:11:182:12 | tv | variables.rs:181:9:181:10 | tv |
+| variables.rs:184:26:184:27 | a4 | variables.rs:183:9:183:81 | a4 |
+| variables.rs:186:11:186:12 | tv | variables.rs:181:9:181:10 | tv |
+| variables.rs:188:26:188:27 | a5 | variables.rs:187:9:187:83 | a5 |
+| variables.rs:190:11:190:12 | tv | variables.rs:181:9:181:10 | tv |
+| variables.rs:192:26:192:27 | a6 | variables.rs:191:9:191:83 | a6 |
+| variables.rs:198:11:198:16 | either | variables.rs:197:9:197:14 | either |
+| variables.rs:200:16:200:17 | a7 | variables.rs:199:9:199:44 | a7 |
+| variables.rs:201:26:201:27 | a7 | variables.rs:199:9:199:44 | a7 |
+| variables.rs:209:11:209:16 | either | variables.rs:207:9:207:14 | either |
+| variables.rs:213:23:213:25 | a11 | variables.rs:211:14:211:51 | a11 |
+| variables.rs:215:15:215:15 | e | variables.rs:210:13:210:13 | e |
+| variables.rs:216:28:216:30 | a12 | variables.rs:214:33:214:35 | a12 |
+| variables.rs:232:11:232:12 | fv | variables.rs:231:9:231:10 | fv |
+| variables.rs:234:26:234:28 | a13 | variables.rs:233:9:233:109 | a13 |
+| variables.rs:244:15:244:16 | a8 | variables.rs:239:5:239:6 | a8 |
+| variables.rs:245:15:245:16 | b3 | variables.rs:241:9:241:10 | b3 |
+| variables.rs:246:15:246:16 | c1 | variables.rs:242:9:242:10 | c1 |
+| variables.rs:252:15:252:16 | a9 | variables.rs:250:6:250:41 | a9 |
+| variables.rs:261:15:261:17 | a10 | variables.rs:257:13:257:15 | a10 |
+| variables.rs:262:15:262:16 | b4 | variables.rs:258:13:258:14 | b4 |
+| variables.rs:263:15:263:16 | c2 | variables.rs:259:13:259:14 | c2 |
+| variables.rs:270:9:270:11 | a10 | variables.rs:257:13:257:15 | a10 |
+| variables.rs:271:9:271:10 | b4 | variables.rs:258:13:258:14 | b4 |
+| variables.rs:272:9:272:10 | c2 | variables.rs:259:13:259:14 | c2 |
+| variables.rs:274:15:274:17 | a10 | variables.rs:257:13:257:15 | a10 |
+| variables.rs:275:15:275:16 | b4 | variables.rs:258:13:258:14 | b4 |
+| variables.rs:276:15:276:16 | c2 | variables.rs:259:13:259:14 | c2 |
+| variables.rs:283:23:283:25 | a10 | variables.rs:280:13:280:15 | a10 |
+| variables.rs:284:23:284:24 | b4 | variables.rs:281:13:281:14 | b4 |
+| variables.rs:288:15:288:17 | a10 | variables.rs:257:13:257:15 | a10 |
+| variables.rs:289:15:289:16 | b4 | variables.rs:258:13:258:14 | b4 |
+| variables.rs:295:9:295:9 | x | variables.rs:294:10:294:10 | x |
+| variables.rs:297:9:297:23 | example_closure | variables.rs:293:9:293:23 | example_closure |
+| variables.rs:298:15:298:16 | n1 | variables.rs:296:9:296:10 | n1 |
+| variables.rs:303:9:303:9 | x | variables.rs:302:10:302:10 | x |
+| variables.rs:305:9:305:26 | immutable_variable | variables.rs:301:9:301:26 | immutable_variable |
+| variables.rs:306:15:306:16 | n2 | variables.rs:304:9:304:10 | n2 |
+| variables.rs:313:12:313:12 | v | variables.rs:310:9:310:9 | v |
+| variables.rs:314:19:314:22 | text | variables.rs:312:9:312:12 | text |
+variableInitializer
+| variables.rs:10:9:10:10 | x1 | variables.rs:10:14:10:16 | "a" |
+| variables.rs:15:13:15:14 | x2 | variables.rs:15:18:15:18 | 4 |
+| variables.rs:22:9:22:10 | x3 | variables.rs:22:14:22:14 | 1 |
+| variables.rs:24:9:24:10 | x3 | variables.rs:25:9:25:14 | ... + ... |
+| variables.rs:30:9:30:10 | x4 | variables.rs:30:14:30:16 | "a" |
+| variables.rs:33:13:33:14 | x4 | variables.rs:33:18:33:20 | "b" |
+| variables.rs:62:9:62:10 | p1 | variables.rs:62:14:62:37 | RecordExpr |
+| variables.rs:72:9:72:10 | s1 | variables.rs:72:14:72:41 | CallExpr |
+| variables.rs:89:9:89:10 | x6 | variables.rs:89:14:89:20 | CallExpr |
+| variables.rs:90:9:90:10 | y1 | variables.rs:90:14:90:15 | 10 |
+| variables.rs:106:9:106:15 | numbers | variables.rs:106:19:106:35 | TupleExpr |
+| variables.rs:133:9:133:10 | p2 | variables.rs:133:14:133:37 | RecordExpr |
+| variables.rs:147:9:147:11 | msg | variables.rs:147:15:147:38 | RecordExpr |
+| variables.rs:167:9:167:14 | either | variables.rs:167:18:167:33 | CallExpr |
+| variables.rs:181:9:181:10 | tv | variables.rs:181:14:181:36 | CallExpr |
+| variables.rs:197:9:197:14 | either | variables.rs:197:18:197:33 | CallExpr |
+| variables.rs:207:9:207:14 | either | variables.rs:207:18:207:33 | CallExpr |
+| variables.rs:231:9:231:10 | fv | variables.rs:231:14:231:35 | CallExpr |
+| variables.rs:293:9:293:23 | example_closure | variables.rs:294:9:295:9 | ClosureExpr |
+| variables.rs:296:9:296:10 | n1 | variables.rs:297:9:297:26 | CallExpr |
+| variables.rs:301:9:301:26 | immutable_variable | variables.rs:302:9:303:9 | ClosureExpr |
+| variables.rs:304:9:304:10 | n2 | variables.rs:305:9:305:29 | CallExpr |
+| variables.rs:310:9:310:9 | v | variables.rs:310:13:310:41 | RefExpr |
diff --git a/rust/ql/test/library-tests/variables/variables.ql b/rust/ql/test/library-tests/variables/variables.ql
@@ -5,6 +5,12 @@ query predicate variable(Variable v) { any() }
 
 query predicate variableAccess(VariableAccess va, Variable v) { v = va.getVariable() }
 
+query predicate variableWriteAccess(VariableWriteAccess va, Variable v) { v = va.getVariable() }
+
+query predicate variableReadAccess(VariableReadAccess va, Variable v) { v = va.getVariable() }
+
+query predicate variableInitializer(Variable v, Expr e) { e = v.getInitializer() }
+
 module VariableAccessTest implements TestSig {
   string getARelevantTag() { result = "access" }
 

diff --git a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/CfgConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/CfgConsistency.expected
@@ -1,8 +1,8 @@
 deadEnd
-| main.rs:14:2:14:23 | ExprStmt |
-| main.rs:38:2:38:23 | ExprStmt |
-| main.rs:93:2:93:44 | ExprStmt |
-| main.rs:107:2:107:20 | LetStmt |
-| main.rs:151:2:151:53 | ExprStmt |
+| main.rs:13:5:13:26 | ExprStmt |
+| main.rs:37:5:37:26 | ExprStmt |
+| main.rs:92:5:92:47 | ExprStmt |
+| main.rs:106:5:106:23 | LetStmt |
+| main.rs:151:5:151:56 | ExprStmt |
 scopeNoFirst
-| main.rs:125:1:133:1 | statics |
+| main.rs:126:1:133:1 | statics |
diff --git a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
@@ -0,0 +1,4 @@
+| main.rs:23:9:23:9 | a | Variable is not used. |
+| main.rs:88:13:88:13 | d | Variable is not used. |
+| main.rs:112:9:112:9 | k | Variable is not used. |
+| main.rs:139:5:139:5 | y | Variable is not used. |