SONARJAVA-4677 Implement S6830: Bean names should adhere to the namin… (

#4516)

* SONARJAVA-4677 Implement S6830: Bean names should adhere to the naming conventions

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

@@ -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(78);
+    softly.assertThat(rulesSilenced).hasSize(79);
 
     /**
      * 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: 3489");
+    softly.assertThat(differences).isEqualTo("Issues differences: 3508");
 
     softly.assertAll();
   }

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

@@ -2927,6 +2927,12 @@
     "falseNegatives": 5,
     "falsePositives": 0
   },
+  {
+    "ruleKey": "S6830",
+    "hasTruePositives": false,
+    "falseNegatives": 19,
+    "falsePositives": 0
+  },
   {
     "ruleKey": "S6831",
     "hasTruePositives": false,

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

@@ -0,0 +1,135 @@
+package checks.spring;
+
+
+import org.springframework.beans.factory.annotation.Autowire;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.stereotype.Component;
+import org.springframework.stereotype.Controller;
+import org.springframework.stereotype.Repository;
+import org.springframework.stereotype.Service;
+import org.springframework.web.bind.annotation.RestController;
+
+class SomeTestClass {
+  void NO_camel_case() {}
+}
+
+@interface MyAnnotationWithValue {
+  String value();
+}
+
+public class SpringBeanNamingConventionCheckSample {
+
+  @Bean("my_bean") // Noncompliant [[sc=9;ec=18]] {{Rename this bean to match the regular expression '^[a-z][a-zA-Z0-9]*$'.}}
+  SomeTestClass myBean1() {
+    return null;
+  }
+
+  @Bean(name = "MyBean") // Noncompliant
+  SomeTestClass myBean2() {
+    return null;
+  }
+
+  @Bean(autowire = Autowire.NO, initMethod = "toString", name = "my_bean", destroyMethod = "toString") // Noncompliant [[sc=58;ec=74]]
+  SomeTestClass myBean3() {
+    return null;
+  }
+
+  @Bean(initMethod = "NO_camel_case", value = "my_bean") // Noncompliant
+  SomeTestClass myBean5() {
+    return null;
+  }
+
+  @Bean("1bean") // Noncompliant
+  SomeTestClass myBean8() {
+    return null;
+  }
+
+  public final static String MY_BEAN = "my_bean";
+  @Bean(MY_BEAN) // Noncompliant
+  SomeTestClass myBean10() {
+    return null;
+  }
+
+  @Bean(initMethod = "NO_camel_case") // Compliant, we are only interested in the bean name
+  SomeTestClass myBean4() {
+    return null;
+  }
+
+  @Bean(initMethod = "NO_camel_case", name = "myBean") // Compliant, name is camel-cased
+  SomeTestClass myBean6() {
+    return null;
+  }
+
+  @Bean("a") // Compliant, name is camel-cased
+  SomeTestClass myBean7() {
+    return null;
+  }
+
+  @Bean // Compliant, no name provided
+  SomeTestClass myBean9() {
+    return null;
+  }
+
+  public final static String MY_BEAN2 = "myBean";
+  @Bean(MY_BEAN2) // Compliant, name is camel-cased
+  SomeTestClass myBean11() {
+    return null;
+  }
+}
+
+@Configuration("my_config") // Noncompliant
+class MyConfig1 {}
+
+@Configuration(value = "my_Config") // Noncompliant
+class MyConfig3 {}
+
+@Configuration("myConfig") // Compliant
+class MyConfig2 {}
+
+@Configuration(value = "myConfig") // Compliant
+class MyConfig4 {}
+
+@Controller("my_controller") // Noncompliant
+class MyController1 {}
+
+@Controller("myController") // Compliant
+class MyController2 {}
+
+@Component("my_component") // Noncompliant
+class MyComponent1 {}
+
+@Component("myComponent") // Compliant
+class MyComponent2 {}
+
+@Qualifier("my_qualifier") // Noncompliant
+class MyQualifier1 {}
+
+@Qualifier("myQualifier") // Compliant
+class MyQualifier2 {}
+
+@Component("my_component") // Noncompliant
+@Qualifier("my_qualifier") // Noncompliant
+class MyComponentAndQualifier1 {}
+
+@Repository("my_repository") // Noncompliant
+class MyRepository1 {}
+
+@Repository("myRepository") // Compliant
+class MyRepository2 {}
+
+@Service("my_service") // Noncompliant
+class MyService1 {}
+
+@Service("myService") // Compliant
+class MyService2 {}
+
+@RestController("my_rest_controller") // Noncompliant
+class MyRestController1 {}
+
+@RestController("myRestController") // Compliant
+class MyRestController2 {}
+
+@MyAnnotationWithValue("my_value") // Compliant, not an annotation of interest
+class MyValue1 {}

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

@@ -149,6 +149,7 @@
 import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
 import org.sonar.java.checks.spring.SpringAntMatcherOrderCheck;
 import org.sonar.java.checks.spring.SpringAutoConfigurationCheck;
+import org.sonar.java.checks.spring.SpringBeanNamingConventionCheck;
 import org.sonar.java.checks.spring.SpringBeansShouldBeAccessibleCheck;
 import org.sonar.java.checks.spring.SpringComponentWithNonAutowiredMembersCheck;
 import org.sonar.java.checks.spring.SpringComponentWithWrongScopeCheck;
@@ -616,6 +617,7 @@ public final class CheckList {
     SpecializedFunctionalInterfacesCheck.class,
     SpringAntMatcherOrderCheck.class,
     SpringAutoConfigurationCheck.class,
+    SpringBeanNamingConventionCheck.class,
     SpringBeansShouldBeAccessibleCheck.class,
     SpringComponentWithNonAutowiredMembersCheck.class,
     SpringComponentWithWrongScopeCheck.class,

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

@@ -0,0 +1,98 @@
+/*
+ * 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.spring;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.regex.Pattern;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.ExpressionsHelper;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.tree.AnnotationTree;
+import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.IdentifierTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S6830")
+public class SpringBeanNamingConventionCheck extends IssuableSubscriptionVisitor {
+
+  private static final List<String> ANNOTATIONS_TO_CHECK = List.of(
+    "org.springframework.beans.factory.annotation.Qualifier",
+    "org.springframework.context.annotation.Bean",
+    "org.springframework.context.annotation.Configuration",
+    "org.springframework.stereotype.Controller",
+    "org.springframework.stereotype.Component",
+    "org.springframework.stereotype.Repository",
+    "org.springframework.stereotype.Service",
+    "org.springframework.web.bind.annotation.RestController");
+
+  private static final Pattern NAMING_CONVENTION = Pattern.compile("^[a-z][a-zA-Z0-9]*$");
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.ANNOTATION);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    var annotation = (AnnotationTree) tree;
+    ANNOTATIONS_TO_CHECK.stream().filter(a -> annotation.symbolType().is(a)).findFirst()
+      .map(a -> getNoncompliantNameArgument(annotation))
+      .ifPresent(n -> reportIssue(n, "Rename this bean to match the regular expression '" + NAMING_CONVENTION.pattern() + "'."));
+  }
+
+  @CheckForNull
+  private static ExpressionTree getNoncompliantNameArgument(AnnotationTree annotation) {
+    return annotation.arguments().stream()
+      .map(arg -> {
+        if (breaksNamingConvention(getArgValue(arg))) {
+          return arg;
+        } else  {
+          return null;
+        }
+      }).filter(Objects::nonNull).findFirst().orElse(null);
+  }
+
+  private static ExpressionTree getArgValue(ExpressionTree argument) {
+    if (argument.is(Tree.Kind.ASSIGNMENT)) {
+      var assignment = (AssignmentExpressionTree) argument;
+      var argName = ((IdentifierTree) assignment.variable()).name();
+      var argValue = assignment.expression();
+      if (argName.equals("name") || argName.equals("value")) {
+        return argValue;
+      }
+    } else {
+      return argument;
+    }
+    return null;
+  }
+
+  private static boolean breaksNamingConvention(@Nullable ExpressionTree nameTree) {
+    if (nameTree == null) {
+      return false;
+    } else {
+      var name = ExpressionsHelper.getConstantValueAsString(nameTree).value();
+      return name != null && !NAMING_CONVENTION.matcher(name).matches();
+    }
+  }
+}

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

@@ -0,0 +1,58 @@
+<h2>Why is this an issue?</h2>
+<p>Consistent naming of beans is important for the readability and maintainability of the code. More precisely, according to the Spring
+documentation:</p>
+<pre>
+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.
+</pre>
+<p>Not following accepted conventions can introduce inconsistent naming, especially when multiple developers work on the same project, leading to
+technical debt.</p>
+<p>The spring documentation establishes a naming convention that consists of camel-cased names with a leading lowercase letter.</p>
+<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>
+<ul>
+  <li> <code>@Bean</code> </li>
+  <li> <code>@Configuration</code> </li>
+  <li> <code>@Controller</code> </li>
+  <li> <code>@Component</code> </li>
+  <li> <code>@Qualifier</code> </li>
+  <li> <code>@Repository</code> </li>
+  <li> <code>@Service</code> </li>
+</ul>
+<h2>How to fix it</h2>
+<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,
+<code>myBean</code>.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@Bean(name = "MyBean") // Noncompliant, the first letter of the name should be lowercase
+public MyBean myBean() {
+    ...
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+@Bean(name = "myBean") // Compliant
+public MyBean myBean() {
+    ...
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+@Service("my_service") // Noncompliant, the name should be camel-cased
+public class MyService {
+    ...
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+@Service("myService") // Compliant
+public class MyService {
+    ...
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Spring Framework Documentation - <a href="https://docs.spring.io/spring-framework/docs/3.0.0.M4/reference/html/ch03s03.html">3.3 Bean
+  overview</a> </li>
+</ul>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> Java Guides - <a href="https://www.javaguides.net/2019/03/spring-boot-best-practices.html">Spring Boot Best Practices</a> </li>
+</ul>
+

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

@@ -0,0 +1,23 @@
+{
+  "title": "Bean names should adhere to the naming conventions",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "spring"
+  ],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-6830",
+  "sqKey": "S6830",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "IDENTIFIABLE"
+  }
+}

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

@@ -489,6 +489,7 @@
     "S6817",
     "S6818",
     "S6829",
+    "S6830",
     "S6831",
     "S6837"
   ]