Validate actual rule object instead of Field#type for @rule

fixes junit-team#1720
vlsi · Oct 10, 2021 · 9f1699c · 9f1699c
1 parent 7167b23
commit 9f1699c
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 0 deletions.
diff --git a/src/main/java/org/junit/internal/runners/rules/RuleMemberValidator.java b/src/main/java/org/junit/internal/runners/rules/RuleMemberValidator.java
@@ -230,6 +230,11 @@ public void validate(FrameworkMember<?> member, Class<? extends Annotation> anno
      */
     private static final class FieldMustBeARule implements RuleValidator {
         public void validate(FrameworkMember<?> member, Class<? extends Annotation> annotation, List<Throwable> errors) {
+            if (true) {
+                // Field type is not validated as field value can still implement the interface
+                // even in case field type does not.
+                return;
+            }
             if (!isRuleType(member)) {
                 errors.add(new ValidationError(member, annotation,
                         "must implement MethodRule or TestRule."));
@@ -270,6 +275,11 @@ private static final class FieldMustBeATestRule implements RuleValidator {
 
         public void validate(FrameworkMember<?> member,
                 Class<? extends Annotation> annotation, List<Throwable> errors) {
+            if (true) {
+                // Field type is not validated as field value can still implement the interface
+                // even in case field type does not.
+                return;
+            }
             if (!isTestRule(member)) {
                 errors.add(new ValidationError(member, annotation,
                         "must implement TestRule."));

diff --git a/src/main/java/org/junit/runners/model/TestClass.java b/src/main/java/org/junit/runners/model/TestClass.java
@@ -22,6 +22,8 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.internal.MethodSorter;
+import org.junit.rules.MethodRule;
+import org.junit.rules.TestRule;
 
 /**
  * Wraps a class to be run, providing method validation and annotation searching
@@ -249,6 +251,10 @@ public <T> void collectAnnotatedFieldValues(Object test,
                 Object fieldValue = each.get(test);
                 if (valueClass.isInstance(fieldValue)) {
                     consumer.accept(each, valueClass.cast(fieldValue));
+                } else if (fieldValue != null && (valueClass == TestRule.class || valueClass == MethodRule.class)
+                        && !(fieldValue instanceof TestRule) && !(fieldValue instanceof MethodRule)) {
+                    throw new IllegalArgumentException(each + " must implement MethodRule or TestRule." +
+                            " Actual type is " + fieldValue.getClass());
                 }
             } catch (IllegalAccessException e) {
                 throw new RuntimeException(

diff --git a/src/test/java/org/junit/rules/RuleMemberValidatorTest.java b/src/test/java/org/junit/rules/RuleMemberValidatorTest.java
@@ -10,6 +10,7 @@
 import java.util.List;
 
 import org.junit.ClassRule;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runners.model.FrameworkMethod;
@@ -76,6 +77,7 @@ static class NonPublicTestWithClassRule {
      * <a href="https://github.com/junit-team/junit4/issues/1019">Issue #1019</a>
      */
     @Test
+    @Ignore("junit-team/junit4/issues/1720, validation uses actual object value, not just field type")
     public void rejectClassRuleThatIsImplementationOfMethodRule() {
         TestClass target = new TestClass(TestWithClassRuleIsImplementationOfMethodRule.class);
         CLASS_RULE_VALIDATOR.validate(target, errors);
@@ -126,6 +128,7 @@ public Statement apply(Statement base, FrameworkMethod method, Object target) {
      * <a href="https://github.com/junit-team/junit4/issues/1019">Issue #1019</a>
      */
     @Test
+    @Ignore("junit-team/junit4/issues/1720, validation uses actual object value, not just field type")
     public void rejectClassRuleIsAnArbitraryObject() throws Exception {
         TestClass target = new TestClass(TestWithClassRuleIsAnArbitraryObject.class);
         CLASS_RULE_VALIDATOR.validate(target, errors);
@@ -212,6 +215,7 @@ public Statement apply(Statement base, FrameworkMethod method,
     }
 
     @Test
+    @Ignore("junit-team/junit4/issues/1720, validation uses actual object value, not just field type")
     public void rejectArbitraryObjectWithRuleAnnotation() throws Exception {
         TestClass target = new TestClass(TestWithArbitraryObjectWithRuleAnnotation.class);
         RULE_VALIDATOR.validate(target, errors);