Skip to content

Commit d6415cd

Browse files
hvitvedhvitved
authored
Merge pull request #17642 from hvitved/rust/unused-variable
Rust: Implement `UnusedVariable.ql`
2 parents 3fa52ad + 3a1f6ef commit d6415cd

File tree

7 files changed

+282
-106
lines changed

7 files changed

+282
-106
lines changed

Diff for: ‎rust/ql/lib/codeql/rust/elements/Variable.qll

+4
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,7 @@ private import internal.VariableImpl
77
final class Variable = Impl::Variable;
88

99
final class VariableAccess = Impl::VariableAccess;
10+
11+
final class VariableWriteAccess = Impl::VariableWriteAccess;
12+
13+
final class VariableReadAccess = Impl::VariableReadAccess;

Diff for: ‎rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

+53
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,27 @@ module Impl {
9898

9999
/** Gets an access to this variable. */
100100
VariableAccess getAnAccess() { result.getVariable() = this }
101+
102+
/**
103+
* Gets the pattern that declares this variable.
104+
*
105+
* Normally, the pattern is unique, except when introduced in an or pattern:
106+
*
107+
* ```rust
108+
* match either {
109+
* Either::Left(x) | Either::Right(x) => println!(x),
110+
* }
111+
* ```
112+
*/
113+
IdentPat getPat() { variableDecl(definingNode, result, name) }
114+
115+
/** Gets the initial value of this variable, if any. */
116+
Expr getInitializer() {
117+
exists(LetStmt let |
118+
this.getPat() = let.getPat() and
119+
result = let.getInitializer()
120+
)
121+
}
101122
}
102123

103124
/** A path expression that may access a local variable. */
@@ -366,6 +387,38 @@ module Impl {
366387
override string getAPrimaryQlClass() { result = "VariableAccess" }
367388
}
368389

390+
/** Holds if `e` occurs in the LHS of an assignment or compound assignment. */
391+
private predicate assignLhs(Expr e, boolean compound) {
392+
exists(BinaryExpr be, string op |
393+
op = be.getOperatorName().regexpCapture("(.*)=", 1) and
394+
e = be.getLhs()
395+
|
396+
op = "" and compound = false
397+
or
398+
op != "" and compound = true
399+
)
400+
or
401+
exists(Expr mid |
402+
assignLhs(mid, compound) and
403+
getImmediateParent(e) = mid
404+
)
405+
}
406+
407+
/** A variable write. */
408+
class VariableWriteAccess extends VariableAccess {
409+
VariableWriteAccess() { assignLhs(this, _) }
410+
}
411+
412+
/** A variable read. */
413+
class VariableReadAccess extends VariableAccess {
414+
VariableReadAccess() {
415+
not this instanceof VariableWriteAccess
416+
or
417+
// consider LHS in compound assignments both reads and writes
418+
assignLhs(this, true)
419+
}
420+
}
421+
369422
cached
370423
private module Cached {
371424
cached

Diff for: ‎rust/ql/src/queries/unusedentities/UnusedVariable.ql

+7-4
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@
33
* @description Unused variables may be an indication that the code is incomplete or has a typo.
44
* @kind problem
55
* @problem.severity recommendation
6-
* @precision medium
6+
* @precision high
77
* @id rust/unused-variable
88
* @tags maintainability
99
*/
1010

1111
import rust
1212

13-
from Locatable e
14-
where none() // TODO: implement query
15-
select e, "Variable is not used."
13+
from Variable v
14+
where
15+
not exists(v.getAnAccess()) and
16+
not exists(v.getInitializer()) and
17+
not v.getName().charAt(0) = "_"
18+
select v, "Variable is not used."

Diff for: ‎rust/ql/test/library-tests/variables/variables.expected

+106
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,109 @@ variableAccess
146146
| variables.rs:306:15:306:16 | n2 | variables.rs:304:9:304:10 | n2 |
147147
| variables.rs:313:12:313:12 | v | variables.rs:310:9:310:9 | v |
148148
| variables.rs:314:19:314:22 | text | variables.rs:312:9:312:12 | text |
149+
variableWriteAccess
150+
| variables.rs:17:5:17:6 | x2 | variables.rs:15:13:15:14 | x2 |
151+
| variables.rs:266:9:266:10 | c2 | variables.rs:259:13:259:14 | c2 |
152+
| variables.rs:267:9:267:10 | b4 | variables.rs:258:13:258:14 | b4 |
153+
| variables.rs:268:9:268:11 | a10 | variables.rs:257:13:257:15 | a10 |
154+
variableReadAccess
155+
| variables.rs:11:15:11:16 | x1 | variables.rs:10:9:10:10 | x1 |
156+
| variables.rs:16:15:16:16 | x2 | variables.rs:15:13:15:14 | x2 |
157+
| variables.rs:18:15:18:16 | x2 | variables.rs:15:13:15:14 | x2 |
158+
| variables.rs:23:15:23:16 | x3 | variables.rs:22:9:22:10 | x3 |
159+
| variables.rs:25:9:25:10 | x3 | variables.rs:22:9:22:10 | x3 |
160+
| variables.rs:26:15:26:16 | x3 | variables.rs:24:9:24:10 | x3 |
161+
| variables.rs:31:15:31:16 | x4 | variables.rs:30:9:30:10 | x4 |
162+
| variables.rs:34:19:34:20 | x4 | variables.rs:33:13:33:14 | x4 |
163+
| variables.rs:36:15:36:16 | x4 | variables.rs:30:9:30:10 | x4 |
164+
| variables.rs:55:15:55:16 | a1 | variables.rs:47:13:47:14 | a1 |
165+
| variables.rs:56:15:56:16 | b1 | variables.rs:48:13:48:14 | b1 |
166+
| variables.rs:57:15:57:15 | x | variables.rs:51:13:51:13 | x |
167+
| variables.rs:58:15:58:15 | y | variables.rs:52:13:52:13 | y |
168+
| variables.rs:66:9:66:10 | p1 | variables.rs:62:9:62:10 | p1 |
169+
| variables.rs:67:15:67:16 | a2 | variables.rs:64:12:64:13 | a2 |
170+
| variables.rs:68:15:68:16 | b2 | variables.rs:65:12:65:13 | b2 |
171+
| variables.rs:75:11:75:12 | s1 | variables.rs:72:9:72:10 | s1 |
172+
| variables.rs:76:19:76:20 | s2 | variables.rs:74:21:74:22 | s2 |
173+
| variables.rs:85:15:85:16 | x5 | variables.rs:81:14:81:15 | x5 |
174+
| variables.rs:92:11:92:12 | x6 | variables.rs:89:9:89:10 | x6 |
175+
| variables.rs:97:23:97:24 | y1 | variables.rs:94:14:94:15 | y1 |
176+
| variables.rs:102:15:102:16 | y1 | variables.rs:90:9:90:10 | y1 |
177+
| variables.rs:108:11:108:17 | numbers | variables.rs:106:9:106:15 | numbers |
178+
| variables.rs:114:23:114:27 | first | variables.rs:110:13:110:17 | first |
179+
| variables.rs:115:23:115:27 | third | variables.rs:111:13:111:17 | third |
180+
| variables.rs:116:23:116:27 | fifth | variables.rs:112:13:112:17 | fifth |
181+
| variables.rs:120:11:120:17 | numbers | variables.rs:106:9:106:15 | numbers |
182+
| variables.rs:126:23:126:27 | first | variables.rs:122:13:122:17 | first |
183+
| variables.rs:127:23:127:26 | last | variables.rs:124:13:124:16 | last |
184+
| variables.rs:135:11:135:12 | p2 | variables.rs:133:9:133:10 | p2 |
185+
| variables.rs:138:24:138:25 | x7 | variables.rs:137:16:137:17 | x7 |
186+
| variables.rs:149:11:149:13 | msg | variables.rs:147:9:147:11 | msg |
187+
| variables.rs:152:24:152:34 | id_variable | variables.rs:151:17:151:27 | id_variable |
188+
| variables.rs:157:23:157:24 | id | variables.rs:156:26:156:27 | id |
189+
| variables.rs:168:11:168:16 | either | variables.rs:167:9:167:14 | either |
190+
| variables.rs:170:26:170:27 | a3 | variables.rs:169:9:169:44 | a3 |
191+
| variables.rs:182:11:182:12 | tv | variables.rs:181:9:181:10 | tv |
192+
| variables.rs:184:26:184:27 | a4 | variables.rs:183:9:183:81 | a4 |
193+
| variables.rs:186:11:186:12 | tv | variables.rs:181:9:181:10 | tv |
194+
| variables.rs:188:26:188:27 | a5 | variables.rs:187:9:187:83 | a5 |
195+
| variables.rs:190:11:190:12 | tv | variables.rs:181:9:181:10 | tv |
196+
| variables.rs:192:26:192:27 | a6 | variables.rs:191:9:191:83 | a6 |
197+
| variables.rs:198:11:198:16 | either | variables.rs:197:9:197:14 | either |
198+
| variables.rs:200:16:200:17 | a7 | variables.rs:199:9:199:44 | a7 |
199+
| variables.rs:201:26:201:27 | a7 | variables.rs:199:9:199:44 | a7 |
200+
| variables.rs:209:11:209:16 | either | variables.rs:207:9:207:14 | either |
201+
| variables.rs:213:23:213:25 | a11 | variables.rs:211:14:211:51 | a11 |
202+
| variables.rs:215:15:215:15 | e | variables.rs:210:13:210:13 | e |
203+
| variables.rs:216:28:216:30 | a12 | variables.rs:214:33:214:35 | a12 |
204+
| variables.rs:232:11:232:12 | fv | variables.rs:231:9:231:10 | fv |
205+
| variables.rs:234:26:234:28 | a13 | variables.rs:233:9:233:109 | a13 |
206+
| variables.rs:244:15:244:16 | a8 | variables.rs:239:5:239:6 | a8 |
207+
| variables.rs:245:15:245:16 | b3 | variables.rs:241:9:241:10 | b3 |
208+
| variables.rs:246:15:246:16 | c1 | variables.rs:242:9:242:10 | c1 |
209+
| variables.rs:252:15:252:16 | a9 | variables.rs:250:6:250:41 | a9 |
210+
| variables.rs:261:15:261:17 | a10 | variables.rs:257:13:257:15 | a10 |
211+
| variables.rs:262:15:262:16 | b4 | variables.rs:258:13:258:14 | b4 |
212+
| variables.rs:263:15:263:16 | c2 | variables.rs:259:13:259:14 | c2 |
213+
| variables.rs:270:9:270:11 | a10 | variables.rs:257:13:257:15 | a10 |
214+
| variables.rs:271:9:271:10 | b4 | variables.rs:258:13:258:14 | b4 |
215+
| variables.rs:272:9:272:10 | c2 | variables.rs:259:13:259:14 | c2 |
216+
| variables.rs:274:15:274:17 | a10 | variables.rs:257:13:257:15 | a10 |
217+
| variables.rs:275:15:275:16 | b4 | variables.rs:258:13:258:14 | b4 |
218+
| variables.rs:276:15:276:16 | c2 | variables.rs:259:13:259:14 | c2 |
219+
| variables.rs:283:23:283:25 | a10 | variables.rs:280:13:280:15 | a10 |
220+
| variables.rs:284:23:284:24 | b4 | variables.rs:281:13:281:14 | b4 |
221+
| variables.rs:288:15:288:17 | a10 | variables.rs:257:13:257:15 | a10 |
222+
| variables.rs:289:15:289:16 | b4 | variables.rs:258:13:258:14 | b4 |
223+
| variables.rs:295:9:295:9 | x | variables.rs:294:10:294:10 | x |
224+
| variables.rs:297:9:297:23 | example_closure | variables.rs:293:9:293:23 | example_closure |
225+
| variables.rs:298:15:298:16 | n1 | variables.rs:296:9:296:10 | n1 |
226+
| variables.rs:303:9:303:9 | x | variables.rs:302:10:302:10 | x |
227+
| variables.rs:305:9:305:26 | immutable_variable | variables.rs:301:9:301:26 | immutable_variable |
228+
| variables.rs:306:15:306:16 | n2 | variables.rs:304:9:304:10 | n2 |
229+
| variables.rs:313:12:313:12 | v | variables.rs:310:9:310:9 | v |
230+
| variables.rs:314:19:314:22 | text | variables.rs:312:9:312:12 | text |
231+
variableInitializer
232+
| variables.rs:10:9:10:10 | x1 | variables.rs:10:14:10:16 | "a" |
233+
| variables.rs:15:13:15:14 | x2 | variables.rs:15:18:15:18 | 4 |
234+
| variables.rs:22:9:22:10 | x3 | variables.rs:22:14:22:14 | 1 |
235+
| variables.rs:24:9:24:10 | x3 | variables.rs:25:9:25:14 | ... + ... |
236+
| variables.rs:30:9:30:10 | x4 | variables.rs:30:14:30:16 | "a" |
237+
| variables.rs:33:13:33:14 | x4 | variables.rs:33:18:33:20 | "b" |
238+
| variables.rs:62:9:62:10 | p1 | variables.rs:62:14:62:37 | RecordExpr |
239+
| variables.rs:72:9:72:10 | s1 | variables.rs:72:14:72:41 | CallExpr |
240+
| variables.rs:89:9:89:10 | x6 | variables.rs:89:14:89:20 | CallExpr |
241+
| variables.rs:90:9:90:10 | y1 | variables.rs:90:14:90:15 | 10 |
242+
| variables.rs:106:9:106:15 | numbers | variables.rs:106:19:106:35 | TupleExpr |
243+
| variables.rs:133:9:133:10 | p2 | variables.rs:133:14:133:37 | RecordExpr |
244+
| variables.rs:147:9:147:11 | msg | variables.rs:147:15:147:38 | RecordExpr |
245+
| variables.rs:167:9:167:14 | either | variables.rs:167:18:167:33 | CallExpr |
246+
| variables.rs:181:9:181:10 | tv | variables.rs:181:14:181:36 | CallExpr |
247+
| variables.rs:197:9:197:14 | either | variables.rs:197:18:197:33 | CallExpr |
248+
| variables.rs:207:9:207:14 | either | variables.rs:207:18:207:33 | CallExpr |
249+
| variables.rs:231:9:231:10 | fv | variables.rs:231:14:231:35 | CallExpr |
250+
| variables.rs:293:9:293:23 | example_closure | variables.rs:294:9:295:9 | ClosureExpr |
251+
| variables.rs:296:9:296:10 | n1 | variables.rs:297:9:297:26 | CallExpr |
252+
| variables.rs:301:9:301:26 | immutable_variable | variables.rs:302:9:303:9 | ClosureExpr |
253+
| variables.rs:304:9:304:10 | n2 | variables.rs:305:9:305:29 | CallExpr |
254+
| variables.rs:310:9:310:9 | v | variables.rs:310:13:310:41 | RefExpr |

Diff for: ‎rust/ql/test/library-tests/variables/variables.ql

+6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ query predicate variable(Variable v) { any() }
55

66
query predicate variableAccess(VariableAccess va, Variable v) { v = va.getVariable() }
77

8+
query predicate variableWriteAccess(VariableWriteAccess va, Variable v) { v = va.getVariable() }
9+
10+
query predicate variableReadAccess(VariableReadAccess va, Variable v) { v = va.getVariable() }
11+
12+
query predicate variableInitializer(Variable v, Expr e) { e = v.getInitializer() }
13+
814
module VariableAccessTest implements TestSig {
915
string getARelevantTag() { result = "access" }
1016

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

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| main.rs:23:9:23:9 | a | Variable is not used. |
2+
| main.rs:88:13:88:13 | d | Variable is not used. |
3+
| main.rs:112:9:112:9 | k | Variable is not used. |
4+
| main.rs:139:5:139:5 | y | Variable is not used. |

0 commit comments

Comments
 (0)