Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust: Diagnose unused variable false positives #17656

Merged
merged 5 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,13 @@ module Impl {
scope = ce.getBody() and
scope.getLocation().hasLocationInfo(_, line, column, _, _)
)
or
exists(WhileExpr we, LetExpr let |
let.getPat() = pat and
we.getCondition() = let and
scope = we.getLoopBody() and
scope.getLocation().hasLocationInfo(_, line, column, _, _)
)
)
}

Expand Down
3 changes: 2 additions & 1 deletion rust/ql/src/queries/unusedentities/UnusedVariable.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ from Variable v
where
not exists(v.getAnAccess()) and
not exists(v.getInitializer()) and
not v.getName().charAt(0) = "_"
not v.getName().charAt(0) = "_" and
exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
select v, "Variable is not used."
885 changes: 452 additions & 433 deletions rust/ql/test/library-tests/variables/Cfg.expected

Large diffs are not rendered by default.

369 changes: 188 additions & 181 deletions rust/ql/test/library-tests/variables/variables.expected

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions rust/ql/test/library-tests/variables/variables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ fn let_pattern4() {
print_str(x5); // $ access=x5
}

fn let_pattern5() {
let s1 = Some(String::from("Hello!")); // s1

while let Some(ref s2) // s2
= s1 { // $ access=s1
print_str(s2); // $ access=s2
}
}

fn match_pattern1() {
let x6 = Some(5); // x6
let y1 = 10; // y1_1
Expand Down
17 changes: 13 additions & 4 deletions rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
| 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. |
| main.rs:25:9:25:9 | a | Variable is not used. |
| main.rs:90:13:90:13 | d | Variable is not used. |
| main.rs:114:9:114:9 | k | Variable is not used. |
| main.rs:141:5:141:5 | y | Variable is not used. |
| main.rs:164:9:164:9 | x | Variable is not used. |
| main.rs:169:9:169:9 | x | Variable is not used. |
| main.rs:174:9:174:9 | x | Variable is not used. |
| main.rs:195:17:195:17 | a | Variable is not used. |
| main.rs:203:20:203:22 | val | Variable is not used. |
| main.rs:216:14:216:16 | val | Variable is not used. |
| main.rs:218:9:218:12 | None | Variable is not used. |
| main.rs:227:9:227:12 | None | Variable is not used. |
| main.rs:233:24:233:26 | val | Variable is not used. |
103 changes: 103 additions & 0 deletions rust/ql/test/query-tests/unusedentities/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ fn locals_1() {
let c = 1;
let d = String::from("a"); // BAD: unused value [NOT DETECTED]
let e = String::from("b");
let f = 1;
let _ = 1; // (deliberately unused)

println!("use {}", b);
Expand All @@ -17,6 +18,7 @@ fn locals_1() {
}

println!("use {}", e);
assert!(f == 1);
}

fn locals_2() {
Expand Down Expand Up @@ -142,11 +144,112 @@ fn parameters(
return x;
}

// --- loops ---

fn loops() {
let mut a: i64 = 10;
let b: i64 = 20;
let c: i64 = 30;
let d: i64 = 40;
let mut e: i64 = 50;

while a < b {
a += 1;
}

for x in c..d {
e += x;
}

for x in 1..10 { // BAD: unused variable
}

for _ in 1..10 {}

for x // SPURIOUS: unused variable [macros not yet supported]
in 1..10 {
println!("x is {}", x);
}

for x // SPURIOUS: unused variable [macros not yet supported]
in 1..10 {
assert!(x != 11);
}
}

// --- lets ---

enum MyOption<T> {
None,
Some(T),
}

enum YesOrNo {
Yes,
No,
}

fn if_lets() {
let mut total: i64 = 0;

if let Some(a) = Some(10) { // BAD: unused variable
}

if let Some(b) = Some(20) {
total += b;
}

let mut next = Some(30);
while let Some(val) = next // BAD: unused variable
{
next = None;
}

let mut next2 = Some(40);
while let Some(val) = next2 {
total += val;
next2 = None;
}

let c = Some(60);
match c {
Some(val) => { // BAD: unused variable
}
None => { // SPURIOUS: unused variable 'None'
}
}

let d = Some(70);
match d {
Some(val) => {
total += val;
}
None => { // SPURIOUS: unused variable 'None'
}
}

let e = MyOption::Some(80);
match e {
MyOption::Some(val) => { // BAD: unused variable
}
MyOption::None => {}
}

let f = YesOrNo::Yes;
match f {
YesOrNo::Yes => {}
YesOrNo::No => {}
}
}

fn main() {
locals_1();
locals_2();
structs();
arrays();
statics();
loops();
if_lets();

println!("lets use result {}", parameters(1, 2, 3));
}
Loading