Skip to content

Commit

Permalink
SONARJAVA-4681: Implement rule S6832: Non-singleton Spring beans shou…
Browse files Browse the repository at this point in the history
…ld not be injected in a Singleton bean (#4511)
  • Loading branch information
ADarko22 authored Nov 6, 2023
1 parent d2b338d commit 3618545
Show file tree
Hide file tree
Showing 11 changed files with 678 additions and 5 deletions.
4 changes: 2 additions & 2 deletions its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,15 @@ public void javaCheckTestSources() throws Exception {
softly.assertThat(newTotal).isEqualTo(knownTotal);
softly.assertThat(rulesCausingFPs).hasSize(6);
softly.assertThat(rulesNotReporting).hasSize(7);
softly.assertThat(rulesSilenced).hasSize(80);
softly.assertThat(rulesSilenced).hasSize(81);

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

softly.assertAll();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2900,7 +2900,7 @@
{
"ruleKey": "S6813",
"hasTruePositives": true,
"falseNegatives": 33,
"falseNegatives": 55,
"falsePositives": 0
},
{
Expand All @@ -2918,7 +2918,7 @@
{
"ruleKey": "S6818",
"hasTruePositives": false,
"falseNegatives": 4,
"falseNegatives": 7,
"falsePositives": 0
},
{
Expand All @@ -2939,6 +2939,12 @@
"falseNegatives": 6,
"falsePositives": 0
},
{
"ruleKey": "S6832",
"hasTruePositives": false,
"falseNegatives": 41,
"falsePositives": 0
},
{
"ruleKey": "S6833",
"hasTruePositives": false,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
package checks.spring;

import javax.inject.Inject;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Scope;
import org.springframework.context.annotation.ScopedProxyMode;
import org.springframework.stereotype.Component;
import org.springframework.web.context.WebApplicationContext;

import static org.springframework.context.annotation.ScopedProxyMode.TARGET_CLASS;

public class NonSingletonAutowiredInSingletonCheckSample {

private static final String SINGLETON_SCOPE = "singleton";
private static final String PROTOTYPE_SCOPE = "prototype";
private static final String CUSTOM_SCOPE = "custom";

@Component
@Scope(value = WebApplicationContext.SCOPE_REQUEST, proxyMode = TARGET_CLASS)
public class RequestBean1 { }

@Scope("prototype")
public class PrototypeBean1 { }

@Scope(value = "prototype")
public class PrototypeBean2 { }


@Scope(value = PROTOTYPE_SCOPE, scopeName = "prototype", proxyMode = ScopedProxyMode.TARGET_CLASS)
public class PrototypeBean3 { }


public class SingletonBean {
@Autowired
private RequestBean1 requestBean1; // Noncompliant, [[sc=13;ec=25]] {{Don't auto-wire this non-Singleton bean into a Singleton bean (autowired field).}}
@Autowired
private PrototypeBean1 prototypeBean1; // Noncompliant
@Autowired
private PrototypeBean2 prototypeBean2; // Noncompliant
@Autowired
private PrototypeBean3 prototypeBean3; // Noncompliant

@Autowired // Noncompliant@+1
public SingletonBean(RequestBean1 requestBean1) { // Noncompliant, [[sc=26;ec=38]] {{Don't auto-wire this non-Singleton bean into a Singleton bean (autowired constructor).}}
}

@Autowired // Noncompliant@+1
public SingletonBean(PrototypeBean2 prototypeBean2) { // Noncompliant
}

@Autowired
public SingletonBean(RequestBean1 requestBean1, // Noncompliant
PrototypeBean1 prototypeBean1, // Noncompliant
PrototypeBean2 prototypeBean2, // Noncompliant
PrototypeBean3 prototypeBean3) { // Noncompliant
}

@Autowired
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).}}
this.requestBean1 = requestBean1;
}

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).}}
this.prototypeBean1 = prototypeBean1;
}

@Autowired
private void setPrototypeBean2(@Autowired PrototypeBean2 prototypeBean2) { // Compliant
this.prototypeBean2 = prototypeBean2;
}

public void method(@Autowired RequestBean1 requestBean1) { // Compliant, not a setter or constructor
// ...
}
}

@Scope(value = "singleton")
public class SingletonBean2 {
@Autowired
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
@Autowired
private RequestBean1 requestBean1; // Noncompliant
@Autowired
private PrototypeBean1 prototypeBean1; // Noncompliant

@Autowired
private SingletonBean2(@Autowired SingletonBean singletonBean) {// Compliant, since scope is non-Singleton
this.singletonBean = singletonBean;
}

@Autowired // Noncompliant@+1
public SingletonBean2(RequestBean1 requestBean1) { // Noncompliant
this.requestBean1 = requestBean1;
}
}

@Scope(scopeName = "singleton")
public class SingletonBean3 {
@Autowired
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
@Autowired
private RequestBean1 requestBean1; // Noncompliant
@Autowired
private PrototypeBean1 prototypeBean1; // Noncompliant

// Noncompliant@+2 Autowired constructor
@Autowired // Noncompliant@+1 Autowired constructor parameter
public SingletonBean3(@Autowired RequestBean1 requestBean1) { // Noncompliant, single parameter constructor
}
}

@Scope(proxyMode = TARGET_CLASS)
public class SingletonBean4 {
@Autowired
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
@Autowired
private RequestBean1 requestBean1; // Noncompliant
@Autowired
private PrototypeBean1 prototypeBean1; // Noncompliant
}

@Scope(value = SINGLETON_SCOPE, scopeName = "singleton")
public class SingletonBean5 {
@Autowired
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
@Autowired
private RequestBean1 requestBean1; // Noncompliant
@Autowired
private PrototypeBean1 prototypeBean1; // Noncompliant
}

@Scope("singleton")
public class SingletonBean6 {
@Inject
private SingletonBean5 singletonBean5; // Compliant, since scope is non-Singleton
@Inject
private RequestBean1 requestBean1; // Noncompliant
private PrototypeBean1 prototypeBean1;
private PrototypeBean2 prototypeBean2;

public SingletonBean6(PrototypeBean1 prototypeBean1) { // Noncompliant
this.prototypeBean1 = prototypeBean1;
}

@Inject
public SingletonBean6(PrototypeBean1 prototypeBean1, // Noncompliant
PrototypeBean2 prototypeBean2) { // Noncompliant
this.prototypeBean1 = prototypeBean1;
this.prototypeBean2 = prototypeBean2;
}

@Inject
public void setPrototypeBean2(PrototypeBean2 prototypeBean2) { // Noncompliant
this.prototypeBean2 = prototypeBean2;
}
}

public class SingletonBean7 {
@jakarta.inject.Inject
private SingletonBean5 singletonBean5; // Compliant, since scope is non-Singleton
@jakarta.inject.Inject
private RequestBean1 requestBean1; // Noncompliant
private PrototypeBean1 prototypeBean1;
private PrototypeBean2 prototypeBean2;

public SingletonBean7(PrototypeBean1 prototypeBean1) { // Noncompliant
this.prototypeBean1 = prototypeBean1;
}

@jakarta.inject.Inject
public SingletonBean7(PrototypeBean1 prototypeBean1, // Noncompliant
PrototypeBean2 prototypeBean2) { // Noncompliant
this.prototypeBean1 = prototypeBean1;
this.prototypeBean2 = prototypeBean2;
}

@jakarta.inject.Inject
public void setPrototypeBean2(PrototypeBean2 prototypeBean2) { // Noncompliant
this.prototypeBean2 = prototypeBean2;
}
}

@Scope(value = CUSTOM_SCOPE, scopeName = "custom", proxyMode = TARGET_CLASS)
public class CustomBean {
@Autowired
private SingletonBean singletonBean; // Compliant, since scope is non-Singleton
@Autowired
private SingletonBean5 singletonBean5; // Compliant, since scope is non-Singleton
@Autowired
public PrototypeBean1 prototypeBean1; // Compliant, since scope is non-Singleton
}

public class SingletonBean11 {
@Autowired
private CustomBean customBean; // Noncompliant

@Autowired // Noncompliant@+1
public SingletonBean11(CustomBean customBean) { // Noncompliant
this.customBean = customBean;
}

@Autowired
public void setCustomBean(CustomBean customBean) { // Noncompliant
this.customBean = customBean;
}

@Autowired
public void method(CustomBean customBean) { // Compliant, not a setter or constructor
// ...
}

public void method2(@Autowired CustomBean customBean) { // Compliant, not a setter or constructor
// ...
}
}

@Autowired
public @interface customAutowired {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
import org.sonar.java.checks.spring.NonSingletonAutowiredInSingletonCheck;
import org.sonar.java.checks.spring.OptionalRestParametersShouldBeObjectsCheck;
import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
Expand Down Expand Up @@ -516,6 +517,7 @@ public final class CheckList {
NestedTernaryOperatorsCheck.class,
NioFileDeleteCheck.class,
NoCheckstyleTagPresenceCheck.class,
NonSingletonAutowiredInSingletonCheck.class,
NoPmdTagPresenceCheck.class,
NoSonarCheck.class,
NonSerializableWriteCheck.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,19 @@ public static boolean isHashCodeMethod(MethodTree m) {
return isPublic(m) && !isStatic(m) && hasHashCodeSignature;
}

public static boolean isSetterMethod(MethodTree m) {
boolean publicNonStaticVoid = isPublic(m) && !isStatic(m) && returnsPrimitive(m, "void");
return publicNonStaticVoid && isNamePrefix(m, "set") && m.parameters().size() == 1;
}

private static boolean isNamed(MethodTree m, String name) {
return name.equals(m.simpleName().name());
}

private static boolean isNamePrefix(MethodTree m, String prefix) {
return m.simpleName().name().startsWith(prefix);
}

private static boolean isStatic(MethodTree m) {
return ModifiersUtils.hasModifier(m.modifiers(), Modifier.STATIC);
}
Expand Down Expand Up @@ -135,7 +144,7 @@ public static Optional<MethodInvocationTree> subsequentMethodInvocation(Tree tre

@VisibleForTesting
static boolean hasKind(@Nullable Tree tree, Tree.Kind kind) {
return tree != null && tree.kind() == kind;
return tree != null && tree.kind() == kind;
}

public static class MethodInvocationCollector extends BaseTreeVisitor {
Expand Down
Loading

0 comments on commit 3618545

Please sign in to comment.