Skip to content

Commit 3618545

Browse files
author
Angelo Buono
authored
SONARJAVA-4681: Implement rule S6832: Non-singleton Spring beans should not be injected in a Singleton bean (#4511)
1 parent d2b338d commit 3618545

11 files changed

Lines changed: 678 additions & 5 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(80);
183+
softly.assertThat(rulesSilenced).hasSize(81);
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: 3513");
191+
softly.assertThat(differences).isEqualTo("Issues differences: 3579");
192192

193193
softly.assertAll();
194194
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2900,7 +2900,7 @@
29002900
{
29012901
"ruleKey": "S6813",
29022902
"hasTruePositives": true,
2903-
"falseNegatives": 33,
2903+
"falseNegatives": 55,
29042904
"falsePositives": 0
29052905
},
29062906
{
@@ -2918,7 +2918,7 @@
29182918
{
29192919
"ruleKey": "S6818",
29202920
"hasTruePositives": false,
2921-
"falseNegatives": 4,
2921+
"falseNegatives": 7,
29222922
"falsePositives": 0
29232923
},
29242924
{
@@ -2939,6 +2939,12 @@
29392939
"falseNegatives": 6,
29402940
"falsePositives": 0
29412941
},
2942+
{
2943+
"ruleKey": "S6832",
2944+
"hasTruePositives": false,
2945+
"falseNegatives": 41,
2946+
"falsePositives": 0
2947+
},
29422948
{
29432949
"ruleKey": "S6833",
29442950
"hasTruePositives": false,

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

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
package checks.spring;
2+
3+
import javax.inject.Inject;
4+
import org.springframework.beans.factory.annotation.Autowired;
5+
import org.springframework.context.annotation.Scope;
6+
import org.springframework.context.annotation.ScopedProxyMode;
7+
import org.springframework.stereotype.Component;
8+
import org.springframework.web.context.WebApplicationContext;
9+
10+
import static org.springframework.context.annotation.ScopedProxyMode.TARGET_CLASS;
11+
12+
public class NonSingletonAutowiredInSingletonCheckSample {
13+
14+
private static final String SINGLETON_SCOPE = "singleton";
15+
private static final String PROTOTYPE_SCOPE = "prototype";
16+
private static final String CUSTOM_SCOPE = "custom";
17+
18+
@Component
19+
@Scope(value = WebApplicationContext.SCOPE_REQUEST, proxyMode = TARGET_CLASS)
20+
public class RequestBean1 { }
21+
22+
@Scope("prototype")
23+
public class PrototypeBean1 { }
24+
25+
@Scope(value = "prototype")
26+
public class PrototypeBean2 { }
27+
28+
29+
@Scope(value = PROTOTYPE_SCOPE, scopeName = "prototype", proxyMode = ScopedProxyMode.TARGET_CLASS)
30+
public class PrototypeBean3 { }
31+
32+
33+
public class SingletonBean {
34+
@Autowired
35+
private RequestBean1 requestBean1; // Noncompliant, [[sc=13;ec=25]] {{Don't auto-wire this non-Singleton bean into a Singleton bean (autowired field).}}
36+
@Autowired
37+
private PrototypeBean1 prototypeBean1; // Noncompliant
38+
@Autowired
39+
private PrototypeBean2 prototypeBean2; // Noncompliant
40+
@Autowired
41+
private PrototypeBean3 prototypeBean3; // Noncompliant
42+
43+
@Autowired // Noncompliant@+1
44+
public SingletonBean(RequestBean1 requestBean1) { // Noncompliant, [[sc=26;ec=38]] {{Don't auto-wire this non-Singleton bean into a Singleton bean (autowired constructor).}}
45+
}
46+
47+
@Autowired // Noncompliant@+1
48+
public SingletonBean(PrototypeBean2 prototypeBean2) { // Noncompliant
49+
}
50+
51+
@Autowired
52+
public SingletonBean(RequestBean1 requestBean1, // Noncompliant
53+
PrototypeBean1 prototypeBean1, // Noncompliant
54+
PrototypeBean2 prototypeBean2, // Noncompliant
55+
PrototypeBean3 prototypeBean3) { // Noncompliant
56+
}
57+
58+
@Autowired
59+
public void setRequestBean1(RequestBean1 requestBean1) { // Noncompliant, [[sc=33;ec=45]] {{Don't auto-wire this non-Singleton bean into a Singleton bean (autowired setter method).}}
60+
this.requestBean1 = requestBean1;
61+
}
62+
63+
public void setPrototypeBean1(@Autowired PrototypeBean1 prototypeBean1) { // Noncompliant, [[sc=46;ec=60]] {{Don't auto-wire this non-Singleton bean into a Singleton bean (autowired parameter).}}
64+
this.prototypeBean1 = prototypeBean1;
65+
}
66+
67+
@Autowired
68+
private void setPrototypeBean2(@Autowired PrototypeBean2 prototypeBean2) { // Compliant
69+
this.prototypeBean2 = prototypeBean2;
70+
}
71+
72+
public void method(@Autowired RequestBean1 requestBean1) { // Compliant, not a setter or constructor
73+
// ...
74+
}
75+
}
76+
77+
@Scope(value = "singleton")
78+
public class SingletonBean2 {
79+
@Autowired
80+
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
81+
@Autowired
82+
private RequestBean1 requestBean1; // Noncompliant
83+
@Autowired
84+
private PrototypeBean1 prototypeBean1; // Noncompliant
85+
86+
@Autowired
87+
private SingletonBean2(@Autowired SingletonBean singletonBean) {// Compliant, since scope is non-Singleton
88+
this.singletonBean = singletonBean;
89+
}
90+
91+
@Autowired // Noncompliant@+1
92+
public SingletonBean2(RequestBean1 requestBean1) { // Noncompliant
93+
this.requestBean1 = requestBean1;
94+
}
95+
}
96+
97+
@Scope(scopeName = "singleton")
98+
public class SingletonBean3 {
99+
@Autowired
100+
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
101+
@Autowired
102+
private RequestBean1 requestBean1; // Noncompliant
103+
@Autowired
104+
private PrototypeBean1 prototypeBean1; // Noncompliant
105+
106+
// Noncompliant@+2 Autowired constructor
107+
@Autowired // Noncompliant@+1 Autowired constructor parameter
108+
public SingletonBean3(@Autowired RequestBean1 requestBean1) { // Noncompliant, single parameter constructor
109+
}
110+
}
111+
112+
@Scope(proxyMode = TARGET_CLASS)
113+
public class SingletonBean4 {
114+
@Autowired
115+
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
116+
@Autowired
117+
private RequestBean1 requestBean1; // Noncompliant
118+
@Autowired
119+
private PrototypeBean1 prototypeBean1; // Noncompliant
120+
}
121+
122+
@Scope(value = SINGLETON_SCOPE, scopeName = "singleton")
123+
public class SingletonBean5 {
124+
@Autowired
125+
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
126+
@Autowired
127+
private RequestBean1 requestBean1; // Noncompliant
128+
@Autowired
129+
private PrototypeBean1 prototypeBean1; // Noncompliant
130+
}
131+
132+
@Scope("singleton")
133+
public class SingletonBean6 {
134+
@Inject
135+
private SingletonBean5 singletonBean5; // Compliant, since scope is non-Singleton
136+
@Inject
137+
private RequestBean1 requestBean1; // Noncompliant
138+
private PrototypeBean1 prototypeBean1;
139+
private PrototypeBean2 prototypeBean2;
140+
141+
public SingletonBean6(PrototypeBean1 prototypeBean1) { // Noncompliant
142+
this.prototypeBean1 = prototypeBean1;
143+
}
144+
145+
@Inject
146+
public SingletonBean6(PrototypeBean1 prototypeBean1, // Noncompliant
147+
PrototypeBean2 prototypeBean2) { // Noncompliant
148+
this.prototypeBean1 = prototypeBean1;
149+
this.prototypeBean2 = prototypeBean2;
150+
}
151+
152+
@Inject
153+
public void setPrototypeBean2(PrototypeBean2 prototypeBean2) { // Noncompliant
154+
this.prototypeBean2 = prototypeBean2;
155+
}
156+
}
157+
158+
public class SingletonBean7 {
159+
@jakarta.inject.Inject
160+
private SingletonBean5 singletonBean5; // Compliant, since scope is non-Singleton
161+
@jakarta.inject.Inject
162+
private RequestBean1 requestBean1; // Noncompliant
163+
private PrototypeBean1 prototypeBean1;
164+
private PrototypeBean2 prototypeBean2;
165+
166+
public SingletonBean7(PrototypeBean1 prototypeBean1) { // Noncompliant
167+
this.prototypeBean1 = prototypeBean1;
168+
}
169+
170+
@jakarta.inject.Inject
171+
public SingletonBean7(PrototypeBean1 prototypeBean1, // Noncompliant
172+
PrototypeBean2 prototypeBean2) { // Noncompliant
173+
this.prototypeBean1 = prototypeBean1;
174+
this.prototypeBean2 = prototypeBean2;
175+
}
176+
177+
@jakarta.inject.Inject
178+
public void setPrototypeBean2(PrototypeBean2 prototypeBean2) { // Noncompliant
179+
this.prototypeBean2 = prototypeBean2;
180+
}
181+
}
182+
183+
@Scope(value = CUSTOM_SCOPE, scopeName = "custom", proxyMode = TARGET_CLASS)
184+
public class CustomBean {
185+
@Autowired
186+
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
187+
@Autowired
188+
private SingletonBean5 singletonBean5; // Compliant, since scope is non-Singleton
189+
@Autowired
190+
public PrototypeBean1 prototypeBean1; // Compliant, since scope is non-Singleton
191+
}
192+
193+
public class SingletonBean11 {
194+
@Autowired
195+
private CustomBean customBean; // Noncompliant
196+
197+
@Autowired // Noncompliant@+1
198+
public SingletonBean11(CustomBean customBean) { // Noncompliant
199+
this.customBean = customBean;
200+
}
201+
202+
@Autowired
203+
public void setCustomBean(CustomBean customBean) { // Noncompliant
204+
this.customBean = customBean;
205+
}
206+
207+
@Autowired
208+
public void method(CustomBean customBean) { // Compliant, not a setter or constructor
209+
// ...
210+
}
211+
212+
public void method2(@Autowired CustomBean customBean) { // Compliant, not a setter or constructor
213+
// ...
214+
}
215+
}
216+
217+
@Autowired
218+
public @interface customAutowired {
219+
}
220+
}

‎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
@@ -145,6 +145,7 @@
145145
import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
146146
import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
147147
import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
148+
import org.sonar.java.checks.spring.NonSingletonAutowiredInSingletonCheck;
148149
import org.sonar.java.checks.spring.OptionalRestParametersShouldBeObjectsCheck;
149150
import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
150151
import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
@@ -516,6 +517,7 @@ public final class CheckList {
516517
NestedTernaryOperatorsCheck.class,
517518
NioFileDeleteCheck.class,
518519
NoCheckstyleTagPresenceCheck.class,
520+
NonSingletonAutowiredInSingletonCheck.class,
519521
NoPmdTagPresenceCheck.class,
520522
NoSonarCheck.class,
521523
NonSerializableWriteCheck.class,

‎java-checks/src/main/java/org/sonar/java/checks/helpers/MethodTreeUtils.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,19 @@ public static boolean isHashCodeMethod(MethodTree m) {
8080
return isPublic(m) && !isStatic(m) && hasHashCodeSignature;
8181
}
8282

83+
public static boolean isSetterMethod(MethodTree m) {
84+
boolean publicNonStaticVoid = isPublic(m) && !isStatic(m) && returnsPrimitive(m, "void");
85+
return publicNonStaticVoid && isNamePrefix(m, "set") && m.parameters().size() == 1;
86+
}
87+
8388
private static boolean isNamed(MethodTree m, String name) {
8489
return name.equals(m.simpleName().name());
8590
}
8691

92+
private static boolean isNamePrefix(MethodTree m, String prefix) {
93+
return m.simpleName().name().startsWith(prefix);
94+
}
95+
8796
private static boolean isStatic(MethodTree m) {
8897
return ModifiersUtils.hasModifier(m.modifiers(), Modifier.STATIC);
8998
}
@@ -135,7 +144,7 @@ public static Optional<MethodInvocationTree> subsequentMethodInvocation(Tree tre
135144

136145
@VisibleForTesting
137146
static boolean hasKind(@Nullable Tree tree, Tree.Kind kind) {
138-
return tree != null && tree.kind() == kind;
147+
return tree != null && tree.kind() == kind;
139148
}
140149

141150
public static class MethodInvocationCollector extends BaseTreeVisitor {

0 commit comments

Comments
 (0)