SONARJAVA-4681: Fix bug due to wrong semantics usage on S6832 (#4524)

SonarSource · Nov 7, 2023 · 156b7eb · 156b7eb
1 parent f25d9b7
commit 156b7eb
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 68 deletions.
diff --git a/...test-sources/src/main/java/checks/spring/NonSingletonAutowiredInSingletonCheckSample.java b/...test-sources/src/main/java/checks/spring/NonSingletonAutowiredInSingletonCheckSample.java
@@ -1,34 +1,18 @@
 package checks.spring;
 
+import checks.spring.NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition.PrototypeBean1;
+import checks.spring.NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition.PrototypeBean2;
+import checks.spring.NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition.PrototypeBean3;
+import checks.spring.NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition.RequestBean1;
 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
@@ -180,7 +164,7 @@ public void setPrototypeBean2(PrototypeBean2 prototypeBean2) { // Noncompliant
     }
   }
 
-  @Scope(value = CUSTOM_SCOPE, scopeName = "custom", proxyMode = TARGET_CLASS)
+  @Scope(value = "custom", scopeName = "custom", proxyMode = TARGET_CLASS)
   public class CustomBean {
     @Autowired
     private SingletonBean singletonBean; // Compliant, since scope is non-Singleton

diff --git a/...checks/spring/NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition.java b/...checks/spring/NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition.java
@@ -0,0 +1,29 @@
+package checks.spring;
+
+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 NonSingletonAutowiredInSingletonCheckSampleNonSingletonBeansDefinition {
+  private static final String PROTOTYPE_SCOPE = "prototype";
+
+  @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 {
+  }
+}
diff --git a/...cks/src/main/java/org/sonar/java/checks/spring/NonSingletonAutowiredInSingletonCheck.java b/...cks/src/main/java/org/sonar/java/checks/spring/NonSingletonAutowiredInSingletonCheck.java
@@ -27,13 +27,9 @@
 import org.sonar.java.checks.helpers.MethodTreeUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
-import org.sonar.plugins.java.api.tree.Arguments;
-import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
-import org.sonar.plugins.java.api.tree.IdentifierTree;
-import org.sonar.plugins.java.api.tree.LiteralTree;
-import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.VariableTree;
@@ -45,7 +41,6 @@ public class NonSingletonAutowiredInSingletonCheck extends IssuableSubscriptionV
   private static final String JAVAX_INJECT_ANNOTATION = "javax.inject.Inject";
   private static final String JAKARTA_INJECT_ANNOTATION = "jakarta.inject.Inject";
   private static final Set<String> AUTO_WIRING_ANNOTATIONS = Set.of(AUTOWIRED_ANNOTATION, JAVAX_INJECT_ANNOTATION, JAKARTA_INJECT_ANNOTATION);
-  private static final Set<String> SINGLETON_LITERALS = Set.of("singleton", "\"singleton\"");
 
   @Override
   public List<Tree.Kind> nodesToVisit() {
@@ -157,57 +152,32 @@ private void reportIfNonSingletonInSingleton(ClassTree enclosingClassTree, Varia
   }
 
   private static boolean hasTypeNotSingletonBean(VariableTree variableTree) {
-    var typeSymbolDeclaration = variableTree.type().symbolType().symbol().declaration();
-    return typeSymbolDeclaration != null && hasNotSingletonScopeAnnotation(typeSymbolDeclaration.modifiers().annotations());
+    return hasNotSingletonScopeAnnotation(variableTree.symbol().type().symbol().metadata().annotations());
   }
 
   private static boolean isAutoWiringAnnotation(AnnotationTree annotationTree) {
     return AUTO_WIRING_ANNOTATIONS.contains(annotationTree.symbolType().fullyQualifiedName());
   }
 
   private static boolean isSingletonBean(ClassTree classTree) {
-    return !hasNotSingletonScopeAnnotation(classTree.modifiers().annotations());
+    return !hasNotSingletonScopeAnnotation(classTree.symbol().metadata().annotations());
   }
 
-  private static boolean hasNotSingletonScopeAnnotation(List<AnnotationTree> annotations) {
+  private static boolean hasNotSingletonScopeAnnotation(List<SymbolMetadata.AnnotationInstance> annotations) {
     // Only classes annotated with @Scope, having a value different from "singleton", are considered as non-Singleton
     return annotations.stream().anyMatch(NonSingletonAutowiredInSingletonCheck::isNotSingletonScopeAnnotation);
   }
 
-  private static boolean isNotSingletonScopeAnnotation(AnnotationTree annotationTree) {
-    return annotationTree.symbolType().is(SCOPED_ANNOTATION)
-      && (isNotSingletonLiteralValue(annotationTree.arguments()) || isNotSingletonAssignmentValue(annotationTree.arguments()));
+  private static boolean isNotSingletonScopeAnnotation(SymbolMetadata.AnnotationInstance annotationInstance) {
+    return annotationInstance.symbol().type().is(SCOPED_ANNOTATION)
+      && annotationInstance.values()
+        .stream()
+        .anyMatch(NonSingletonAutowiredInSingletonCheck::isNotSingletonAnnotationValue);
   }
 
-  private static boolean isNotSingletonLiteralValue(Arguments arguments) {
-    return arguments.size() == 1
-      && arguments.get(0).is(Tree.Kind.STRING_LITERAL)
-      && isNotSingletonLiteral(((LiteralTree) arguments.get(0)).value());
+  private static boolean isNotSingletonAnnotationValue(SymbolMetadata.AnnotationValue annotationValue) {
+    return ("value".equals(annotationValue.name()) || "scopeName".equals(annotationValue.name()))
+      // both "value" and "scopeName" in @Scope annotation have String type
+      && !"singleton".equalsIgnoreCase((String) annotationValue.value());
   }
-
-  private static boolean isNotSingletonAssignmentValue(Arguments arguments) {
-    return arguments
-      .stream()
-      .filter(argument -> argument.is(Tree.Kind.ASSIGNMENT))
-      .map(AssignmentExpressionTree.class::cast)
-      .anyMatch(NonSingletonAutowiredInSingletonCheck::isNotAssignmentToSingletonValue);
-  }
-
-  private static boolean isNotAssignmentToSingletonValue(AssignmentExpressionTree assignmentExpressionTree) {
-    var expression = assignmentExpressionTree.expression();
-
-    if (expression.is(Tree.Kind.STRING_LITERAL)) {
-      return isNotSingletonLiteral(((LiteralTree) expression).value());
-    }
-
-    var variable = (IdentifierTree) assignmentExpressionTree.variable();
-
-    return ("value".equals(variable.name()) || "scopeName".equals(variable.name()))
-      && (expression.is(Tree.Kind.MEMBER_SELECT) && isNotSingletonLiteral(((MemberSelectExpressionTree) expression).identifier().name()));
-  }
-
-  private static boolean isNotSingletonLiteral(String value) {
-    return SINGLETON_LITERALS.stream().noneMatch(singletonLiteral -> singletonLiteral.equalsIgnoreCase(value));
-  }
-
 }
diff --git a/...src/test/java/org/sonar/java/checks/spring/NonSingletonAutowiredInSingletonCheckTest.java b/...src/test/java/org/sonar/java/checks/spring/NonSingletonAutowiredInSingletonCheckTest.java
@@ -23,9 +23,6 @@
 import org.sonar.java.checks.verifier.CheckVerifier;
 import org.sonar.java.checks.verifier.TestUtils;
 
-import static org.junit.jupiter.api.Assertions.*;
-import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
-
 class NonSingletonAutowiredInSingletonCheckTest {
 
   @Test
@@ -39,7 +36,7 @@ void test() {
   @Test
   void test_non_semantics() {
     CheckVerifier.newVerifier()
-      .onFile(mainCodeSourcesPath("checks/spring/NonSingletonAutowiredInSingletonCheckSample.java"))
+      .onFile(TestUtils.mainCodeSourcesPath("checks/spring/NonSingletonAutowiredInSingletonCheckSample.java"))
       .withCheck(new NonSingletonAutowiredInSingletonCheck())
       .withoutSemantic()
       .verifyNoIssues();