Skip to content

Commit 6e197b5

Browse files
aibaarsaibaars
authored
Merge pull request github#17773 from geoffw0/unusedval2
Rust: Implement UnusedValue.ql (2)
2 parents b07c788 + 7e2542b commit 6e197b5

File tree

8 files changed

+246
-78
lines changed

8 files changed

+246
-78
lines changed

‎rust/ql/src/queries/unusedentities/UnusedValue.ql

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,16 @@
99
*/
1010

1111
import rust
12+
import codeql.rust.dataflow.Ssa
13+
import codeql.rust.dataflow.internal.SsaImpl
14+
import UnusedVariable
1215

13-
from Locatable e
14-
where none() // TODO: implement query
15-
select e, "Variable is assigned a value that is never used."
16+
from AstNode write, Ssa::Variable v
17+
where
18+
variableWrite(write, v) and
19+
// SSA definitions are only created for live writes
20+
not write = any(Ssa::WriteDefinition def).getWriteAccess().getAstNode() and
21+
// avoid overlap with the unused variable query
22+
not isUnused(v) and
23+
not v instanceof DiscardVariable
24+
select write, "Variable is assigned a value that is never used."

‎rust/ql/src/queries/unusedentities/UnusedVariable.ql

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@
99
*/
1010

1111
import rust
12+
import UnusedVariable
1213

1314
from Variable v
14-
where
15-
not exists(v.getAnAccess()) and
16-
not exists(v.getInitializer()) and
17-
not v.getName().charAt(0) = "_" and
18-
exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
15+
where isUnused(v)
1916
select v, "Variable is not used."

‎rust/ql/src/queries/unusedentities/UnusedVariable.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import rust
2+
3+
/** A deliberately unused variable. */
4+
class DiscardVariable extends Variable {
5+
DiscardVariable() { this.getName().charAt(0) = "_" }
6+
}
7+
8+
/** Holds if variable `v` is unused. */
9+
predicate isUnused(Variable v) {
10+
not exists(v.getAnAccess()) and
11+
not exists(v.getInitializer()) and
12+
not v instanceof DiscardVariable and
13+
exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
14+
}

‎rust/ql/test/query-tests/unusedentities/CONSISTENCY/CfgConsistency.expected

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
multipleSuccessors
2-
| main.rs:428:17:428:17 | 0 | successor | main.rs:428:9:428:10 | a2 |
3-
| main.rs:428:17:428:17 | 0 | successor | main.rs:428:20:428:62 | ClosureExpr |
4-
| main.rs:431:17:431:17 | 0 | successor | main.rs:431:9:431:10 | a3 |
5-
| main.rs:431:17:431:17 | 0 | successor | main.rs:431:20:431:41 | ClosureExpr |
6-
| main.rs:434:17:434:17 | 0 | successor | main.rs:434:9:434:10 | a4 |
7-
| main.rs:434:17:434:17 | 0 | successor | main.rs:434:20:434:35 | ClosureExpr |
8-
| main.rs:437:17:437:17 | 0 | successor | main.rs:437:9:437:10 | a5 |
9-
| main.rs:437:17:437:17 | 0 | successor | main.rs:437:20:437:35 | ClosureExpr |
10-
| main.rs:441:17:441:17 | 0 | successor | main.rs:441:9:441:10 | a6 |
11-
| main.rs:441:17:441:17 | 0 | successor | main.rs:441:20:441:46 | ClosureExpr |
2+
| main.rs:421:17:421:17 | 0 | successor | main.rs:421:9:421:10 | a2 |
3+
| main.rs:421:17:421:17 | 0 | successor | main.rs:421:20:421:62 | ClosureExpr |
4+
| main.rs:424:17:424:17 | 0 | successor | main.rs:424:9:424:10 | a3 |
5+
| main.rs:424:17:424:17 | 0 | successor | main.rs:424:20:424:41 | ClosureExpr |
6+
| main.rs:427:17:427:17 | 0 | successor | main.rs:427:9:427:10 | a4 |
7+
| main.rs:427:17:427:17 | 0 | successor | main.rs:427:20:427:35 | ClosureExpr |
8+
| main.rs:430:17:430:17 | 0 | successor | main.rs:430:9:430:10 | a5 |
9+
| main.rs:430:17:430:17 | 0 | successor | main.rs:430:20:430:35 | ClosureExpr |
10+
| main.rs:434:17:434:17 | 0 | successor | main.rs:434:9:434:10 | a6 |
11+
| main.rs:434:17:434:17 | 0 | successor | main.rs:434:20:434:46 | ClosureExpr |
12+
| more.rs:32:17:32:17 | a | successor | more.rs:32:5:32:5 | i |
13+
| more.rs:32:17:32:17 | a | successor | more.rs:32:20:32:20 | b |
14+
deadEnd
15+
| more.rs:9:9:9:19 | Param |
16+
| more.rs:38:23:38:28 | Param |
17+
| more.rs:42:19:42:24 | Param |

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
| main.rs:6:9:6:9 | a | Variable is assigned a value that is never used. |
2+
| main.rs:9:9:9:9 | d | Variable is assigned a value that is never used. |
3+
| main.rs:35:5:35:5 | b | Variable is assigned a value that is never used. |
4+
| main.rs:37:5:37:5 | c | Variable is assigned a value that is never used. |
5+
| main.rs:40:5:40:5 | c | Variable is assigned a value that is never used. |
6+
| main.rs:44:9:44:9 | d | Variable is assigned a value that is never used. |
7+
| main.rs:50:5:50:5 | e | Variable is assigned a value that is never used. |
8+
| main.rs:61:5:61:5 | f | Variable is assigned a value that is never used. |
9+
| main.rs:63:5:63:5 | f | Variable is assigned a value that is never used. |
10+
| main.rs:65:5:65:5 | g | Variable is assigned a value that is never used. |
11+
| main.rs:87:9:87:9 | a | Variable is assigned a value that is never used. |
12+
| main.rs:108:9:108:10 | is | Variable is assigned a value that is never used. |
13+
| main.rs:133:13:133:17 | total | Variable is assigned a value that is never used. |
14+
| main.rs:203:13:203:31 | res | Variable is assigned a value that is never used. |
15+
| main.rs:218:9:218:24 | kind | Variable is assigned a value that is never used. |
16+
| main.rs:223:9:223:32 | kind | Variable is assigned a value that is never used. |
17+
| main.rs:280:13:280:17 | total | Variable is assigned a value that is never used. |
18+
| main.rs:348:5:348:39 | kind | Variable is assigned a value that is never used. |
19+
| main.rs:370:9:370:9 | x | Variable is assigned a value that is never used. |
20+
| main.rs:378:17:378:17 | x | Variable is assigned a value that is never used. |
21+
| main.rs:432:9:432:10 | i6 | Variable is assigned a value that is never used. |
22+
| more.rs:8:9:8:13 | times | Variable is assigned a value that is never used. |
23+
| more.rs:9:9:9:14 | unused | Variable is assigned a value that is never used. |
24+
| more.rs:21:9:21:14 | unused | Variable is assigned a value that is never used. |
25+
| more.rs:38:23:38:25 | val | Variable is assigned a value that is never used. |
26+
| more.rs:42:19:42:21 | val | Variable is assigned a value that is never used. |
27+
| more.rs:58:9:58:11 | val | Variable is assigned a value that is never used. |
28+
| more.rs:80:9:80:14 | a_ptr4 | Variable is assigned a value that is never used. |
29+
| more.rs:95:9:95:13 | d_ptr | Variable is assigned a value that is never used. |
30+
| more.rs:101:9:101:17 | f_ptr | Variable is assigned a value that is never used. |

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,17 @@
66
| main.rs:201:9:201:9 | x | Variable is not used. |
77
| main.rs:250:17:250:17 | a | Variable is not used. |
88
| main.rs:258:20:258:22 | val | Variable is not used. |
9-
| main.rs:271:14:271:16 | val | Variable is not used. |
10-
| main.rs:288:22:288:24 | val | Variable is not used. |
11-
| main.rs:296:24:296:26 | val | Variable is not used. |
12-
| main.rs:305:13:305:15 | num | Variable is not used. |
13-
| main.rs:320:12:320:12 | j | Variable is not used. |
14-
| main.rs:342:25:342:25 | y | Variable is not used. |
15-
| main.rs:346:28:346:28 | a | Variable is not used. |
16-
| main.rs:350:9:350:9 | p | Variable is not used. |
17-
| main.rs:365:9:365:13 | right | Variable is not used. |
18-
| main.rs:371:9:371:14 | right2 | Variable is not used. |
19-
| main.rs:378:13:378:13 | y | Variable is not used. |
20-
| main.rs:386:21:386:21 | y | Variable is not used. |
21-
| main.rs:434:27:434:29 | val | Variable is not used. |
22-
| main.rs:437:22:437:24 | acc | Variable is not used. |
9+
| main.rs:272:14:272:16 | val | Variable is not used. |
10+
| main.rs:287:22:287:24 | val | Variable is not used. |
11+
| main.rs:294:24:294:26 | val | Variable is not used. |
12+
| main.rs:302:13:302:15 | num | Variable is not used. |
13+
| main.rs:317:12:317:12 | j | Variable is not used. |
14+
| main.rs:337:25:337:25 | y | Variable is not used. |
15+
| main.rs:340:28:340:28 | a | Variable is not used. |
16+
| main.rs:343:9:343:9 | p | Variable is not used. |
17+
| main.rs:358:9:358:13 | right | Variable is not used. |
18+
| main.rs:364:9:364:14 | right2 | Variable is not used. |
19+
| main.rs:371:13:371:13 | y | Variable is not used. |
20+
| main.rs:379:21:379:21 | y | Variable is not used. |
21+
| main.rs:427:27:427:29 | val | Variable is not used. |
22+
| main.rs:430:22:430:24 | acc | Variable is not used. |

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

Lines changed: 39 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
// --- locals ---
44

55
fn locals_1() {
6-
let a = 1; // BAD: unused value [NOT DETECTED]
6+
let a = 1; // BAD: unused value
77
let b = 1;
88
let c = 1;
9-
let d = String::from("a"); // BAD: unused value [NOT DETECTED]
9+
let d = String::from("a"); // BAD: unused value
1010
let e = String::from("b");
1111
let f = 1;
1212
let _ = 1; // (deliberately unused)
@@ -32,22 +32,22 @@ fn locals_2() {
3232
let h: i32;
3333
let i: i32;
3434

35-
b = 1; // BAD: unused value [NOT DETECTED]
35+
b = 1; // BAD: unused value
3636

37-
c = 1; // BAD: unused value [NOT DETECTED]
37+
c = 1; // BAD: unused value
3838
c = 2;
3939
println!("use {}", c);
40-
c = 3; // BAD: unused value [NOT DETECTED]
40+
c = 3; // BAD: unused value
4141

4242
d = 1;
4343
if cond() {
44-
d = 2; // BAD: unused value [NOT DETECTED]
44+
d = 2; // BAD: unused value
4545
d = 3;
4646
} else {
4747
}
4848
println!("use {}", d);
4949

50-
e = 1; // BAD: unused value [NOT DETECTED]
50+
e = 1; // BAD: unused value
5151
if cond() {
5252
e = 2;
5353
} else {
@@ -58,16 +58,16 @@ fn locals_2() {
5858
f = 1;
5959
f += 1;
6060
println!("use {}", f);
61-
f += 1; // BAD: unused value [NOT DETECTED]
61+
f += 1; // BAD: unused value
6262
f = 1;
63-
f += 1; // BAD: unused value [NOT DETECTED]
63+
f += 1; // BAD: unused value
6464

65-
g = if cond() { 1 } else { 2 }; // BAD: unused value (x2) [NOT DETECTED]
65+
g = if cond() { 1 } else { 2 }; // BAD: unused value
6666
h = if cond() { 3 } else { 4 };
6767
i = if cond() { h } else { 5 };
6868
println!("use {}", i);
6969

70-
_ = 1; // (deliberately unused) [NOT DETECTED]
70+
_ = 1; // GOOD (deliberately unused)
7171
}
7272

7373
// --- structs ---
@@ -84,7 +84,7 @@ impl MyStruct {
8484
}
8585

8686
fn structs() {
87-
let a = MyStruct { val: 1 }; // BAD: unused value [NOT DETECTED]
87+
let a = MyStruct { val: 1 }; // BAD: unused value
8888
let b = MyStruct { val: 2 };
8989
let c = MyStruct { val: 3 };
9090
let mut d: MyStruct; // BAD: unused variable
@@ -105,7 +105,7 @@ fn structs() {
105105
// --- arrays ---
106106

107107
fn arrays() {
108-
let is = [1, 2, 3]; // BAD: unused values (x3) [NOT DETECTED]
108+
let is = [1, 2, 3]; // BAD: unused value
109109
let js = [1, 2, 3];
110110
let ks = [1, 2, 3];
111111

@@ -130,7 +130,7 @@ fn statics() {
130130
static mut STAT4: i32 = 0; // BAD: unused value [NOT DETECTED]
131131

132132
unsafe {
133-
let total = CON1 + STAT1 + STAT3;
133+
let total = CON1 + STAT1 + STAT3; // BAD: unused value
134134
}
135135
}
136136

@@ -200,7 +200,7 @@ fn loops() {
200200

201201
for x // SPURIOUS: unused variable
202202
in 1..10 {
203-
_ = format!("x is {x}");
203+
_ = format!("x is {x}"); // SPURIOUS: unused value `res`
204204
}
205205

206206
for x
@@ -215,12 +215,12 @@ fn loops() {
215215

216216
for x
217217
in 1..10 {
218-
assert_eq!(x, 1);
218+
assert_eq!(x, 1); // SPURIOUS: unused value `kind`
219219
}
220220

221221
for x
222222
in 1..10 {
223-
assert_eq!(id(x), id(1));
223+
assert_eq!(id(x), id(1)); // SPURIOUS: unused value `kind`
224224
}
225225

226226
}
@@ -255,7 +255,8 @@ fn if_lets_matches() {
255255
}
256256

257257
let mut next = Some(30);
258-
while let Some(val) = next // BAD: unused variable
258+
while let Some(val) = // BAD: unused variable
259+
next
259260
{
260261
next = None;
261262
}
@@ -270,25 +271,22 @@ fn if_lets_matches() {
270271
match c {
271272
Some(val) => { // BAD: unused variable
272273
}
273-
None => {
274-
}
274+
None => {}
275275
}
276276

277277
let d = Some(70);
278278
match d {
279279
Some(val) => {
280-
total += val;
281-
}
282-
None => {
280+
total += val; // BAD: unused value
283281
}
282+
None => {}
284283
}
285284

286285
let e = Option::Some(80);
287286
match e {
288287
Option::Some(val) => { // BAD: unused variable
289288
}
290-
Option::None => {
291-
}
289+
Option::None => {}
292290
}
293291

294292
let f = MyOption::Some(90);
@@ -298,10 +296,9 @@ fn if_lets_matches() {
298296
MyOption::None => {}
299297
}
300298

301-
let g : Result<i64, i64> = Ok(100);
299+
let g: Result<i64, i64> = Ok(100);
302300
match g {
303-
Ok(_) => {
304-
}
301+
Ok(_) => {}
305302
Err(num) => {} // BAD: unused variable
306303
}
307304

@@ -327,8 +324,7 @@ fn if_lets_matches() {
327324
}
328325

329326
let l = Yes;
330-
if let Yes = l {
331-
}
327+
if let Yes = l {}
332328

333329
match 1 {
334330
1 => {}
@@ -337,22 +333,19 @@ fn if_lets_matches() {
337333

338334
let p1 = MyPoint { x: 1, y: 2 };
339335
match p1 {
340-
MyPoint { x: 0, y: 0 } => {
341-
}
336+
MyPoint { x: 0, y: 0 } => {}
342337
MyPoint { x: 1, y } => { // BAD: unused variable
343338
}
344-
MyPoint { x: 2, y: _ } => {
345-
}
339+
MyPoint { x: 2, y: _ } => {}
346340
MyPoint { x: 3, y: a } => { // BAD: unused variable
347341
}
348-
MyPoint { x: 4, .. } => {
349-
}
342+
MyPoint { x: 4, .. } => {}
350343
p => { // BAD: unused variable
351344
}
352345
}
353346

354347
let duration1 = std::time::Duration::new(10, 0); // ten seconds
355-
assert_eq!(duration1.as_secs(), 10);
348+
assert_eq!(duration1.as_secs(), 10); // SPURIOUS: unused value `kind`
356349

357350
let duration2:Result<std::time::Duration, String> =
358351
Ok(std::time::Duration::new(10, 0));
@@ -374,15 +367,15 @@ fn if_lets_matches() {
374367
}
375368

376369
fn shadowing() -> i32 {
377-
let x = 1; // BAD: unused value [NOT DETECTED]
370+
let x = 1; // BAD: unused value
378371
let mut y: i32; // BAD: unused variable
379372

380373
{
381374
let x = 2;
382375
let mut y: i32;
383376

384377
{
385-
let x = 3; // BAD: unused value [NOT DETECTED]
378+
let x = 3; // BAD: unused value
386379
let mut y: i32; // BAD: unused variable
387380
}
388381

@@ -436,7 +429,7 @@ fn folds_and_closures() {
436429
let a5 = 1..10;
437430
_ = a5.fold(0, | acc, val | val); // BAD: unused variable
438431

439-
let i6 = 1;
432+
let i6 = 1; // SPURIOUS: unused value
440433
let a6 = 1..10;
441434
_ = a6.fold(0, | acc, val | acc + val + i6);
442435
}
@@ -449,16 +442,16 @@ fn main() {
449442
structs();
450443
arrays();
451444
statics();
452-
println!("lets use result {}", parameters(1, 2, 3));
445+
println!("lets use result {}", parameters(1, 2, 3));
453446
loops();
454447
if_lets_matches();
455448
shadowing();
456449
func_ptrs();
457450
folds_and_closures();
458451

459-
unreachable_if();
460-
unreachable_panic();
461-
unreachable_match();
462-
unreachable_loop();
463-
unreachable_paren();
452+
unreachable_if();
453+
unreachable_panic();
454+
unreachable_match();
455+
unreachable_loop();
456+
unreachable_paren();
464457
}

0 commit comments

Comments
 (0)