Skip to content

Commit d2b338d

Browse files
irina-batinic-sonarsourceirina-batinic-sonarsource
authored
SONARJAVA-4679 S6833: @controller should be replaced with @RestContro… (#4521)
1 parent 0e3edb0 commit d2b338d

9 files changed

Lines changed: 318 additions & 2 deletions

File tree

‎its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,15 @@ public void javaCheckTestSources() throws Exception {
180180
softly.assertThat(newTotal).isEqualTo(knownTotal);
181181
softly.assertThat(rulesCausingFPs).hasSize(6);
182182
softly.assertThat(rulesNotReporting).hasSize(7);
183-
softly.assertThat(rulesSilenced).hasSize(79);
183+
softly.assertThat(rulesSilenced).hasSize(80);
184184

185185
/**
186186
* 4. Check total number of differences (FPs + FNs)
187187
*
188188
* No differences would mean that we find the same issues with and without the bytecode and libraries
189189
*/
190190
String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences"));
191-
softly.assertThat(differences).isEqualTo("Issues differences: 3508");
191+
softly.assertThat(differences).isEqualTo("Issues differences: 3513");
192192

193193
softly.assertAll();
194194
}

‎its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2939,6 +2939,12 @@
29392939
"falseNegatives": 6,
29402940
"falsePositives": 0
29412941
},
2942+
{
2943+
"ruleKey": "S6833",
2944+
"hasTruePositives": false,
2945+
"falseNegatives": 5,
2946+
"falsePositives": 0
2947+
},
29422948
{
29432949
"ruleKey": "S6837",
29442950
"hasTruePositives": false,

‎java-checks-test-sources/src/main/java/checks/spring/ControllerWithRestControllerReplacementCheck.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package checks.spring;
2+
3+
import javax.annotation.Nullable;
4+
import org.springframework.stereotype.Controller;
5+
import org.springframework.stereotype.Service;
6+
import org.springframework.web.bind.annotation.ResponseBody;
7+
8+
public class ControllerWithRestControllerReplacementCheck {
9+
10+
@Controller // Noncompliant [[sc=3;ec=14;secondary=16;quickfixes=qf1,qf2]] {{Replace the "@Controller" annotation by "@RestController" and remove all
11+
// "@ResponseBody" annotations.}}
12+
// fix@qf2 {{Replace "@Controller" by "@RestController".}}
13+
// edit@qf2 [[sl=10;el=10;sc=3;ec=14]] {{@RestController}}
14+
class ClassOne {
15+
16+
@ResponseBody
17+
// fix@qf1 {{Remove "@ResponseBody" annotations.}}
18+
// edit@qf1 [[sl=16;el=16;sc=5;ec=18]] {{}}
19+
public void m() {
20+
}
21+
}
22+
23+
@Controller // Noncompliant
24+
@ResponseBody
25+
class ClassTwo {
26+
27+
public void m() {
28+
}
29+
}
30+
31+
@Controller // Noncompliant
32+
class ClassThree {
33+
34+
@ResponseBody
35+
public void m() {
36+
}
37+
38+
public void m2() {
39+
}
40+
}
41+
42+
@Controller // Compliant
43+
class ClassFour {
44+
45+
public void m() {
46+
}
47+
}
48+
49+
@Controller // Compliant
50+
class ClassFive {
51+
52+
@Nullable
53+
public void m() {
54+
}
55+
}
56+
57+
@Service // Compliant
58+
class ClassSix {
59+
60+
@ResponseBody
61+
public void m() {
62+
}
63+
}
64+
65+
@Controller // Noncompliant [[sc=3;ec=14;secondary=71,77;quickfixes=qf3,qf4]] {{Replace the "@Controller" annotation by "@RestController" and remove all
66+
// "@ResponseBody" annotations.}}
67+
// fix@qf4 {{Replace "@Controller" by "@RestController".}}
68+
// edit@qf4 [[sl=65;el=65;sc=3;ec=14]] {{@RestController}}
69+
class ClassSeven {
70+
71+
@ResponseBody
72+
// fix@qf3 {{Remove "@ResponseBody" annotations.}}
73+
// edit@qf3 [[sl=71;el=71;sc=5;ec=18]] {{}}
74+
public void m() {
75+
}
76+
77+
@ResponseBody
78+
// fix@qf3 {{Remove "@ResponseBody" annotations.}}
79+
// edit@qf3 [[sl=77;el=77;sc=5;ec=18]] {{}}
80+
public void m2() {
81+
}
82+
}
83+
}

‎java-checks/src/main/java/org/sonar/java/checks/CheckList.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@
141141
import org.sonar.java.checks.spring.AutowiredOnConstructorWhenMultipleConstructorsCheck;
142142
import org.sonar.java.checks.spring.AutowiredOnMultipleConstructorsCheck;
143143
import org.sonar.java.checks.spring.AvoidQualifierOnBeanMethodsCheck;
144+
import org.sonar.java.checks.spring.ControllerWithRestControllerReplacementCheck;
144145
import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
145146
import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
146147
import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
@@ -364,6 +365,7 @@ public final class CheckList {
364365
ConstructorCallingOverridableCheck.class,
365366
ConstructorInjectionCheck.class,
366367
ControlCharacterInLiteralCheck.class,
368+
ControllerWithRestControllerReplacementCheck.class,
367369
ControllerWithSessionAttributesCheck.class,
368370
CookieHttpOnlyCheck.class,
369371
HardCodedCredentialsShouldNotBeUsedCheck.class,

‎java-checks/src/main/java/org/sonar/java/checks/spring/ControllerWithRestControllerReplacementCheck.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2023 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.checks.spring;
21+
22+
import java.util.ArrayList;
23+
import java.util.List;
24+
import org.sonar.check.Rule;
25+
import org.sonar.java.checks.helpers.QuickFixHelper;
26+
import org.sonar.java.reporting.JavaQuickFix;
27+
import org.sonar.java.reporting.JavaTextEdit;
28+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
29+
import org.sonar.plugins.java.api.JavaFileScannerContext;
30+
import org.sonar.plugins.java.api.tree.ClassTree;
31+
import org.sonar.plugins.java.api.tree.MethodTree;
32+
import org.sonar.plugins.java.api.tree.Tree;
33+
34+
@Rule(key = "S6833")
35+
public class ControllerWithRestControllerReplacementCheck extends IssuableSubscriptionVisitor {
36+
37+
@Override
38+
public List<Tree.Kind> nodesToVisit() {
39+
return List.of(Tree.Kind.CLASS);
40+
}
41+
42+
@Override
43+
public void visitNode(Tree tree) {
44+
var classTree = (ClassTree) tree;
45+
46+
var annotation = classTree.modifiers().annotations().stream()
47+
.filter(a -> "org.springframework.stereotype.Controller".equals(a.annotationType().symbolType().fullyQualifiedName()))
48+
.findFirst();
49+
50+
if (annotation.isEmpty()) {
51+
return;
52+
}
53+
54+
var secondaryLocations = new ArrayList<JavaFileScannerContext.Location>();
55+
List<JavaTextEdit> edits = new ArrayList<>();
56+
57+
classTree.members().stream()
58+
.filter(member -> member.is(Tree.Kind.METHOD))
59+
.map(MethodTree.class::cast)
60+
.forEach(method -> {
61+
var methodAnnotation = method.modifiers().annotations().stream()
62+
.filter(a -> "org.springframework.web.bind.annotation.ResponseBody".equals(a.annotationType().symbolType().fullyQualifiedName()))
63+
.findFirst();
64+
methodAnnotation.ifPresent(annotationTree -> secondaryLocations.add(new JavaFileScannerContext.Location("Remove this \"@ResponseBody\" annotation.", annotationTree)));
65+
methodAnnotation.ifPresent(annotationTree -> edits.add(JavaTextEdit.removeTree(annotationTree)));
66+
});
67+
68+
classTree.modifiers().annotations().stream()
69+
.filter(a -> "org.springframework.web.bind.annotation.ResponseBody".equals(a.annotationType().symbolType().fullyQualifiedName()))
70+
.forEach(annotationTree -> secondaryLocations.add(new JavaFileScannerContext.Location("Remove this \"@ResponseBody\" annotation.", annotationTree)));
71+
72+
if (secondaryLocations.isEmpty()) {
73+
return;
74+
}
75+
76+
QuickFixHelper.newIssue(context)
77+
.forRule(this)
78+
.onTree(annotation.get())
79+
.withMessage("Replace the \"@Controller\" annotation by \"@RestController\" and remove all \"@ResponseBody\" annotations.")
80+
.withSecondaries(secondaryLocations)
81+
.withQuickFixes(() -> List.of(JavaQuickFix.newQuickFix("Remove \"@ResponseBody\" annotations.").addTextEdits(edits).build(),
82+
JavaQuickFix.newQuickFix("Replace \"@Controller\" by \"@RestController\".").addTextEdit(JavaTextEdit.replaceTree(annotation.get(), "@RestController")).build()))
83+
.report();
84+
85+
}
86+
87+
}

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6833.html‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>Classes annotated as <code>@Controller</code> in Spring are responsible for handling incoming web requests. When annotating methods or the entire
3+
controller with <code>@ResponseBody</code>, the return value of said methods will be serialized and set as the response body. In other words, it tells
4+
the Spring framework that this method does not produce a view. This mechanism is commonly used to create API endpoints.</p>
5+
<p>Spring provides <code>@RestController</code> as a convenient annotation to replace the combination of <code>@Controller</code> and
6+
<code>@ResponseBody</code>. The two are functionally identical, so the single annotation approach is preferred.</p>
7+
<p>This rule will raise an issue on a class that is annotated with <code>@Controller</code> if:</p>
8+
<ul>
9+
<li> the class is also annotated with <code>@ResponseBody</code> or </li>
10+
<li> all methods in said class are annotated with <code>@ResponseBody</code>. </li>
11+
</ul>
12+
<h2>How to fix it</h2>
13+
<p>Replace the <code>@Controller</code> annotation with the <code>@RestController</code> annotation and remove all <code>@ResponseBody</code>
14+
annotations from the class and its methods.</p>
15+
<h3>Code examples</h3>
16+
<h4>Noncompliant code example</h4>
17+
<pre data-diff-id="1" data-diff-type="noncompliant">
18+
@Controller
19+
@ResponseBody
20+
public class MyController {
21+
@GetMapping("/hello")
22+
public String hello() {
23+
return "Hello World!";
24+
}
25+
}
26+
</pre>
27+
<h4>Compliant solution</h4>
28+
<pre data-diff-id="1" data-diff-type="compliant">
29+
@RestController
30+
public class MyController {
31+
@GetMapping("/hello")
32+
public String hello() {
33+
return "Hello World!";
34+
}
35+
}
36+
</pre>
37+
<h4>Noncompliant code example</h4>
38+
<pre data-diff-id="2" data-diff-type="noncompliant">
39+
@Controller
40+
public class MyController {
41+
@ResponseBody
42+
@GetMapping("/hello")
43+
public String hello() {
44+
return "Hello World!";
45+
}
46+
47+
@ResponseBody
48+
@GetMapping("/foo")
49+
public String foo() {
50+
return "Foo";
51+
}
52+
}
53+
</pre>
54+
<h4>Compliant solution</h4>
55+
<pre data-diff-id="2" data-diff-type="compliant">
56+
@RestController
57+
public class MyController {
58+
@GetMapping("/hello")
59+
public String hello() {
60+
return "Hello World!";
61+
}
62+
63+
@GetMapping("/foo")
64+
public String foo() {
65+
return "Foo";
66+
}
67+
}
68+
</pre>
69+
<h2>Resources</h2>
70+
<h3>Articles &amp; blog posts</h3>
71+
<ul>
72+
<li> Spring Guides - <a href="https://spring.io/guides/gs/rest-service/">Building a RESTful Web Service</a> </li>
73+
<li> Baeldung - <a href="https://www.baeldung.com/spring-controller-vs-restcontroller">The Spring @Controller and @RestController Annotations</a>
74+
</li>
75+
<li> Baeldung - <a href="https://www.baeldung.com/spring-request-response-body">Spring’s RequestBody and ResponseBody Annotations</a> </li>
76+
</ul>
77+

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6833.json‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"title": "\"@Controller\" should be replaced with \"@RestController\"",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"spring"
11+
],
12+
"defaultSeverity": "Major",
13+
"ruleSpecification": "RSPEC-6833",
14+
"sqKey": "S6833",
15+
"scope": "All",
16+
"quickfix": "covered",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "MEDIUM"
20+
},
21+
"attribute": "CONVENTIONAL"
22+
}
23+
}

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@
491491
"S6829",
492492
"S6830",
493493
"S6831",
494+
"S6833",
494495
"S6837"
495496
]
496497
}

‎java-checks/src/test/java/org/sonar/java/checks/spring/ControllerWithRestControllerReplacementCheckTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2023 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.checks.spring;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.sonar.java.checks.verifier.internal.InternalCheckVerifier;
24+
25+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
26+
27+
class ControllerWithRestControllerReplacementCheckTest {
28+
29+
@Test
30+
void test() {
31+
InternalCheckVerifier.newInstance()
32+
.onFile(mainCodeSourcesPath("checks/spring/ControllerWithRestControllerReplacementCheck.java"))
33+
.withCheck(new ControllerWithRestControllerReplacementCheck())
34+
.withQuickFixes()
35+
.verifyIssues();
36+
}
37+
}

0 commit comments

Comments
 (0)