From: Gustavo Martin Morcuende Date: Tue, 9 Aug 2016 20:52:43 +0000 (+0200) Subject: sonar-java-plugin: check for mutable Spring Service instance fields X-Git-Url: https://git.gumartinm.name/?a=commitdiff_plain;h=2290cf920d1970f92afc4685dabdbd5dfa0fbf81;p=JavaForFun 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 --- 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 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 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 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 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 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 --- 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()); } }