Skip to content

Commit 2ec4f18

Browse files
committed
Add Optional-nullable-wrapping-unwrapping.ql
1 parent 3a28d26 commit 2ec4f18

File tree

2 files changed

+89
-8
lines changed

2 files changed

+89
-8
lines changed
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* Finds code which wraps a nullable value in an `Optional` and then directly unwraps it again.
3+
* Such code can be simplified using for example the methods `requireNonNull`, `requireNonNullElse` or
4+
* `requireNonNullElseGet` of the class `java.util.Objects`, which avoids having to temporarily wrap
5+
* the value in an `Optional`.
6+
*
7+
* For example:
8+
* ```java
9+
* Optional.ofNullable(value).orElse("default-value")
10+
* // Could be written as
11+
* Objects.requireNonNullElse(value, "default-value")
12+
* ```
13+
*
14+
* @kind problem
15+
* @id todo
16+
*/
17+
18+
import java
19+
import lib.Optionals
20+
21+
from Call wrappingCall, MethodAccess unwrappingCall
22+
where
23+
wrappingCall.getCallee() instanceof NewNullableOptionalCallable and
24+
unwrappingCall.getQualifier() = wrappingCall and
25+
// Only consider this when it is directly unwrapped again; ignore for example if `filter(...)` or similar
26+
// is called first, since rewriting it might make it more verbose
27+
(
28+
unwrappingCall.getMethod() instanceof OptionalGetValueMethod or
29+
unwrappingCall.getMethod() instanceof OptionalOrMethod
30+
)
31+
select unwrappingCall,
32+
"Use `Objects.requireNonNullElse` or similar instead of wrapping as `Optional`"

codeql-custom-queries-java/queries/lib/Optionals.qll

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,15 @@ abstract class OptionalPresenceCheckMethod extends Method {
9191
abstract class OptionalGetValueMethod extends Method {
9292
}
9393

94+
/**
95+
* Instance method which uses an alternative value in case the optional is empty.
96+
*
97+
* Does not include methods which throw an exception if the optional is empty
98+
* (e.g. `orElseThrow`).
99+
*/
100+
abstract class OptionalOrMethod extends Method {
101+
}
102+
94103
/**
95104
* JDK class representing an optional value. This includes the `Object` reference
96105
* storing `java.util.Optional` as well as the primitive value variants, such as
@@ -143,9 +152,20 @@ class JavaObjectOptionalOfNullableMethod extends NewNullableOptionalCallable, Me
143152
class JavaObjectOptionalGetMethod extends OptionalGetValueMethod {
144153
JavaObjectOptionalGetMethod() {
145154
getDeclaringSourceOrASupertype(this) instanceof JavaObjectOptional
146-
and hasStringSignature([
147-
"get()",
148-
"orElseThrow()" // Equivalent to get()
155+
and (
156+
hasStringSignature("get()")
157+
or hasName("orElseThrow") // Equivalent to get()
158+
)
159+
}
160+
}
161+
162+
class JavaObjectOptionalOrMethod extends OptionalOrMethod {
163+
JavaObjectOptionalOrMethod() {
164+
getDeclaringSourceOrASupertype(this) instanceof JavaObjectOptional
165+
and hasName([
166+
"or", // TODO: Maybe exclude this because it takes a `Supplier<Optional>` and returns an `Optional`
167+
"orElse",
168+
"orElseGet",
149169
])
150170
}
151171
}
@@ -196,11 +216,23 @@ class JavaOptionalPresenceCheckMethod extends OptionalPresenceCheckMethod {
196216
class JavaPrimitiveOptionalGetMethod extends OptionalGetValueMethod {
197217
JavaPrimitiveOptionalGetMethod() {
198218
getDeclaringType() instanceof JavaPrimitiveOptional
199-
and hasStringSignature([
200-
"orElseThrow()", // Exists for all Optional types and is equivalent to their specific methods
201-
"getAsDouble()", // OptionalDouble
202-
"getAsInt()", // OptionalInt
203-
"getAsLong()" // OptionalLong
219+
and (
220+
hasStringSignature([
221+
"getAsDouble()", // OptionalDouble
222+
"getAsInt()", // OptionalInt
223+
"getAsLong()", // OptionalLong
224+
])
225+
or hasName("orElseThrow") // Exists for all Optional types and is equivalent to their specific `get...` methods
226+
)
227+
}
228+
}
229+
230+
class JavaPrimitiveOptionalOrMethod extends OptionalOrMethod {
231+
JavaPrimitiveOptionalOrMethod() {
232+
getDeclaringType() instanceof JavaPrimitiveOptional
233+
and hasName([
234+
"orElse",
235+
"orElseGet",
204236
])
205237
}
206238
}
@@ -244,6 +276,23 @@ class GuavaOptionalIsPresentMethod extends OptionalPresenceCheckMethod {
244276
boolean polarity() { result = true }
245277
}
246278

279+
class GuavaOptionalGetMethod extends OptionalGetValueMethod {
280+
GuavaOptionalGetMethod() {
281+
getDeclaringSourceOrASupertype(this) instanceof GuavaOptional
282+
and hasStringSignature("get()")
283+
}
284+
}
285+
286+
class GuavaOptionalOrMethod extends OptionalOrMethod {
287+
GuavaOptionalOrMethod() {
288+
getDeclaringSourceOrASupertype(this) instanceof GuavaOptional
289+
and hasName([
290+
"or", // TODO: Maybe exclude overload `or(Optional)`, which returns an `Optional`?
291+
"orNull",
292+
])
293+
}
294+
}
295+
247296
class GuavaOptionalOrNullCall extends OptionalObjectOrNullCall {
248297
GuavaOptionalOrNullCall() {
249298
exists(Method m | m = getMethod() |

0 commit comments

Comments
 (0)