Skip to content

Commit 0e3edb0

Browse files
johann-beleites-sonarsourcejohann-beleites-sonarsource
authored
SONARJAVA-4677 Implement S6830: Bean names should adhere to the namin… (#4516)
* SONARJAVA-4677 Implement S6830: Bean names should adhere to the naming conventions
1 parent 6b43b30 commit 0e3edb0

9 files changed

Lines changed: 359 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(78);
183+
softly.assertThat(rulesSilenced).hasSize(79);
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: 3489");
191+
softly.assertThat(differences).isEqualTo("Issues differences: 3508");
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
@@ -2927,6 +2927,12 @@
29272927
"falseNegatives": 5,
29282928
"falsePositives": 0
29292929
},
2930+
{
2931+
"ruleKey": "S6830",
2932+
"hasTruePositives": false,
2933+
"falseNegatives": 19,
2934+
"falsePositives": 0
2935+
},
29302936
{
29312937
"ruleKey": "S6831",
29322938
"hasTruePositives": false,

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package checks.spring;
2+
3+
4+
import org.springframework.beans.factory.annotation.Autowire;
5+
import org.springframework.beans.factory.annotation.Qualifier;
6+
import org.springframework.context.annotation.Bean;
7+
import org.springframework.context.annotation.Configuration;
8+
import org.springframework.stereotype.Component;
9+
import org.springframework.stereotype.Controller;
10+
import org.springframework.stereotype.Repository;
11+
import org.springframework.stereotype.Service;
12+
import org.springframework.web.bind.annotation.RestController;
13+
14+
class SomeTestClass {
15+
void NO_camel_case() {}
16+
}
17+
18+
@interface MyAnnotationWithValue {
19+
String value();
20+
}
21+
22+
public class SpringBeanNamingConventionCheckSample {
23+
24+
@Bean("my_bean") // Noncompliant [[sc=9;ec=18]] {{Rename this bean to match the regular expression '^[a-z][a-zA-Z0-9]*$'.}}
25+
SomeTestClass myBean1() {
26+
return null;
27+
}
28+
29+
@Bean(name = "MyBean") // Noncompliant
30+
SomeTestClass myBean2() {
31+
return null;
32+
}
33+
34+
@Bean(autowire = Autowire.NO, initMethod = "toString", name = "my_bean", destroyMethod = "toString") // Noncompliant [[sc=58;ec=74]]
35+
SomeTestClass myBean3() {
36+
return null;
37+
}
38+
39+
@Bean(initMethod = "NO_camel_case", value = "my_bean") // Noncompliant
40+
SomeTestClass myBean5() {
41+
return null;
42+
}
43+
44+
@Bean("1bean") // Noncompliant
45+
SomeTestClass myBean8() {
46+
return null;
47+
}
48+
49+
public final static String MY_BEAN = "my_bean";
50+
@Bean(MY_BEAN) // Noncompliant
51+
SomeTestClass myBean10() {
52+
return null;
53+
}
54+
55+
@Bean(initMethod = "NO_camel_case") // Compliant, we are only interested in the bean name
56+
SomeTestClass myBean4() {
57+
return null;
58+
}
59+
60+
@Bean(initMethod = "NO_camel_case", name = "myBean") // Compliant, name is camel-cased
61+
SomeTestClass myBean6() {
62+
return null;
63+
}
64+
65+
@Bean("a") // Compliant, name is camel-cased
66+
SomeTestClass myBean7() {
67+
return null;
68+
}
69+
70+
@Bean // Compliant, no name provided
71+
SomeTestClass myBean9() {
72+
return null;
73+
}
74+
75+
public final static String MY_BEAN2 = "myBean";
76+
@Bean(MY_BEAN2) // Compliant, name is camel-cased
77+
SomeTestClass myBean11() {
78+
return null;
79+
}
80+
}
81+
82+
@Configuration("my_config") // Noncompliant
83+
class MyConfig1 {}
84+
85+
@Configuration(value = "my_Config") // Noncompliant
86+
class MyConfig3 {}
87+
88+
@Configuration("myConfig") // Compliant
89+
class MyConfig2 {}
90+
91+
@Configuration(value = "myConfig") // Compliant
92+
class MyConfig4 {}
93+
94+
@Controller("my_controller") // Noncompliant
95+
class MyController1 {}
96+
97+
@Controller("myController") // Compliant
98+
class MyController2 {}
99+
100+
@Component("my_component") // Noncompliant
101+
class MyComponent1 {}
102+
103+
@Component("myComponent") // Compliant
104+
class MyComponent2 {}
105+
106+
@Qualifier("my_qualifier") // Noncompliant
107+
class MyQualifier1 {}
108+
109+
@Qualifier("myQualifier") // Compliant
110+
class MyQualifier2 {}
111+
112+
@Component("my_component") // Noncompliant
113+
@Qualifier("my_qualifier") // Noncompliant
114+
class MyComponentAndQualifier1 {}
115+
116+
@Repository("my_repository") // Noncompliant
117+
class MyRepository1 {}
118+
119+
@Repository("myRepository") // Compliant
120+
class MyRepository2 {}
121+
122+
@Service("my_service") // Noncompliant
123+
class MyService1 {}
124+
125+
@Service("myService") // Compliant
126+
class MyService2 {}
127+
128+
@RestController("my_rest_controller") // Noncompliant
129+
class MyRestController1 {}
130+
131+
@RestController("myRestController") // Compliant
132+
class MyRestController2 {}
133+
134+
@MyAnnotationWithValue("my_value") // Compliant, not an annotation of interest
135+
class MyValue1 {}

‎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
@@ -149,6 +149,7 @@
149149
import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
150150
import org.sonar.java.checks.spring.SpringAntMatcherOrderCheck;
151151
import org.sonar.java.checks.spring.SpringAutoConfigurationCheck;
152+
import org.sonar.java.checks.spring.SpringBeanNamingConventionCheck;
152153
import org.sonar.java.checks.spring.SpringBeansShouldBeAccessibleCheck;
153154
import org.sonar.java.checks.spring.SpringComponentWithNonAutowiredMembersCheck;
154155
import org.sonar.java.checks.spring.SpringComponentWithWrongScopeCheck;
@@ -616,6 +617,7 @@ public final class CheckList {
616617
SpecializedFunctionalInterfacesCheck.class,
617618
SpringAntMatcherOrderCheck.class,
618619
SpringAutoConfigurationCheck.class,
620+
SpringBeanNamingConventionCheck.class,
619621
SpringBeansShouldBeAccessibleCheck.class,
620622
SpringComponentWithNonAutowiredMembersCheck.class,
621623
SpringComponentWithWrongScopeCheck.class,

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
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.List;
23+
import java.util.Objects;
24+
import java.util.regex.Pattern;
25+
import javax.annotation.CheckForNull;
26+
import javax.annotation.Nullable;
27+
import org.sonar.check.Rule;
28+
import org.sonar.java.checks.helpers.ExpressionsHelper;
29+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
30+
import org.sonar.plugins.java.api.tree.AnnotationTree;
31+
import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
32+
import org.sonar.plugins.java.api.tree.ExpressionTree;
33+
import org.sonar.plugins.java.api.tree.IdentifierTree;
34+
import org.sonar.plugins.java.api.tree.Tree;
35+
36+
@Rule(key = "S6830")
37+
public class SpringBeanNamingConventionCheck extends IssuableSubscriptionVisitor {
38+
39+
private static final List<String> ANNOTATIONS_TO_CHECK = List.of(
40+
"org.springframework.beans.factory.annotation.Qualifier",
41+
"org.springframework.context.annotation.Bean",
42+
"org.springframework.context.annotation.Configuration",
43+
"org.springframework.stereotype.Controller",
44+
"org.springframework.stereotype.Component",
45+
"org.springframework.stereotype.Repository",
46+
"org.springframework.stereotype.Service",
47+
"org.springframework.web.bind.annotation.RestController");
48+
49+
private static final Pattern NAMING_CONVENTION = Pattern.compile("^[a-z][a-zA-Z0-9]*$");
50+
51+
@Override
52+
public List<Tree.Kind> nodesToVisit() {
53+
return List.of(Tree.Kind.ANNOTATION);
54+
}
55+
56+
@Override
57+
public void visitNode(Tree tree) {
58+
var annotation = (AnnotationTree) tree;
59+
ANNOTATIONS_TO_CHECK.stream().filter(a -> annotation.symbolType().is(a)).findFirst()
60+
.map(a -> getNoncompliantNameArgument(annotation))
61+
.ifPresent(n -> reportIssue(n, "Rename this bean to match the regular expression '" + NAMING_CONVENTION.pattern() + "'."));
62+
}
63+
64+
@CheckForNull
65+
private static ExpressionTree getNoncompliantNameArgument(AnnotationTree annotation) {
66+
return annotation.arguments().stream()
67+
.map(arg -> {
68+
if (breaksNamingConvention(getArgValue(arg))) {
69+
return arg;
70+
} else {
71+
return null;
72+
}
73+
}).filter(Objects::nonNull).findFirst().orElse(null);
74+
}
75+
76+
private static ExpressionTree getArgValue(ExpressionTree argument) {
77+
if (argument.is(Tree.Kind.ASSIGNMENT)) {
78+
var assignment = (AssignmentExpressionTree) argument;
79+
var argName = ((IdentifierTree) assignment.variable()).name();
80+
var argValue = assignment.expression();
81+
if (argName.equals("name") || argName.equals("value")) {
82+
return argValue;
83+
}
84+
} else {
85+
return argument;
86+
}
87+
return null;
88+
}
89+
90+
private static boolean breaksNamingConvention(@Nullable ExpressionTree nameTree) {
91+
if (nameTree == null) {
92+
return false;
93+
} else {
94+
var name = ExpressionsHelper.getConstantValueAsString(nameTree).value();
95+
return name != null && !NAMING_CONVENTION.matcher(name).matches();
96+
}
97+
}
98+
}

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>Consistent naming of beans is important for the readability and maintainability of the code. More precisely, according to the Spring
3+
documentation:</p>
4+
<pre>
5+
Naming beans consistently makes your configuration easier to read and understand, and if you are using Spring AOP it helps a lot when applying advice to a set of beans related by name.
6+
</pre>
7+
<p>Not following accepted conventions can introduce inconsistent naming, especially when multiple developers work on the same project, leading to
8+
technical debt.</p>
9+
<p>The spring documentation establishes a naming convention that consists of camel-cased names with a leading lowercase letter.</p>
10+
<p>This rule raises an issue when a bean name defined in one of the following annotations does not adhere to the naming convention:</p>
11+
<ul>
12+
<li> <code>@Bean</code> </li>
13+
<li> <code>@Configuration</code> </li>
14+
<li> <code>@Controller</code> </li>
15+
<li> <code>@Component</code> </li>
16+
<li> <code>@Qualifier</code> </li>
17+
<li> <code>@Repository</code> </li>
18+
<li> <code>@Service</code> </li>
19+
</ul>
20+
<h2>How to fix it</h2>
21+
<p>Change the bean’s name to adhere to the naming conventions. Names should be camel-cased and start with a lowercase letter, for example,
22+
<code>myBean</code>.</p>
23+
<h3>Code examples</h3>
24+
<h4>Noncompliant code example</h4>
25+
<pre data-diff-id="1" data-diff-type="noncompliant">
26+
@Bean(name = "MyBean") // Noncompliant, the first letter of the name should be lowercase
27+
public MyBean myBean() {
28+
...
29+
</pre>
30+
<h4>Compliant solution</h4>
31+
<pre data-diff-id="1" data-diff-type="compliant">
32+
@Bean(name = "myBean") // Compliant
33+
public MyBean myBean() {
34+
...
35+
</pre>
36+
<h4>Noncompliant code example</h4>
37+
<pre data-diff-id="2" data-diff-type="noncompliant">
38+
@Service("my_service") // Noncompliant, the name should be camel-cased
39+
public class MyService {
40+
...
41+
</pre>
42+
<h4>Compliant solution</h4>
43+
<pre data-diff-id="2" data-diff-type="compliant">
44+
@Service("myService") // Compliant
45+
public class MyService {
46+
...
47+
</pre>
48+
<h2>Resources</h2>
49+
<h3>Documentation</h3>
50+
<ul>
51+
<li> Spring Framework Documentation - <a href="https://docs.spring.io/spring-framework/docs/3.0.0.M4/reference/html/ch03s03.html">3.3 Bean
52+
overview</a> </li>
53+
</ul>
54+
<h3>Articles &amp; blog posts</h3>
55+
<ul>
56+
<li> Java Guides - <a href="https://www.javaguides.net/2019/03/spring-boot-best-practices.html">Spring Boot Best Practices</a> </li>
57+
</ul>
58+

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"title": "Bean names should adhere to the naming conventions",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"spring"
11+
],
12+
"defaultSeverity": "Minor",
13+
"ruleSpecification": "RSPEC-6830",
14+
"sqKey": "S6830",
15+
"scope": "All",
16+
"quickfix": "unknown",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "MEDIUM"
20+
},
21+
"attribute": "IDENTIFIABLE"
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
@@ -489,6 +489,7 @@
489489
"S6817",
490490
"S6818",
491491
"S6829",
492+
"S6830",
492493
"S6831",
493494
"S6837"
494495
]

0 commit comments

Comments
 (0)