SONAR-2900 The AnnotationCheckFactory should accept check objects and not only check classes

2 files changed, 68 insertions, 21 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/checks/AnnotationCheckFactory.java b/sonar-plugin-api/src/main/java/org/sonar/api/checks/AnnotationCheckFactory.java
index a6e9147d22c..7916319e88b 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/checks/AnnotationCheckFactory.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/checks/AnnotationCheckFactory.java
@@ -24,6 +24,7 @@ import org.apache.commons.lang.StringUtils;
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.rules.ActiveRule;
 import org.sonar.api.rules.ActiveRuleParam;
+import org.sonar.api.utils.AnnotationUtils;
 import org.sonar.api.utils.FieldUtils2;
 import org.sonar.api.utils.SonarException;
 import org.sonar.check.Check;
@@ -41,39 +42,42 @@ import java.util.Map;
  */
 public final class AnnotationCheckFactory extends CheckFactory {
 
-  private Map<String, Class> checkClassesByKey = Maps.newHashMap();
+  private Map<String, Object> checksByKey = Maps.newHashMap();
 
-  private AnnotationCheckFactory(RulesProfile profile, String repositoryKey, Collection<Class> checkClasses) {
+  private AnnotationCheckFactory(RulesProfile profile, String repositoryKey, Collection checks) {
     super(profile, repositoryKey);
-    groupClassesByKey(checkClasses);
+    groupByKey(checks);
   }
 
-  public static AnnotationCheckFactory create(RulesProfile profile, String repositoryKey, Collection<Class> checkClasses) {
+  public static AnnotationCheckFactory create(RulesProfile profile, String repositoryKey, Collection checkClasses) {
     AnnotationCheckFactory factory = new AnnotationCheckFactory(profile, repositoryKey, checkClasses);
     factory.init();
     return factory;
   }
 
-  private void groupClassesByKey(Collection<Class> checkClasses) {
-    for (Class checkClass : checkClasses) {
-      String key = getRuleKey(checkClass);
+  private void groupByKey(Collection checks) {
+    for (Object check : checks) {
+      String key = getRuleKey(check);
       if (key != null) {
-        checkClassesByKey.put(key, checkClass);
+        checksByKey.put(key, check);
       }
     }
   }
 
   protected Object createCheck(ActiveRule activeRule) {
-    Class clazz = checkClassesByKey.get(activeRule.getConfigKey());
-    if (clazz != null) {
-      return instantiate(activeRule, clazz);
+    Object object = checksByKey.get(activeRule.getConfigKey());
+    if (object != null) {
+      return instantiate(activeRule, object);
     }
     return null;
   }
 
-  private Object instantiate(ActiveRule activeRule, Class clazz) {
+  private Object instantiate(ActiveRule activeRule, Object checkClassOrInstance) {
     try {
-      Object check = clazz.newInstance();
+      Object check = checkClassOrInstance;
+      if (check instanceof Class) {
+        check = ((Class) checkClassOrInstance).newInstance();
+      }
       configureFields(activeRule, check);
       return check;
 
@@ -163,18 +167,22 @@ public final class AnnotationCheckFactory extends CheckFactory {
     return null;
   }
 
-  private String getRuleKey(Class annotatedClass) {
+  private String getRuleKey(Object annotatedClassOrObject) {
     String key = null;
-    Rule ruleAnnotation = (Rule) annotatedClass.getAnnotation(Rule.class);
+    Rule ruleAnnotation = AnnotationUtils.getClassAnnotation(annotatedClassOrObject, Rule.class);
     if (ruleAnnotation != null) {
       key = ruleAnnotation.key();
     } else {
-      Check checkAnnotation = (Check) annotatedClass.getAnnotation(Check.class);
+      Check checkAnnotation = AnnotationUtils.getClassAnnotation(annotatedClassOrObject, Check.class);
       if (checkAnnotation != null) {
         key = checkAnnotation.key();
 
       }
     }
-    return StringUtils.defaultIfEmpty(key, annotatedClass.getCanonicalName());
+    Class clazz = annotatedClassOrObject.getClass();
+    if (annotatedClassOrObject instanceof Class) {
+      clazz = (Class) annotatedClassOrObject;
+    }
+    return StringUtils.defaultIfEmpty(key, clazz.getCanonicalName());
   }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/checks/AnnotationCheckFactoryTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/checks/AnnotationCheckFactoryTest.java
index 5c2832f3f14..60670a372d3 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/checks/AnnotationCheckFactoryTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/checks/AnnotationCheckFactoryTest.java
@@ -21,6 +21,7 @@ package org.sonar.api.checks;
 
 import org.hamcrest.Matcher;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.rules.ActiveRule;
 import org.sonar.api.rules.Rule;
@@ -30,11 +31,13 @@ import java.util.Arrays;
 
 import static org.hamcrest.Matchers.*;
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
 
 public class AnnotationCheckFactoryTest {
 
+  @org.junit.Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   @Test
   public void createCheckWithoutProperties() {
     RulesProfile profile = RulesProfile.create("repo", "java");
@@ -62,8 +65,10 @@ public class AnnotationCheckFactoryTest {
     assertThat(((CheckWithStringProperty) check).getPattern(), is("foo"));
   }
 
-  @Test(expected = SonarException.class)
+  @Test
   public void failIfMissingProperty() {
+    thrown.expect(SonarException.class);
+
     RulesProfile profile = RulesProfile.create("repo", "java");
     Rule rule = Rule.create("repo", "org.sonar.api.checks.CheckWithStringProperty", "");
     rule.createParameter("unknown");
@@ -121,8 +126,10 @@ public class AnnotationCheckFactoryTest {
     assertThat(((ImplementedCheck) check).getMax(), is(300));
   }
 
-  @Test(expected = SonarException.class)
+  @Test
   public void failIfPropertyTypeIsNotSupported() {
+    thrown.expect(SonarException.class);
+
     RulesProfile profile = RulesProfile.create("repo", "java");
     Rule rule = Rule.create("repo", "org.sonar.api.checks.CheckWithUnsupportedPropertyType", "");
     rule.createParameter("max");
@@ -158,4 +165,36 @@ public class AnnotationCheckFactoryTest {
 
     assertThat(factory.getChecks(), (Matcher) not(hasItems(nullValue())));
   }
+
+  /**
+   * SONAR-2900
+   */
+  @Test
+  public void create_accept_objects() {
+    RulesProfile profile = RulesProfile.create("repo", "java");
+    ActiveRule activeRule = profile.activateRule(Rule.create("repo", "org.sonar.api.checks.CheckWithoutProperties", ""), null);
+    CheckWithoutProperties checkInstance = new CheckWithoutProperties();
+    AnnotationCheckFactory factory = AnnotationCheckFactory.create(profile, "repo", Arrays.asList(checkInstance));
+
+    Object check = factory.getCheck(activeRule);
+    assertNotNull(check);
+    assertSame(check, checkInstance);
+  }
+
+  @Test
+  public void create_instance_with_string_property() {
+    RulesProfile profile = RulesProfile.create("repo", "java");
+    Rule rule = Rule.create("repo", "org.sonar.api.checks.CheckWithStringProperty", "");
+    rule.createParameter("pattern");
+
+    ActiveRule activeRule = profile.activateRule(rule, null);
+    activeRule.setParameter("pattern", "foo");
+    CheckWithStringProperty checkInstance = new CheckWithStringProperty();
+    AnnotationCheckFactory factory = AnnotationCheckFactory.create(profile, "repo", Arrays.asList(checkInstance));
+
+    Object check = factory.getCheck(activeRule);
+    assertNotNull(check);
+    assertSame(check, checkInstance);
+    assertThat(checkInstance.getPattern(), is("foo"));
+  }
 }