Skip to content

Commit

Permalink
Create rule S6814: Optional REST parameters should have an object type (
Browse files Browse the repository at this point in the history
  • Loading branch information
github-actions[bot] authored Oct 18, 2023
1 parent 1763d7f commit 4371239
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 0 deletions.
23 changes: 23 additions & 0 deletions rules/S6814/java/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "Optional REST parameters should have an object type",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-6814",
"sqKey": "S6814",
"scope": "All",
"defaultQualityProfiles": ["Sonar way"],
"quickfix": "unknown",
"code": {
"impacts": {
"RELIABILITY": "HIGH"
},
"attribute": "CONVENTIONAL"
}
}
68 changes: 68 additions & 0 deletions rules/S6814/java/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
== Why is this an issue?

Spring provides two options to mark a REST parameter as optional:

1. Use `required = false` in the `@PathVariable` or `@RequestParam` annotation of the respective method parameter or
2. Use type `java.util.Optional<T>` for the method parameter
When using 1., the absence of the parameter, when the REST function is called, is encoded by `null`, which can only be used for object types.
If `required = false` is used for a parameter with a primitive and the REST function is called without the parameter, a runtime exception occurs because the Spring data mapper cannot map the `null` value to the parameter.

== How to fix it

Replace primitive types, such as `boolean`, `char`, `int`, with the corresponding wrapper type, such as `Boolean`, `Character`, `Integer`.

Alternatively, you might choose to remove `required = false` from the annotation and use an `Optional<T>` type for the parameter, such as `Optional<Boolean>` or `Optional<String>`, which automatically makes the REST parameter optional.
This is the preferred approach because it enforces the proper handling of `null` in the method implementation.

=== Code examples

==== Noncompliant code example

[source,java,diff-id=1,diff-type=noncompliant]
----
@RequestMapping(value = {"/article", "/article/{id}"})
public Article getArticle(@PathVariable(required = false) int articleId) { // Noncompliant, null cannot be mapped to int
//...
}
----

==== Compliant solution

[source,java,diff-id=1,diff-type=compliant]
----
@RequestMapping(value = {"/article", "/article/{id}"})
public Article getArticle(@PathVariable(required = false) Integer articleId) { // Compliant
//...
}
----

==== Noncompliant code example

[source,java,diff-id=2,diff-type=noncompliant]
----
@RequestMapping(value = {"/article", "/article/{id}"})
public Article getArticle(@PathVariable(required = false) int articleId) { // Noncompliant, null cannot be mapped to int
//...
}
----

==== Compliant solution

[source,java,diff-id=2,diff-type=compliant]
----
@RequestMapping(value = {"/article", "/article/{id}"})
public Article getArticle(@PathVariable Optional<Integer> articleId) { // Compliant and preferred approach
//...
}
----

== Resources

=== Documentation

- https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/bind/annotation/PathVariable.html[Spring Framework API - Annotation Interface PathVariable]

=== Articles & blog posts

- https://www.baeldung.com/spring-optional-path-variables[Baeldung - Spring Optional Path Variables]
2 changes: 2 additions & 0 deletions rules/S6814/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}

0 comments on commit 4371239

Please sign in to comment.