Skip to content

Commit 204ca08

Browse files
committed
Add misleading-boolean-setter.ql
1 parent 45c202f commit 204ca08

File tree

1 file changed

+39
-0
lines changed

1 file changed

+39
-0
lines changed
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* Finds methods which look like a setter method taking a boolean parameter,
3+
* but which actually use `|= ` (OR) or `&=` (AND) to set the value.
4+
* Such methods can be misleading because they effectively ignore `false`
5+
* respectively `true` values.
6+
* For example:
7+
* ```java
8+
* public void setEnabled(boolean enabled) {
9+
* this.enabled |= enabled;
10+
* }
11+
* ```
12+
* Without knowing how the method is implemented, a caller might assume that
13+
* `setEnabled(false)` disables the functionality. However, if `this.enabled`
14+
* is already `true`, then this call has no effect (because `true | false` is `true`).
15+
*
16+
* It might be less confusing if the `boolean` parameter was removed from the method:
17+
* ```java
18+
* public void setEnabled() {
19+
* this.enabled = true;
20+
* }
21+
* ```
22+
* It is clear then that there is no way to 'disable' the functionality.
23+
*
24+
* @kind problem
25+
* @id todo
26+
*/
27+
28+
import java
29+
30+
// TODO: Maybe also consider setters which use `||` or `&&`, or guard the assignment with an `if` statement
31+
from Method setter, Parameter param, AssignOp assignExpr
32+
where
33+
setter.getNumberOfParameters() = 1 and
34+
param = setter.getParameter(0) and
35+
param.getType() instanceof BooleanType and
36+
assignExpr = setter.getBody().(SingletonBlock).getStmt().(ExprStmt).getExpr() and
37+
assignExpr.getSource() = param.getAnAccess() and
38+
(assignExpr instanceof AssignOrExpr or assignExpr instanceof AssignAndExpr)
39+
select setter, "Misleading boolean setter method"

0 commit comments

Comments
 (0)