From 9e90c2843cdac884de21717ea67efd9b80633c11 Mon Sep 17 00:00:00 2001 From: Angelo Buono Date: Tue, 31 Oct 2023 11:11:11 +0100 Subject: [PATCH 1/6] SONARJAVA-4678: Implement S6831: @Qualifier should not be used on @Bean methods --- .../java/org/sonar/java/it/AutoScanTest.java | 4 +- .../autoscan/autoscan-diff-by-rules.json | 14 +++- .../AvoidQualifierOnBeanMethodsCheck.java | 28 +++++++ .../java/org/sonar/java/checks/CheckList.java | 6 +- .../AvoidQualifierOnBeanMethodsCheck.java | 71 ++++++++++++++++ .../org/sonar/l10n/java/rules/java/S6831.html | 82 +++++++++++++++++++ .../org/sonar/l10n/java/rules/java/S6831.json | 24 ++++++ .../java/rules/java/Sonar_way_profile.json | 3 +- .../AvoidQualifierOnBeanMethodsCheckTest.java | 45 ++++++++++ 9 files changed, 268 insertions(+), 9 deletions(-) create mode 100644 java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java create mode 100644 java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.html create mode 100644 java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.json create mode 100644 java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java diff --git a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java index fd4dbbb7f2c..1ff51ae61b8 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -180,7 +180,7 @@ public void javaCheckTestSources() throws Exception { softly.assertThat(newTotal).isEqualTo(knownTotal); softly.assertThat(rulesCausingFPs).hasSize(6); softly.assertThat(rulesNotReporting).hasSize(7); - softly.assertThat(rulesSilenced).hasSize(75); + softly.assertThat(rulesSilenced).hasSize(76); /** * 4. Check total number of differences (FPs + FNs) @@ -188,7 +188,7 @@ public void javaCheckTestSources() throws Exception { * 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: 3478"); + softly.assertThat(differences).isEqualTo("Issues differences: 3480"); softly.assertAll(); } diff --git a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json index c521820f991..961bdaaca0d 100644 --- a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/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 }, { @@ -2920,5 +2920,11 @@ "hasTruePositives": false, "falseNegatives": 5, "falsePositives": 0 + }, + { + "ruleKey": "S6831", + "hasTruePositives": false, + "falseNegatives": 2, + "falsePositives": 0 } ] diff --git a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java new file mode 100644 index 00000000000..60b294ce78b --- /dev/null +++ b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -0,0 +1,28 @@ +package checks.spring; + +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +public class AvoidQualifierOnBeanMethodsCheck { + + @Configuration + class Configuration1 { + @Bean + @Qualifier("foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation}} + public String foo() { + return "foo"; + } + + @Bean + @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation}} + public String bar() { + return "bar"; + } + + @Bean("foobar") // Compliant + public String foobar() { + return "foobar"; + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/CheckList.java b/java-checks/src/main/java/org/sonar/java/checks/CheckList.java index 961a6451c82..7373bd16532 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CheckList.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CheckList.java @@ -137,11 +137,12 @@ import org.sonar.java.checks.serialization.SerializableSuperConstructorCheck; import org.sonar.java.checks.spring.AsyncMethodsCalledViaThisCheck; import org.sonar.java.checks.spring.AsyncMethodsReturnTypeCheck; -import org.sonar.java.checks.spring.AutowiredOnMultipleConstructorsCheck; 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.ModelAttributeNamingConventionForSpELCheck; import org.sonar.java.checks.spring.FieldDependencyInjectionCheck; +import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck; import org.sonar.java.checks.spring.OptionalRestParametersShouldBeObjectsCheck; import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck; import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck; @@ -287,6 +288,7 @@ public final class CheckList { AuthorizationsStrongDecisionsCheck.class, AutowiredOnConstructorWhenMultipleConstructorsCheck.class, AutowiredOnMultipleConstructorsCheck.class, + AvoidQualifierOnBeanMethodsCheck.class, AwsConsumerBuilderUsageCheck.class, AwsCredentialsShouldBeSetExplicitlyCheck.class, AwsLambdaSyncCallCheck.class, diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java new file mode 100644 index 00000000000..2fbbad40959 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -0,0 +1,71 @@ +/* + * 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 org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.ModifiersTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S6831") +public class AvoidQualifierOnBeanMethodsCheck extends IssuableSubscriptionVisitor { + private static final String CONFIGURATION_ANNOTATION = "org.springframework.context.annotation.Configuration"; + 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 nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + var classTree = (ClassTree) tree; + + if(hasAnnotation(classTree.modifiers(), CONFIGURATION_ANNOTATION)) { + classTree.members() + .stream() + .filter(member -> member.is(Tree.Kind.METHOD)) + .map(MethodTree.class::cast) + .filter(methodTree -> hasAnnotation(methodTree.modifiers(), BEAN_ANNOTATION)) + .filter(methodTree -> hasAnnotation(methodTree.modifiers(), QUALIFIER_ANNOTATION)) + .forEach(methodTree -> reportIssue(getQualifierAnnotation(methodTree), "Remove this redundant \"@Qualifier\" annotation")); + } + } + + private static boolean hasAnnotation(ModifiersTree modifiersTree, String annotation) { + return modifiersTree.annotations() + .stream() + .anyMatch(annotationTree -> annotationTree.symbolType().is(annotation)); + } + + private static AnnotationTree getQualifierAnnotation(MethodTree methodTree) { + return methodTree.modifiers() + .annotations() + .stream() + .filter(annotationTree -> annotationTree.symbolType().is(QUALIFIER_ANNOTATION)) + .findFirst() + .orElse(null); + } +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.html b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.html new file mode 100644 index 00000000000..a6532465541 --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.html @@ -0,0 +1,82 @@ +

Why is this an issue?

+

In Spring Framework, the @Qualifier annotation is typically used to disambiguate between multiple beans of the same type when +auto-wiring dependencies. It is not necessary to use @Qualifier when defining a bean using the @Bean annotation because the +bean’s name can be explicitly specified using the name attribute or derived from the method name. Using @Qualifier on +@Bean methods can lead to confusion and redundancy. Beans should be named appropriately using either the name attribute of +the @Bean annotation or the method name itself.

+

Noncompliant code example

+
+@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();
+  }
+}
+
+

Compliant solution

+
+@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();
+  }
+}
+
+

Resources

+

Documentation

+ +

Articles & blog posts

+ + diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6831.json new file mode 100644 index 00000000000..d19f9272c1a --- /dev/null +++ b/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" + } +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 87e03b7dd2a..5210655047f 100644 --- a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -487,6 +487,7 @@ "S6813", "S6814", "S6818", - "S6829" + "S6829", + "S6831" ] } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java new file mode 100644 index 00000000000..6e773614c12 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java @@ -0,0 +1,45 @@ +/* + * 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 org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class AvoidQualifierOnBeanMethodsCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheck.java")) + .withCheck(new AvoidQualifierOnBeanMethodsCheck()) + .verifyIssues(); + } + + @Test + void test_no_semantics() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheck.java")) + .withCheck(new AvoidQualifierOnBeanMethodsCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +} From b4d8af83b5590e0c2d4d483ca310f624ac0d098e Mon Sep 17 00:00:00 2001 From: Angelo Buono Date: Tue, 31 Oct 2023 14:18:31 +0100 Subject: [PATCH 2/6] SONARJAVA-4678: Implement S6831: @Qualifier should not be used on @Bean methods --- .../java/org/sonar/java/it/AutoScanTest.java | 2 +- .../autoscan/autoscan-diff-by-rules.json | 2 +- .../AvoidQualifierOnBeanMethodsCheck.java | 46 ++++++++++++++++++- .../AvoidQualifierOnBeanMethodsCheck.java | 43 ++++++++--------- .../AvoidQualifierOnBeanMethodsCheckTest.java | 4 +- 5 files changed, 71 insertions(+), 26 deletions(-) diff --git a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java index 1ff51ae61b8..ee79ef0bbc7 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -188,7 +188,7 @@ public void javaCheckTestSources() throws Exception { * 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: 3480"); + softly.assertThat(differences).isEqualTo("Issues differences: 3484"); softly.assertAll(); } diff --git a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json index 961bdaaca0d..a15c15cf91a 100644 --- a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -2924,7 +2924,7 @@ { "ruleKey": "S6831", "hasTruePositives": false, - "falseNegatives": 2, + "falseNegatives": 6, "falsePositives": 0 } ] diff --git a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java index 60b294ce78b..13a796b8ae8 100644 --- a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -3,13 +3,16 @@ 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 AvoidQualifierOnBeanMethodsCheck { @Configuration class Configuration1 { @Bean - @Qualifier("foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier("foo") // Noncompliant, [[sc=5;ec=22;quickfixes=qf1]] {{Remove this redundant "@Qualifier" annotation}} + // fix@qf1 {{Remove "@Qualifier"}} + // edit@qf1 [[sc=5;ec=22]] {{}} public String foo() { return "foo"; } @@ -20,7 +23,46 @@ public String bar() { return "bar"; } - @Bean("foobar") // Compliant + @Bean // Compliant + public String foobar() { + return "foobar"; + } + } + + @Component + class Component1 { + @Bean("foo") + @Qualifier("foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation}} + public String foo() { + return "foo"; + } + + @Bean(name = "bar") + @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation}} + public String bar() { + return "bar"; + } + + @Bean("foobar") // Compliant + public String foobar() { + return "foobar"; + } + } + + class Class1 { + @Bean("foo") + @Qualifier("foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation}} + public String foo() { + return "foo"; + } + + @Bean(name = "bar") + @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation}} + public String bar() { + return "bar"; + } + + @Bean("foobar") // Compliant public String foobar() { return "foobar"; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java index 2fbbad40959..66f9b3b99fd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -21,50 +21,51 @@ import java.util.List; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.QuickFixHelper; +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.ClassTree; import org.sonar.plugins.java.api.tree.MethodTree; -import org.sonar.plugins.java.api.tree.ModifiersTree; import org.sonar.plugins.java.api.tree.Tree; @Rule(key = "S6831") public class AvoidQualifierOnBeanMethodsCheck extends IssuableSubscriptionVisitor { - private static final String CONFIGURATION_ANNOTATION = "org.springframework.context.annotation.Configuration"; 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 nodesToVisit() { - return List.of(Tree.Kind.CLASS); + 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 classTree = (ClassTree) tree; + var methodTree = (MethodTree) tree; - if(hasAnnotation(classTree.modifiers(), CONFIGURATION_ANNOTATION)) { - classTree.members() - .stream() - .filter(member -> member.is(Tree.Kind.METHOD)) - .map(MethodTree.class::cast) - .filter(methodTree -> hasAnnotation(methodTree.modifiers(), BEAN_ANNOTATION)) - .filter(methodTree -> hasAnnotation(methodTree.modifiers(), QUALIFIER_ANNOTATION)) - .forEach(methodTree -> reportIssue(getQualifierAnnotation(methodTree), "Remove this redundant \"@Qualifier\" annotation")); - } - } + var beanAnnotation = getAnnotation(methodTree, BEAN_ANNOTATION); + var qualifierAnnotation = getAnnotation(methodTree, QUALIFIER_ANNOTATION); - private static boolean hasAnnotation(ModifiersTree modifiersTree, String annotation) { - return modifiersTree.annotations() - .stream() - .anyMatch(annotationTree -> annotationTree.symbolType().is(annotation)); + if(beanAnnotation != null && qualifierAnnotation != null) { + QuickFixHelper.newIssue(context) + .forRule(this) + .onTree(qualifierAnnotation) + .withMessage("Remove this redundant \"@Qualifier\" annotation") + .withQuickFix(() -> JavaQuickFix.newQuickFix("Remove \"@Qualifier\"") + .addTextEdit(JavaTextEdit.removeTree(qualifierAnnotation)) + .build()) + .report(); + } } - private static AnnotationTree getQualifierAnnotation(MethodTree methodTree) { + private static AnnotationTree getAnnotation(MethodTree methodTree, String annotation) { return methodTree.modifiers() .annotations() .stream() - .filter(annotationTree -> annotationTree.symbolType().is(QUALIFIER_ANNOTATION)) + .filter(annotationTree -> annotationTree.symbolType().is(annotation)) .findFirst() .orElse(null); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java index 6e773614c12..be47ea08a35 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java @@ -21,6 +21,7 @@ import org.junit.jupiter.api.Test; import org.sonar.java.checks.verifier.CheckVerifier; +import org.sonar.java.checks.verifier.internal.InternalCheckVerifier; import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; @@ -28,9 +29,10 @@ class AvoidQualifierOnBeanMethodsCheckTest { @Test void test() { - CheckVerifier.newVerifier() + ((InternalCheckVerifier) CheckVerifier.newVerifier()) .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheck.java")) .withCheck(new AvoidQualifierOnBeanMethodsCheck()) + .withQuickFixes() .verifyIssues(); } From d54f6f827daa2fc72d1ed0281a19702daecea7d0 Mon Sep 17 00:00:00 2001 From: Angelo Buono Date: Tue, 31 Oct 2023 17:25:05 +0100 Subject: [PATCH 3/6] Improved quick fixe --- .../AvoidQualifierOnBeanMethodsCheck.java | 14 +++--- .../AvoidQualifierOnBeanMethodsCheck.java | 49 +++++++++++++++++-- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java index 13a796b8ae8..62d1edebe35 100644 --- a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -10,7 +10,7 @@ public class AvoidQualifierOnBeanMethodsCheck { @Configuration class Configuration1 { @Bean - @Qualifier("foo") // Noncompliant, [[sc=5;ec=22;quickfixes=qf1]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier("foo") // Noncompliant, [[sc=5;ec=22;quickfixes=qf1]] {{Remove this redundant "@Qualifier" annotation.}} // fix@qf1 {{Remove "@Qualifier"}} // edit@qf1 [[sc=5;ec=22]] {{}} public String foo() { @@ -18,7 +18,9 @@ public String foo() { } @Bean - @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30;quickfixes=qf2]] {{Remove this redundant "@Qualifier" annotation.}} + // fix@qf2 {{Remove "@Qualifier"}} + // edit@qf2 [[sc=5;ec=30]] {{}} public String bar() { return "bar"; } @@ -32,13 +34,13 @@ public String foobar() { @Component class Component1 { @Bean("foo") - @Qualifier("foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier("Foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation.}} public String foo() { return "foo"; } @Bean(name = "bar") - @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier(value = "Bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation.}} public String bar() { return "bar"; } @@ -51,13 +53,13 @@ public String foobar() { class Class1 { @Bean("foo") - @Qualifier("foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier("Foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation.}} public String foo() { return "foo"; } @Bean(name = "bar") - @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation}} + @Qualifier // Noncompliant, [[sc=5;ec=15]] {{Remove this redundant "@Qualifier" annotation.}} public String bar() { return "bar"; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java index 66f9b3b99fd..e6b6c8f4be0 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -19,9 +19,12 @@ */ 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; @@ -53,10 +56,8 @@ public void visitNode(Tree tree) { QuickFixHelper.newIssue(context) .forRule(this) .onTree(qualifierAnnotation) - .withMessage("Remove this redundant \"@Qualifier\" annotation") - .withQuickFix(() -> JavaQuickFix.newQuickFix("Remove \"@Qualifier\"") - .addTextEdit(JavaTextEdit.removeTree(qualifierAnnotation)) - .build()) + .withMessage("Remove this redundant \"@Qualifier\" annotation.") + .withQuickFixes(() -> getQuickFix(methodTree, qualifierAnnotation)) .report(); } } @@ -69,4 +70,44 @@ private static AnnotationTree getAnnotation(MethodTree methodTree, String annota .findFirst() .orElse(null); } + + private static List getQuickFix(MethodTree methodTree, AnnotationTree qualifierAnnotation) { + List quickFixes = new LinkedList<>(); + + 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) { + // @Qualifier annotation without argument can be always removed + if(qualifierAnnotation.arguments().isEmpty()) { + return true; + } + + // @Qualifier that matches the method name is redundant and can be removed + var methodName = methodTree.simpleName().name(); + var qualifierAnnotationValue = getQualifierAnnotationValue(qualifierAnnotation); + return methodName.equals(qualifierAnnotationValue); + } + + private static String getQualifierAnnotationValue(AnnotationTree qualifierAnnotation) { + var argument = qualifierAnnotation.arguments().get(0); + + switch (argument.kind()) { + case ASSIGNMENT: + return ((LiteralTreeImpl) ((AssignmentExpressionTreeImpl) argument).expression()).value().replace("\"", ""); + case STRING_LITERAL: + return ((LiteralTreeImpl) argument).token().text().replace("\"", ""); + default: + return ""; + } + } + } + From 6ee4534351b34c20f2a94a614772a3621c9a866f Mon Sep 17 00:00:00 2001 From: Angelo Buono Date: Wed, 1 Nov 2023 09:21:00 +0100 Subject: [PATCH 4/6] Fix conflicts and add comments --- .../java/org/sonar/java/it/AutoScanTest.java | 2 +- .../autoscan/autoscan-diff-by-rules.json | 2 ++ ...oidQualifierOnBeanMethodsCheckSample.java} | 17 ++++++----- .../AvoidQualifierOnBeanMethodsCheck.java | 30 +++++++++++-------- .../AvoidQualifierOnBeanMethodsCheckTest.java | 4 +-- 5 files changed, 32 insertions(+), 23 deletions(-) rename java-checks-test-sources/src/main/java/checks/spring/{AvoidQualifierOnBeanMethodsCheck.java => AvoidQualifierOnBeanMethodsCheckSample.java} (69%) diff --git a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java index fef73df426e..f91832e1b15 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -180,7 +180,7 @@ public void javaCheckTestSources() throws Exception { softly.assertThat(newTotal).isEqualTo(knownTotal); softly.assertThat(rulesCausingFPs).hasSize(6); softly.assertThat(rulesNotReporting).hasSize(7); - softly.assertThat(rulesSilenced).hasSize(76); + softly.assertThat(rulesSilenced).hasSize(77); /** * 4. Check total number of differences (FPs + FNs) diff --git a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json index 6175541f349..7d6554602df 100644 --- a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -2925,10 +2925,12 @@ "ruleKey": "S6831", "hasTruePositives": false, "falseNegatives": 6, + "falsePositives": 0 }, { "ruleKey": "S6837", "hasTruePositives": false, "falseNegatives": 2, + "falsePositives": 0 } ] diff --git a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java similarity index 69% rename from java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java rename to java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java index 62d1edebe35..3d7142c7a52 100644 --- a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java @@ -5,12 +5,12 @@ import org.springframework.context.annotation.Configuration; import org.springframework.stereotype.Component; -public class AvoidQualifierOnBeanMethodsCheck { +public class AvoidQualifierOnBeanMethodsCheckSample { @Configuration class Configuration1 { @Bean - @Qualifier("foo") // Noncompliant, [[sc=5;ec=22;quickfixes=qf1]] {{Remove this redundant "@Qualifier" annotation.}} + @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() { @@ -18,7 +18,7 @@ public String foo() { } @Bean - @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30;quickfixes=qf2]] {{Remove this redundant "@Qualifier" annotation.}} + @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30;quickfixes=qf2]] {{Remove this redundant "@Qualifier" annotation and rely on the @Bean method.}} // fix@qf2 {{Remove "@Qualifier"}} // edit@qf2 [[sc=5;ec=30]] {{}} public String bar() { @@ -34,13 +34,13 @@ public String foobar() { @Component class Component1 { @Bean("foo") - @Qualifier("Foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation.}} + @Qualifier("Foo") // Noncompliant public String foo() { return "foo"; } @Bean(name = "bar") - @Qualifier(value = "Bar") // Noncompliant, [[sc=5;ec=30]] {{Remove this redundant "@Qualifier" annotation.}} + @Qualifier(value = "Bar") // Noncompliant public String bar() { return "bar"; } @@ -53,13 +53,15 @@ public String foobar() { class Class1 { @Bean("foo") - @Qualifier("Foo") // Noncompliant, [[sc=5;ec=22]] {{Remove this redundant "@Qualifier" annotation.}} + @Qualifier("Foo") // Noncompliant public String foo() { return "foo"; } @Bean(name = "bar") - @Qualifier // Noncompliant, [[sc=5;ec=15]] {{Remove this redundant "@Qualifier" annotation.}} + @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"; } @@ -69,4 +71,5 @@ public String foobar() { return "foobar"; } } + } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java index e6b6c8f4be0..ff53e84c424 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -29,6 +29,7 @@ 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; @@ -56,7 +57,7 @@ public void visitNode(Tree tree) { QuickFixHelper.newIssue(context) .forRule(this) .onTree(qualifierAnnotation) - .withMessage("Remove this redundant \"@Qualifier\" annotation.") + .withMessage("Remove this redundant \"@Qualifier\" annotation and rely on the @Bean method.") .withQuickFixes(() -> getQuickFix(methodTree, qualifierAnnotation)) .report(); } @@ -74,6 +75,7 @@ private static AnnotationTree getAnnotation(MethodTree methodTree, String annota private static List getQuickFix(MethodTree methodTree, AnnotationTree qualifierAnnotation) { List 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)) @@ -85,28 +87,30 @@ private static List getQuickFix(MethodTree methodTree, AnnotationT } private static boolean isFixable(MethodTree methodTree, AnnotationTree qualifierAnnotation) { + var arguments = qualifierAnnotation.arguments(); + // @Qualifier annotation without argument can be always removed - if(qualifierAnnotation.arguments().isEmpty()) { + if(arguments.isEmpty()) { return true; } // @Qualifier that matches the method name is redundant and can be removed var methodName = methodTree.simpleName().name(); - var qualifierAnnotationValue = getQualifierAnnotationValue(qualifierAnnotation); - return methodName.equals(qualifierAnnotationValue); + return getQualifierAnnotationValue(arguments).equals(methodName); } - private static String getQualifierAnnotationValue(AnnotationTree qualifierAnnotation) { - var argument = qualifierAnnotation.arguments().get(0); + private static String getQualifierAnnotationValue(Arguments arguments) { + var argument = arguments.get(0); + var qualifierAnnotationValue = ""; - switch (argument.kind()) { - case ASSIGNMENT: - return ((LiteralTreeImpl) ((AssignmentExpressionTreeImpl) argument).expression()).value().replace("\"", ""); - case STRING_LITERAL: - return ((LiteralTreeImpl) argument).token().text().replace("\"", ""); - default: - return ""; + if(argument.is(Tree.Kind.ASSIGNMENT)) { + qualifierAnnotationValue = ((LiteralTreeImpl) ((AssignmentExpressionTreeImpl) argument).expression()).value().replace("\"", ""); + } + else if(argument.is(Tree.Kind.STRING_LITERAL)) { + qualifierAnnotationValue = ((LiteralTreeImpl) argument).token().text().replace("\"", ""); } + + return qualifierAnnotationValue; } } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java index be47ea08a35..ad3527bf871 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheckTest.java @@ -30,7 +30,7 @@ class AvoidQualifierOnBeanMethodsCheckTest { @Test void test() { ((InternalCheckVerifier) CheckVerifier.newVerifier()) - .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheck.java")) + .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java")) .withCheck(new AvoidQualifierOnBeanMethodsCheck()) .withQuickFixes() .verifyIssues(); @@ -39,7 +39,7 @@ void test() { @Test void test_no_semantics() { CheckVerifier.newVerifier() - .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheck.java")) + .onFile(mainCodeSourcesPath("checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java")) .withCheck(new AvoidQualifierOnBeanMethodsCheck()) .withoutSemantic() .verifyNoIssues(); From 9afbc468b9992cb0ec1839d5370a6c1e33ae07d9 Mon Sep 17 00:00:00 2001 From: Angelo Buono Date: Thu, 2 Nov 2023 10:59:20 +0100 Subject: [PATCH 5/6] Address comments --- ...voidQualifierOnBeanMethodsCheckSample.java | 13 +++++--- .../AvoidQualifierOnBeanMethodsCheck.java | 32 +++++++++++-------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java index 3d7142c7a52..d3e2bc875d8 100644 --- a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java +++ b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java @@ -7,6 +7,9 @@ public class AvoidQualifierOnBeanMethodsCheckSample { + private static final String FOO = "foo"; + private static final String CAPITALIZED_FOO = "Foo"; + @Configuration class Configuration1 { @Bean @@ -18,9 +21,7 @@ public String foo() { } @Bean - @Qualifier(value = "bar") // Noncompliant, [[sc=5;ec=30;quickfixes=qf2]] {{Remove this redundant "@Qualifier" annotation and rely on the @Bean method.}} - // fix@qf2 {{Remove "@Qualifier"}} - // edit@qf2 [[sc=5;ec=30]] {{}} + @Qualifier(value = "bar") // Noncompliant public String bar() { return "bar"; } @@ -34,7 +35,7 @@ public String foobar() { @Component class Component1 { @Bean("foo") - @Qualifier("Foo") // Noncompliant + @Qualifier(CAPITALIZED_FOO) // Noncompliant public String foo() { return "foo"; } @@ -53,7 +54,9 @@ public String foobar() { class Class1 { @Bean("foo") - @Qualifier("Foo") // Noncompliant + @Qualifier(FOO) // Noncompliant, [[sc=5;ec=20;quickfixes=qf2]] {{Remove this redundant "@Qualifier" annotation and rely on the @Bean method.}} + // fix@qf2 {{Remove "@Qualifier"}} + // edit@qf2 [[sc=5;ec=20]] {{}} public String foo() { return "foo"; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java index ff53e84c424..2122f6d0f35 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AvoidQualifierOnBeanMethodsCheck.java @@ -53,13 +53,13 @@ public void visitNode(Tree tree) { var beanAnnotation = getAnnotation(methodTree, BEAN_ANNOTATION); var qualifierAnnotation = getAnnotation(methodTree, QUALIFIER_ANNOTATION); - if(beanAnnotation != null && qualifierAnnotation != null) { + if (beanAnnotation != null && qualifierAnnotation != null) { QuickFixHelper.newIssue(context) - .forRule(this) + .forRule(this) .onTree(qualifierAnnotation) .withMessage("Remove this redundant \"@Qualifier\" annotation and rely on the @Bean method.") .withQuickFixes(() -> getQuickFix(methodTree, qualifierAnnotation)) - .report(); + .report(); } } @@ -76,7 +76,7 @@ private static List getQuickFix(MethodTree methodTree, AnnotationT List quickFixes = new LinkedList<>(); // quick fix only for @Qualifier annotations without arguments or with argument that matches the method name - if(isFixable(methodTree, qualifierAnnotation)) { + if (isFixable(methodTree, qualifierAnnotation)) { var quickFix = JavaQuickFix.newQuickFix("Remove \"@Qualifier\"") .addTextEdit(JavaTextEdit.removeTree(qualifierAnnotation)) .build(); @@ -90,7 +90,7 @@ private static boolean isFixable(MethodTree methodTree, AnnotationTree qualifier var arguments = qualifierAnnotation.arguments(); // @Qualifier annotation without argument can be always removed - if(arguments.isEmpty()) { + if (arguments.isEmpty()) { return true; } @@ -101,17 +101,21 @@ private static boolean isFixable(MethodTree methodTree, AnnotationTree qualifier private static String getQualifierAnnotationValue(Arguments arguments) { var argument = arguments.get(0); - var qualifierAnnotationValue = ""; - - if(argument.is(Tree.Kind.ASSIGNMENT)) { - qualifierAnnotationValue = ((LiteralTreeImpl) ((AssignmentExpressionTreeImpl) argument).expression()).value().replace("\"", ""); - } - else if(argument.is(Tree.Kind.STRING_LITERAL)) { - qualifierAnnotationValue = ((LiteralTreeImpl) argument).token().text().replace("\"", ""); + 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 qualifierAnnotationValue; + return removeQuotes(qualifierAnnotationValue); } + private static String removeQuotes(String value) { + return value.replace("\"", ""); + } } - From ca0251f3b7d3dcedede2767b5728a6cb886eafc7 Mon Sep 17 00:00:00 2001 From: Angelo Buono Date: Thu, 2 Nov 2023 11:49:45 +0100 Subject: [PATCH 6/6] Fix Tests --- its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java | 2 +- .../checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java index 2dabbafab76..dce4771dc97 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -188,7 +188,7 @@ public void javaCheckTestSources() throws Exception { * 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: 3487"); + softly.assertThat(differences).isEqualTo("Issues differences: 3489"); softly.assertAll(); } diff --git a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java index d3e2bc875d8..76b4e33d2bb 100644 --- a/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java +++ b/java-checks-test-sources/src/main/java/checks/spring/AvoidQualifierOnBeanMethodsCheckSample.java @@ -54,9 +54,7 @@ public String foobar() { class Class1 { @Bean("foo") - @Qualifier(FOO) // Noncompliant, [[sc=5;ec=20;quickfixes=qf2]] {{Remove this redundant "@Qualifier" annotation and rely on the @Bean method.}} - // fix@qf2 {{Remove "@Qualifier"}} - // edit@qf2 [[sc=5;ec=20]] {{}} + @Qualifier(FOO) // Noncompliant public String foo() { return "foo"; }