Skip to content

Commit

Permalink
SONARJAVA-4727 S6862: Beans in "@configuration" class should have dif…
Browse files Browse the repository at this point in the history
…ferent names (#4599)
  • Loading branch information
irina-batinic-sonarsource authored Dec 5, 2023
1 parent 7688424 commit d44fbed
Show file tree
Hide file tree
Showing 9 changed files with 299 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -2938,5 +2938,11 @@
"hasTruePositives": false,
"falseNegatives": 2,
"falsePositives": 0
},
{
"ruleKey": "S6862",
"hasTruePositives": false,
"falseNegatives": 5,
"falsePositives": 0
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ruleKey": "S6862",
"hasTruePositives": false,
"falseNegatives": 5,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package checks;

import javax.annotation.Nullable;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

public class ConfigurationBeanNamesCheckSample {

class User {
}

@Configuration
class Config1 {
@Bean
public User user() {
return new User();
}

@Bean
public User user(String name) { // Noncompliant [[sc=17;ec=21]] {{Rename this bean method to prevent any conflict with other beans.}}
return new User();
}
}

@Configuration
class Config2 {
@Bean
public User user() {
return new User();
}

@Bean
public User userWithName(String name) { // Compliant
return new User();
}
}

@Configuration
class Config3 {
@Bean
public User user() { // Compliant
return new User();
}
}

@Configuration
class Config4 {
@Bean
public User user() { // Compliant
return new User();
}

@Nullable
public User user(String name) {
return new User();
}
}

@Configuration
class Config5 {
@Bean
public User user() {
return new User();
}

@Bean
public User userWithName(String name) { // Compliant
return new User();
}

@Bean
public User user(String name, String password) { // Noncompliant
return new User();
}

@Bean
public User user(String name, String password, boolean enabled) { // Noncompliant
return new User();
}
}

@Configuration
class Config6 {
}

@Configuration
class Config7 {
@Bean
public User user1() {
return new User();
}

@Bean
public User user1(String name) { // Noncompliant
return new User();
}

@Bean
public User user2() {
return new User();
}

@Bean
public User user2(String name) { // Noncompliant
return new User();
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* SonarQube Java
* Copyright (C) 2012-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.java.checks;

import java.util.HashSet;
import java.util.List;
import java.util.stream.Collectors;
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.tree.ClassTree;
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.Tree;

@Rule(key = "S6862")
public class ConfigurationBeanNamesCheck extends IssuableSubscriptionVisitor {

@Override
public List<Tree.Kind> nodesToVisit() {
return List.of(Tree.Kind.CLASS);
}

@Override
public void visitNode(Tree tree) {
var classTree = (ClassTree) tree;
if (!isConfigurationClass(classTree)) {
return;
}

var beanMethods = getBeanMethods(classTree);
var foundNames = new HashSet<String>();
for (MethodTree beanMethod : beanMethods) {
if (!foundNames.add(beanMethod.simpleName().name())) {
reportIssue(beanMethod.simpleName(), "Rename this bean method to prevent any conflict with other beans.");
}
}
}

private static boolean isConfigurationClass(ClassTree classTree) {
return classTree.symbol().metadata().isAnnotatedWith("org.springframework.context.annotation.Configuration");
}

private static List<MethodTree> getBeanMethods(ClassTree classTree) {
return classTree.members().stream()
.filter(member -> member.is(Tree.Kind.METHOD))
.map(MethodTree.class::cast)
.filter(method -> method.symbol().metadata().isAnnotatedWith("org.springframework.context.annotation.Bean"))
.collect(Collectors.toList());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* SonarQube Java
* Copyright (C) 2012-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.java.checks;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;
import org.sonar.java.checks.verifier.TestUtils;

class ConfigurationBeanNamesCheckTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile(TestUtils.mainCodeSourcesPath("checks/ConfigurationBeanNamesCheckSample.java"))
.withCheck(new ConfigurationBeanNamesCheck())
.verifyIssues();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
import org.sonar.java.checks.CompareToReturnValueCheck;
import org.sonar.java.checks.ConcatenationWithStringValueOfCheck;
import org.sonar.java.checks.ConditionalOnNewLineCheck;
import org.sonar.java.checks.ConfigurationBeanNamesCheck;
import org.sonar.java.checks.ConfusingOverloadCheck;
import org.sonar.java.checks.ConfusingVarargCheck;
import org.sonar.java.checks.ConstantMathCheck;
Expand Down Expand Up @@ -738,6 +739,7 @@ public final class CheckList {
BooleanMethodNameCheck.class,
BooleanMethodReturnCheck.class,
BrainMethodCheck.class,
ConfigurationBeanNamesCheck.class,
CORSCheck.class,
CallOuterPrivateMethodCheck.class,
CallSuperMethodFromInnerClassCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<h2>Why is this an issue?</h2>
<p>Naming conventions play a crucial role in maintaining code clarity and readability. The uniqueness of bean names in Spring configurations is vital
to the clarity and readability of the code. When two beans share the same name within a configuration, it is not obvious to the reader which bean is
being referred to. This leads to potential misunderstandings and errors.</p>
<h2>How to fix it</h2>
<p>To address this issue, ensure each bean within a configuration has a distinct and meaningful name. Choose names that accurately represent the
purpose or functionality of the bean.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
@Configuration
class Config {
@Bean
public User user() {
return currentUser();
}
@Bean
public User user(AuthService auth) { // Noncompliant
return auth.user();
}
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
@Configuration
class Config {
@Bean
public User user() {
return currentUser();
}
@Bean
public User userFromAuth(AuthService auth) {
return auth.user();
}
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://docs.spring.io/spring-framework/reference/core/beans/java/basic-concepts.html">Spring IO - Basic concepts: @Bean and
@Configuration</a> </li>
<li> <a href="https://docs.spring.io/spring-framework/reference/core/beans/java/configuration-annotation.html">Spring IO - Using the @Configuration
annotation</a> </li>
<li> <a href="https://docs.spring.io/spring-framework/reference/core/beans/java/bean-annotation.html">Spring IO - Using the @Bean annotation</a>
</li>
</ul>

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "Beans in \"@Configuration\" class should have different names",
"type": "BUG",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"spring"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-6862",
"sqKey": "S6862",
"scope": "Main",
"quickfix": "unknown",
"code": {
"impacts": {
"RELIABILITY": "MEDIUM"
},
"attribute": "CLEAR"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@
"S6832",
"S6833",
"S6837",
"S6856"
"S6856",
"S6862"
]
}

0 comments on commit d44fbed

Please sign in to comment.