SONARJAVA-4678: mplement S6831: @qualifier should not be used on @bean …

…methods (#4518)

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

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

@@ -2880,15 +2880,15 @@
     "falsePositives": 0
   },
   {
-    "ruleKey": "S6809",
+    "ruleKey": "S6806",
     "hasTruePositives": false,
-    "falseNegatives": 94,
+    "falseNegatives": 10,
     "falsePositives": 0
   },
   {
-    "ruleKey": "S6806",
+    "ruleKey": "S6809",
     "hasTruePositives": false,
-    "falseNegatives": 10,
+    "falseNegatives": 94,
     "falsePositives": 0
   },
   {
@@ -2927,6 +2927,12 @@
     "falseNegatives": 5,
     "falsePositives": 0
   },
+  {
+    "ruleKey": "S6831",
+    "hasTruePositives": false,
+    "falseNegatives": 6,
+    "falsePositives": 0
+  },
   {
     "ruleKey": "S6837",
     "hasTruePositives": false,

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

@@ -0,0 +1,76 @@
+package checks.spring;
+
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.stereotype.Component;
+
+public class AvoidQualifierOnBeanMethodsCheckSample {
+
+  private static final String FOO = "foo";
+  private static final String CAPITALIZED_FOO = "Foo";
+
+  @Configuration
+  class Configuration1 {
+    @Bean
+    @Qualifier("foo") // Noncompliant, [[sc=5;ec=22;quickfixes=qf1]] {{Remove this redundant "@Qualifier" annotation and rely on the @Bean method.}}
+    // fix@qf1 {{Remove "@Qualifier"}}
+    // edit@qf1 [[sc=5;ec=22]] {{}}
+    public String foo() {
+      return "foo";
+    }
+
+    @Bean
+    @Qualifier(value = "bar") // Noncompliant
+    public String bar() {
+      return "bar";
+    }
+
+    @Bean // Compliant
+    public String foobar() {
+      return "foobar";
+    }
+  }
+
+  @Component
+  class Component1 {
+    @Bean("foo")
+    @Qualifier(CAPITALIZED_FOO) // Noncompliant
+    public String foo() {
+      return "foo";
+    }
+
+    @Bean(name = "bar")
+    @Qualifier(value = "Bar") // Noncompliant
+    public String bar() {
+      return "bar";
+    }
+
+    @Bean("foobar") // Compliant
+    public String foobar() {
+      return "foobar";
+    }
+  }
+
+  class Class1 {
+    @Bean("foo")
+    @Qualifier(FOO) // Noncompliant
+    public String foo() {
+      return "foo";
+    }
+
+    @Bean(name = "bar")
+    @Qualifier // Noncompliant, [[sc=5;ec=15;quickfixes=qf3]] {{Remove this redundant "@Qualifier" annotation and rely on the @Bean method.}}
+    // fix@qf3 {{Remove "@Qualifier"}}
+    // edit@qf3 [[sc=5;ec=15]] {{}}
+    public String bar() {
+      return "bar";
+    }
+
+    @Bean("foobar") // Compliant
+    public String foobar() {
+      return "foobar";
+    }
+  }
+
+}

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

@@ -140,6 +140,7 @@
 import org.sonar.java.checks.spring.AsyncMethodsReturnTypeCheck;
 import org.sonar.java.checks.spring.AutowiredOnConstructorWhenMultipleConstructorsCheck;
 import org.sonar.java.checks.spring.AutowiredOnMultipleConstructorsCheck;
+import org.sonar.java.checks.spring.AvoidQualifierOnBeanMethodsCheck;
 import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
 import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
 import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
@@ -290,6 +291,7 @@ public final class CheckList {
     AuthorizationsStrongDecisionsCheck.class,
     AutowiredOnConstructorWhenMultipleConstructorsCheck.class,
     AutowiredOnMultipleConstructorsCheck.class,
+    AvoidQualifierOnBeanMethodsCheck.class,
     AwsConsumerBuilderUsageCheck.class,
     AwsCredentialsShouldBeSetExplicitlyCheck.class,
     AwsLambdaSyncCallCheck.class,

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

@@ -0,0 +1,121 @@
+/*
+ * 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.LinkedList;
+import java.util.List;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.model.expression.AssignmentExpressionTreeImpl;
+import org.sonar.java.model.expression.LiteralTreeImpl;
+import org.sonar.java.reporting.JavaQuickFix;
+import org.sonar.java.reporting.JavaTextEdit;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.tree.AnnotationTree;
+import org.sonar.plugins.java.api.tree.Arguments;
+import org.sonar.plugins.java.api.tree.MethodTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S6831")
+public class AvoidQualifierOnBeanMethodsCheck extends IssuableSubscriptionVisitor {
+  private static final String BEAN_ANNOTATION = "org.springframework.context.annotation.Bean";
+  private static final String QUALIFIER_ANNOTATION = "org.springframework.beans.factory.annotation.Qualifier";
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.METHOD);
+  }
+
+  /**
+   * This rule reports an issue when @Bean methods are annotated with @Qualifier.
+   */
+  @Override
+  public void visitNode(Tree tree) {
+    var methodTree = (MethodTree) tree;
+
+    var beanAnnotation = getAnnotation(methodTree, BEAN_ANNOTATION);
+    var qualifierAnnotation = getAnnotation(methodTree, QUALIFIER_ANNOTATION);
+
+    if (beanAnnotation != null && qualifierAnnotation != null) {
+      QuickFixHelper.newIssue(context)
+        .forRule(this)
+        .onTree(qualifierAnnotation)
+        .withMessage("Remove this redundant \"@Qualifier\" annotation and rely on the @Bean method.")
+        .withQuickFixes(() -> getQuickFix(methodTree, qualifierAnnotation))
+        .report();
+    }
+  }
+
+  private static AnnotationTree getAnnotation(MethodTree methodTree, String annotation) {
+    return methodTree.modifiers()
+      .annotations()
+      .stream()
+      .filter(annotationTree -> annotationTree.symbolType().is(annotation))
+      .findFirst()
+      .orElse(null);
+  }
+
+  private static List<JavaQuickFix> getQuickFix(MethodTree methodTree, AnnotationTree qualifierAnnotation) {
+    List<JavaQuickFix> quickFixes = new LinkedList<>();
+
+    // quick fix only for @Qualifier annotations without arguments or with argument that matches the method name
+    if (isFixable(methodTree, qualifierAnnotation)) {
+      var quickFix = JavaQuickFix.newQuickFix("Remove \"@Qualifier\"")
+        .addTextEdit(JavaTextEdit.removeTree(qualifierAnnotation))
+        .build();
+      quickFixes.add(quickFix);
+    }
+
+    return quickFixes;
+  }
+
+  private static boolean isFixable(MethodTree methodTree, AnnotationTree qualifierAnnotation) {
+    var arguments = qualifierAnnotation.arguments();
+
+    // @Qualifier annotation without argument can be always removed
+    if (arguments.isEmpty()) {
+      return true;
+    }
+
+    // @Qualifier that matches the method name is redundant and can be removed
+    var methodName = methodTree.simpleName().name();
+    return getQualifierAnnotationValue(arguments).equals(methodName);
+  }
+
+  private static String getQualifierAnnotationValue(Arguments arguments) {
+    var argument = arguments.get(0);
+    String qualifierAnnotationValue;
+
+    if (argument.is(Tree.Kind.ASSIGNMENT)) {
+      qualifierAnnotationValue = ((LiteralTreeImpl) ((AssignmentExpressionTreeImpl) argument).expression()).value();
+    } else if (argument.is(Tree.Kind.STRING_LITERAL)) {
+      qualifierAnnotationValue = ((LiteralTreeImpl) argument).token().text();
+    } else {
+      // case when argument is an identifier: don't suggest a quick fix
+      qualifierAnnotationValue = "";
+    }
+
+    return removeQuotes(qualifierAnnotationValue);
+  }
+
+  private static String removeQuotes(String value) {
+    return value.replace("\"", "");
+  }
+}

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

@@ -0,0 +1,82 @@
+<h2>Why is this an issue?</h2>
+<p>In Spring Framework, the <code>@Qualifier</code> annotation is typically used to disambiguate between multiple beans of the same type when
+auto-wiring dependencies. It is not necessary to use <code>@Qualifier</code> when defining a bean using the <code>@Bean</code> annotation because the
+bean’s name can be explicitly specified using the <code>name</code> attribute or derived from the method name. Using <code>@Qualifier</code> on
+<code>@Bean</code> methods can lead to confusion and redundancy. Beans should be named appropriately using either the <code>name</code> attribute of
+the <code>@Bean</code> annotation or the method name itself.</p>
+<h3>Noncompliant code example</h3>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@Configuration
+public class MyConfiguration {
+  @Bean
+  @Qualifier("myService")
+  public MyService myService() {
+    // ...
+    return new MyService();
+  }
+
+  @Bean
+  @Qualifier("betterService")
+  public MyService aBetterService() {
+    // ...
+    return new MyService();
+  }
+
+  @Bean
+  @Qualifier("evenBetterService")
+  public MyService anEvenBetterService() {
+    // ...
+    return new MyService();
+  }
+
+  @Bean
+  @Qualifier("differentService")
+  public MyBean aDifferentService() {
+    // ...
+    return new MyBean();
+  }
+}
+</pre>
+<h3>Compliant solution</h3>
+<pre data-diff-id="1" data-diff-type="compliant">
+@Configuration
+public class MyConfiguration {
+  @Bean
+  public MyService myService() {
+    // ...
+    return new MyService();
+  }
+
+  @Bean(name="betterService")
+  public MyService aBetterService() {
+    // ...
+    return new MyService();
+  }
+
+  @Bean(name="evenBetterService")
+  public MyService anEvenBetterService() {
+    // ...
+    return new MyService();
+  }
+
+  @Bean(name="differentService")
+  public MyBean aDifferentService() {
+    // ...
+    return new MyBean();
+  }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> <a href="https://docs.spring.io/spring-framework/reference/core/beans/java/bean-annotation.html">Spring Framework - Using the @Bean
+  Annotation</a> </li>
+  <li> <a href="https://docs.spring.io/spring-framework/reference/core/beans/annotation-config/autowired-qualifiers.html">Spring Framework - Using
+  @Qualifier</a> </li>
+</ul>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> <a href="https://www.baeldung.com/spring-qualifier-annotation">Baeldung - Spring @Qualifier Annotation</a> </li>
+  <li> <a href="https://www.baeldung.com/spring-bean-annotations">Baeldung - Spring Bean Annotations</a> </li>
+</ul>
+

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

@@ -0,0 +1,24 @@
+{
+  "title": "\"@Qualifier\" should not be used on \"@Bean\" methods",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "spring"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6831",
+  "sqKey": "S6831",
+  "scope": "Main",
+  "quickfix": "targeted",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM",
+      "RELIABILITY": "MEDIUM"
+    },
+    "attribute": "DISTINCT"
+  }
+}

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

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