Skip to content

Commit de1cfd6

Browse files
Implement new rule S8908
This rule detects methods annotated with @CacheResult that return void. Since void methods have no return value to cache, applying @CacheResult to them is meaningless and indicates a misunderstanding of how caching works.
1 parent a992aa7 commit de1cfd6

6 files changed

Lines changed: 268 additions & 0 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package checks;
2+
3+
import io.quarkus.cache.CacheResult;
4+
5+
public class QuarkusCacheResultOnVoidMethodCheckSample {
6+
7+
@CacheResult(cacheName = "my-cache")
8+
public void processData(UnknownType key) { // Noncompliant
9+
}
10+
11+
@CacheResult(cacheName = "my-cache")
12+
public UnknownType getData(String key) { // Compliant - unknown return type, cannot determine if void
13+
}
14+
15+
@CacheResult(cacheName = "my-cache")
16+
public unknownReturnType computeValue(String input) { // Compliant - unknown return type
17+
}
18+
19+
public void methodWithoutAnnotation(UnknownType param) { // Compliant
20+
}
21+
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package checks;
2+
3+
import io.quarkus.cache.CacheInvalidate;
4+
import io.quarkus.cache.CacheInvalidateAll;
5+
import io.quarkus.cache.CacheResult;
6+
import java.util.List;
7+
import java.util.Optional;
8+
9+
public class QuarkusCacheResultOnVoidMethodCheckSample {
10+
11+
@CacheResult(cacheName = "my-cache")
12+
public void processData(String key) { // Noncompliant {{Methods annotated with "@CacheResult" should not return void.}}
13+
// ^^^^
14+
}
15+
16+
@CacheResult(cacheName = "calc-cache", lockTimeout = 1000)
17+
public void performCalculation(String input) { // Noncompliant
18+
}
19+
20+
@CacheResult(cacheName = "cache")
21+
void packagePrivateVoid(String data) { // Noncompliant
22+
}
23+
24+
@CacheResult(cacheName = "cache")
25+
protected void protectedVoid() { // Noncompliant
26+
}
27+
28+
@CacheResult(cacheName = "cache")
29+
private void privateVoid(int value) { // Noncompliant
30+
}
31+
32+
@CacheResult(cacheName = "my-cache")
33+
public String getData(String key) { // Compliant
34+
return "result-" + key;
35+
}
36+
37+
@CacheResult(cacheName = "user-cache")
38+
public User getUser(Long userId) { // Compliant
39+
return new User();
40+
}
41+
42+
@CacheResult(cacheName = "int-cache")
43+
public Integer calculate(String input) { // Compliant
44+
return input.length();
45+
}
46+
47+
@CacheResult(cacheName = "optional-cache")
48+
public Optional<String> findValue(String key) { // Compliant
49+
return Optional.empty();
50+
}
51+
52+
@CacheResult(cacheName = "list-cache")
53+
public List<String> getList(String item) { // Compliant
54+
return List.of(item);
55+
}
56+
57+
@CacheResult(cacheName = "array-cache")
58+
public String[] getArray() { // Compliant
59+
return new String[]{"a", "b"};
60+
}
61+
62+
public void voidMethodNoAnnotation(String key) { // Compliant
63+
}
64+
65+
@CacheInvalidate(cacheName = "my-cache")
66+
public void invalidateCache(String key) { // Compliant
67+
}
68+
69+
@CacheInvalidateAll(cacheName = "my-cache")
70+
public void clearCache() { // Compliant
71+
}
72+
73+
public String methodNoAnnotation(String input) { // Compliant
74+
return input;
75+
}
76+
77+
private static class User {
78+
}
79+
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import java.util.List;
20+
import java.util.Optional;
21+
import java.util.function.Function;
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.java.api.DependencyVersionAware;
24+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
25+
import org.sonar.plugins.java.api.Version;
26+
import org.sonar.plugins.java.api.semantic.Type;
27+
import org.sonar.plugins.java.api.tree.MethodTree;
28+
import org.sonar.plugins.java.api.tree.Tree;
29+
import org.sonar.plugins.java.api.tree.TypeTree;
30+
31+
@Rule(key = "S8908")
32+
public class QuarkusCacheResultOnVoidMethodCheck extends IssuableSubscriptionVisitor implements DependencyVersionAware {
33+
34+
private static final String CACHE_RESULT_ANNOTATION = "io.quarkus.cache.CacheResult";
35+
36+
@Override
37+
public List<Tree.Kind> nodesToVisit() {
38+
return List.of(Tree.Kind.METHOD);
39+
}
40+
41+
@Override
42+
public void visitNode(Tree tree) {
43+
var mt = (MethodTree) tree;
44+
if (mt.symbol().metadata().isAnnotatedWith(CACHE_RESULT_ANNOTATION)) {
45+
TypeTree returnType = mt.returnType();
46+
// returnType can only be null if the method is a constructor. Since the @CacheResult annotation is not allowed on constructors, and since
47+
// we hence only visit methods, not constructors, we assume that returnType is not null.
48+
Type symbolType = returnType.symbolType();
49+
if (symbolType.isUnknown()) {
50+
return;
51+
}
52+
if (symbolType.isVoid()) {
53+
reportIssue(returnType, "Methods annotated with \"@CacheResult\" should not return void.");
54+
}
55+
}
56+
}
57+
58+
@Override
59+
public boolean isCompatibleWithDependencies(Function<String, Optional<Version>> dependencyFinder) {
60+
return dependencyFinder.apply("quarkus-cache").isPresent();
61+
}
62+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
22+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
23+
import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath;
24+
25+
class QuarkusCacheResultOnVoidMethodCheckTest {
26+
@Test
27+
void test() {
28+
CheckVerifier.newVerifier()
29+
.onFile(mainCodeSourcesPath("checks/QuarkusCacheResultOnVoidMethodCheckSample.java"))
30+
.withCheck(new QuarkusCacheResultOnVoidMethodCheck())
31+
.verifyIssues();
32+
}
33+
34+
@Test
35+
void test_non_compiling() {
36+
CheckVerifier.newVerifier()
37+
.onFile(nonCompilingTestSourcesPath("checks/QuarkusCacheResultOnVoidMethodCheckSample.java"))
38+
.withCheck(new QuarkusCacheResultOnVoidMethodCheck())
39+
.verifyIssues();
40+
}
41+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>Return value caching annotations are designed to cache the output of a method. When a method annotated for result caching is invoked, the framework computes a cache key and checks if the result has been previously cached. If found, the cached value is returned without executing the method body. If not found, the method executes and its return value is stored in the cache.</p>
3+
<p>This caching mechanism fundamentally requires a return value to function. A <code>void</code> method, by definition, does not return a value - it only produces side effects. Since there is no value to cache or retrieve, applying result caching annotations to a void method is meaningless and will not provide any caching benefits.</p>
4+
<p>This misuse typically indicates one of the following:</p>
5+
<ul>
6+
<li>A misunderstanding of how result caching works</li>
7+
<li>An attempt to cache side effects, which is not supported by these annotations</li>
8+
<li>A method that should be refactored to return a value if caching is truly needed</li>
9+
</ul>
10+
<p>Framework documentation explicitly states that result caching annotations cannot be used on methods returning <code>void</code>.</p>
11+
<h2>What is the potential impact?</h2>
12+
<p>While this issue does not cause runtime errors or security vulnerabilities, it can lead to:</p>
13+
<ul>
14+
<li>Confusion about the intended behavior of the code</li>
15+
<li>Wasted developer time investigating why caching is not working as expected</li>
16+
<li>Maintenance overhead from unnecessary annotations that serve no purpose</li>
17+
<li>Potential performance misconceptions if developers believe expensive operations are being cached when they are not</li>
18+
</ul>
19+
<h2>How to fix it</h2>
20+
<p>Remove the <code>@CacheResult</code> annotation from the void method if caching is not needed, or refactor the method to return a value that can be meaningfully cached.</p>
21+
<h3>Code examples</h3>
22+
<h4>Noncompliant code example</h4>
23+
<pre data-diff-id="1" data-diff-type="noncompliant">
24+
@CacheResult(cacheName = "my-cache")
25+
public void processData(String key) { // Noncompliant
26+
// Process expensive data
27+
expensiveService.process(key);
28+
}
29+
</pre>
30+
<h4>Compliant solution</h4>
31+
<pre data-diff-id="1" data-diff-type="compliant">
32+
@CacheResult(cacheName = "my-cache")
33+
public String processData(String key) {
34+
// Process expensive data and return result
35+
String result = expensiveService.process(key);
36+
return result;
37+
}
38+
</pre>
39+
<h2>Resources</h2>
40+
<h3>Documentation</h3>
41+
<ul>
42+
<li>Quarkus Cache Guide - <a href="https://quarkus.io/guides/cache">Official Quarkus documentation on application data caching, including proper usage of @CacheResult annotation</a></li>
43+
<li>Quarkus @CacheResult Annotation Reference - <a href="https://quarkus.io/guides/cache#cacheresult">Detailed reference for the @CacheResult annotation, explicitly noting it cannot be used on void methods</a></li>
44+
</ul>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "Methods annotated with \"@CacheResult\" should not return void",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Minor",
11+
"ruleSpecification": "RSPEC-8908",
12+
"sqKey": "S8908",
13+
"scope": "All",
14+
"quickfix": "unknown",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "LOW"
18+
},
19+
"attribute": "CLEAR"
20+
}
21+
}

0 commit comments

Comments
 (0)