Merge pull request #17847 from paldepind/rust-unused-variable-trait

Rust: Don't consider parameters in trait method definitions without bodies as variables

rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

@@ -84,7 +84,10 @@ module Impl {
     // an enum constant (e.g. `None`). This excludes static and constant variables (UPPERCASE),
     // which we don't appear to recognize yet anyway. This also assumes programmers follow the
     // naming guidelines, which they generally do, but they're not enforced.
-    not name.charAt(0).isUppercase()
+    not name.charAt(0).isUppercase() and
+    // exclude parameters from functions without a body as these are trait method declarations
+    // without implementations
+    not exists(Function f | not f.hasBody() and f.getParamList().getAParam().getPat() = p)
   }
 
   /** A variable. */

rust/ql/test/query-tests/unusedentities/UnusedValue.expected

@@ -19,15 +19,10 @@
 | main.rs:370:9:370:9 | x | Variable is assigned a value that is never used. |
 | main.rs:378:17:378:17 | x | Variable is assigned a value that is never used. |
 | main.rs:432:9:432:10 | i6 | Variable is assigned a value that is never used. |
-| more.rs:8:9:8:13 | times | Variable is assigned a value that is never used. |
+| more.rs:24:9:24:11 | val | Variable is assigned a value that is never used. |
-| more.rs:9:9:9:14 | unused | Variable is assigned a value that is never used. |
+| more.rs:46:9:46:14 | a_ptr4 | Variable is assigned a value that is never used. |
-| more.rs:21:9:21:14 | unused | Variable is assigned a value that is never used. |
+| more.rs:61:9:61:13 | d_ptr | Variable is assigned a value that is never used. |
-| more.rs:38:23:38:25 | val | Variable is assigned a value that is never used. |
+| more.rs:67:9:67:17 | f_ptr | Variable is assigned a value that is never used. |
-| more.rs:42:19:42:21 | val | Variable is assigned a value that is never used. |
-| more.rs:58:9:58:11 | val | Variable is assigned a value that is never used. |
-| more.rs:80:9:80:14 | a_ptr4 | Variable is assigned a value that is never used. |
-| more.rs:95:9:95:13 | d_ptr | Variable is assigned a value that is never used. |
-| more.rs:101:9:101:17 | f_ptr | Variable is assigned a value that is never used. |
 | unreachable.rs:166:6:166:6 | x | Variable is assigned a value that is never used. |
 | unreachable.rs:190:14:190:14 | a | Variable is assigned a value that is never used. |
 | unreachable.rs:199:9:199:9 | a | Variable is assigned a value that is never used. |

rust/ql/test/query-tests/unusedentities/UnusedVariable.expected

@@ -18,3 +18,4 @@
 | main.rs:379:21:379:21 | y | Variable is not used. |
 | main.rs:427:27:427:29 | val | Variable is not used. |
 | main.rs:430:22:430:24 | acc | Variable is not used. |
+| main.rs:455:9:455:14 | unused | Variable is not used. |

rust/ql/test/query-tests/unusedentities/main.rs

@@ -434,6 +434,38 @@ fn folds_and_closures() {
     _ = a6.fold(0, | acc, val | acc + val + i6);
 }
 
+// --- traits ---
+
+trait Incrementable {
+    fn increment(
+        &mut self,
+        times: i32,
+        unused: &mut i32
+    );
+}
+
+struct MyValue {
+    value: i32,
+}
+
+impl Incrementable for MyValue {
+    fn increment(
+        &mut self,
+        times: i32,
+        unused: i32 // BAD: unused variable
+    ) {
+        self.value += times;
+    }
+}
+
+fn traits() {
+    let mut i = MyValue { value: 0 };
+    let a = 1;
+    let b = 2;
+
+    i.increment(a, b);
+}
+
 // --- main ---
 
 fn main() {

rust/ql/test/query-tests/unusedentities/more.rs

@@ -1,45 +1,11 @@
-
-
-// --- traits ---
-
-trait Incrementable {
-    fn increment(
-        &mut self,
-        times: i32, // SPURIOUS: unused value
-        unused: i32 // SPURIOUS: unused value
-    );
-}
-
-struct MyValue {
-    value: i32,
-}
-
-impl Incrementable for MyValue {
-    fn increment(
-        &mut self,
-        times: i32,
-        unused: i32 // BAD: unused variable [NOT DETECTED] SPURIOUS: unused value
-    ) {
-        self.value += times;
-    }
-}
-
-fn traits() {
-    let mut i = MyValue { value: 0 };
-    let a = 1;
-    let b = 2;
-
-    i.increment(a, b);
-}
-
 // --- generics ---
 
 trait MySettable<T> {
-    fn set(&mut self, val: T); // SPURIOUS: unused value
+    fn set(&mut self, val: T);
 }
 
 trait MyGettable<T> {
-    fn get(&self, val: T) -> &T; // SPURIOUS: unused value
+    fn get(&self, val: T) -> &T;
 }
 
 struct MyContainer<T> {