sonar-java-plugin: check for mutable Spring Service instance fields

diff --git a/Sonar/Plugins/sonar-custom-java-plugin/src/main/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheck.java b/Sonar/Plugins/sonar-custom-java-plugin/src/main/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheck.java
index f9d5c29..eb3de5f 100644 (file)
--- a/Sonar/Plugins/sonar-custom-java-plugin/src/main/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheck.java
+++ b/Sonar/Plugins/sonar-custom-java-plugin/src/main/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheck.java
@@ -1,17 +1,17 @@
 package de.example.custom.java.checks;
 
+import java.util.ArrayList;
 import java.util.List;
 
-import javax.annotation.Nullable;
-
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.check.Rule;
+import org.sonar.java.model.ModifiersUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
-import org.sonar.plugins.java.api.semantic.Symbol;
-import org.sonar.plugins.java.api.tree.AnnotationTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.Modifier;
+import org.sonar.plugins.java.api.tree.ModifiersTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.Tree.Kind;
 import org.sonar.plugins.java.api.tree.VariableTree;
@@ -21,9 +21,22 @@ import com.google.common.collect.ImmutableList;
 @Rule(key = "GUJ0002")
 public class SpringServiceInstanceFieldCheck extends IssuableSubscriptionVisitor {
        private static final Logger LOG = Loggers.get(SpringServiceInstanceFieldCheck.class);
-       
-        private JavaFileScannerContext context;
 
+       private final List<VariableTree> issuableVariables = new ArrayList<>();
+
+       private boolean isSpringService = false;
+
+       @Override
+       public void scanFile(JavaFileScannerContext context) {
+               if (context.getSemanticModel() == null) {
+                       return;
+               }
+               super.scanFile(context);
+
+               if (this.isSpringService) {
+                       reportIssuesOnVariable();
+               }
+       }
 
        @Override
        public List<Kind> nodesToVisit() {
@@ -32,35 +45,49 @@ public class SpringServiceInstanceFieldCheck extends IssuableSubscriptionVisitor
 
        @Override
        public void visitNode(Tree tree) {
-               
+
                if (tree.is(Kind.CLASS) && isSpringService((ClassTree) tree)) {
-                       
+                       this.isSpringService = true;
+               } else if (tree.is(Kind.VARIABLE)) {
+                       VariableTree variable = (VariableTree) tree;
+                       isOwnedByASpringService(variable);
+                       if (isOwnedByASpringService(variable) && !isStaticOrFinal(variable)) {
+                               issuableVariables.add(variable);
+                       }
+               }
+
+       }
+
+       private static boolean isOwnedByASpringService(VariableTree variable) {
+               if (variable.symbol().owner().metadata().isAnnotatedWith("javax.inject.Named") ||
+                               variable.symbol().owner().metadata().isAnnotatedWith("org.springframework.stereotype.Service")) {
+                       return true;
+               }
+
+               return false;
+       }
+
+       private static boolean isSpringService(ClassTree tree) {
+               if (tree.symbol().metadata().isAnnotatedWith("javax.inject.Named") ||
+                               tree.symbol().metadata().isAnnotatedWith("org.springframework.stereotype.Service")) {
+                       return true;
+               }
+
+               return false;
+       }
+
+       private static boolean isStaticOrFinal(VariableTree variable) {
+               ModifiersTree modifiers = variable.modifiers();
+               return ModifiersUtils.hasModifier(modifiers, Modifier.STATIC)
+                               || ModifiersUtils.hasModifier(modifiers, Modifier.FINAL);
+       }
+
+       private void reportIssuesOnVariable() {
+               for (VariableTree variable : issuableVariables) {
+                       reportIssue(variable.simpleName(),
+                                       "Remove this mutable service instance fields or make it \"static\" and/or \"final\"");
                }
-               
+               issuableVariables.clear();
        }
-       
-       
-         public void visitAnnotation(AnnotationTree annotationTree) {
-                   scan(annotationTree.annotationType());
-                   scan(annotationTree.arguments());
-                 }
-        
-       
-         private static boolean isOwnedByASpringService(VariableTree variable) {
-                   Symbol owner = variable.symbol().owner();
-                   return owner.isTypeSymbol() && (owner.type().isSubtypeOf("javax.servlet.http.HttpServlet") || owner.type().isSubtypeOf("org.apache.struts.action.Action"));
-                 }
-         
-         private static boolean isSpringService(ClassTree tree) {
-                 tree.symbol().metadata().isAnnotatedWith("javax.inject.Inject");
-                 tree.symbol().metadata().isAnnotatedWith("javax.inject.Inject");
-                 return true;
-                 
-        }
-
-         protected void scan(@Nullable Tree tree) {
-                   if (tree != null) {
-                   }
-                 }
 
 }

diff --git a/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceAnnotationInstanceFieldCheck.java b/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceAnnotationInstanceFieldCheck.java
new file mode 100644 (file)
index 0000000..1a9b054
--- /dev/null
+++ b/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceAnnotationInstanceFieldCheck.java
@@ -0,0 +1,22 @@
+package org.springframework.stereotype;
+
+public @interface Service {
+       String value() default "";
+}
+
+
+@Service("aService")
+public class AService {
+       private static final Integer FIELD1;
+       
+       private final Integer field2;
+       
+       private Integer field3;  // Noncompliant {{Remove this mutable service instance fields or make it "static" and/or "final"}}
+       
+       public static final Integer field4;
+       
+       public final Integer field5;
+       
+       public Integer field6;  // Noncompliant {{Remove this mutable service instance fields or make it "static" and/or "final"}}
+
+}

diff --git a/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceInstanceFieldCheck.java b/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceInstanceFieldCheck.java
deleted file mode 100644 (file)
index 16a7c80..0000000
--- a/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceInstanceFieldCheck.java
+++ /dev/null
@@ -1,18 +0,0 @@
-import javax.inject.Named;
-
-@Named("aService")
-public class AService {
-       private static final Integer FIELD1;
-       
-       private final Integer field2;
-       
-       private Integer field3;
-       
-       public static final Integer field4;
-       
-       public final Integer field5;
-       
-       // Noncompliant@+1 [[startColumn=6;endLine=+0;endColumn=10;effortToFix=4]] {{Remove this misleading mutable service instance fields or make it \"static\" and/or \"final\"}}
-       public Integer field6;
-
-}

diff --git a/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceNamedAnnotationInstanceFieldCheck.java b/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceNamedAnnotationInstanceFieldCheck.java
new file mode 100644 (file)
index 0000000..3a17680
--- /dev/null
+++ b/Sonar/Plugins/sonar-custom-java-plugin/src/test/files/checks/SpringServiceNamedAnnotationInstanceFieldCheck.java
@@ -0,0 +1,22 @@
+package javax.inject;
+
+public @interface Named {
+    String value() default "";
+}
+
+
+@Named("aService")
+public class AService {
+       private static final Integer FIELD1;
+       
+       private final Integer field2;
+       
+       private Integer field3;  // Noncompliant {{Remove this mutable service instance fields or make it "static" and/or "final"}}
+       
+       public static final Integer field4;
+       
+       public final Integer field5;
+       
+       public Integer field6;  // Noncompliant {{Remove this mutable service instance fields or make it "static" and/or "final"}}
+
+}

diff --git a/Sonar/Plugins/sonar-custom-java-plugin/src/test/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheckTest.java b/Sonar/Plugins/sonar-custom-java-plugin/src/test/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheckTest.java
index 8b3c478..b373241 100644 (file)
--- a/Sonar/Plugins/sonar-custom-java-plugin/src/test/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheckTest.java
+++ b/Sonar/Plugins/sonar-custom-java-plugin/src/test/java/de/example/custom/java/checks/SpringServiceInstanceFieldCheckTest.java
@@ -4,10 +4,16 @@ import org.junit.Test;
 import org.sonar.java.checks.verifier.JavaCheckVerifier;
 
 public class SpringServiceInstanceFieldCheckTest {
-         private static final String FILENAME = "src/test/files/checks/SpringServiceInstanceFieldCheck.java";
+         private static final String NAMED_ANNOTATION = "src/test/files/checks/SpringServiceNamedAnnotationInstanceFieldCheck.java";
+         private static final String SPRING_SERVICE_ANNOTATION = "src/test/files/checks/SpringServiceAnnotationInstanceFieldCheck.java";
 
          @Test
-         public void test() {
-           JavaCheckVerifier.verify(FILENAME, new SpringServiceInstanceFieldCheck());
+         public void whenNamedAnnotationAndNoStaticOrFinalField() {
+           JavaCheckVerifier.verify(NAMED_ANNOTATION, new SpringServiceInstanceFieldCheck());
+         }
+         
+         @Test
+         public void whenSpringServiceAnnotationAndNoStaticOrFinalField() {
+           JavaCheckVerifier.verify(SPRING_SERVICE_ANNOTATION, new SpringServiceInstanceFieldCheck());
          }
 }