Shadowed static methods/fields should override parent class behavior. (#1514)

Author: kcooney

src/main/java/org/junit/runners/model/FrameworkField.java

@@ -47,12 +47,7 @@ public <T extends Annotation> T getAnnotation(Class<T> annotationType) {
 
     @Override
     public boolean isShadowedBy(FrameworkField otherMember) {
-        return false;
-    }
-
-    @Override
-    FrameworkField handlePossibleBridgeMethod(List<FrameworkField> members) {
-        return this; // fields are never bridge methods
+        return isStatic() && otherMember.getName().equals(getName());
     }
 
     @Override

src/main/java/org/junit/runners/model/FrameworkMember.java

@@ -18,7 +18,31 @@ public abstract class FrameworkMember<T extends FrameworkMember<T>> implements
      * 
      * @return member that should be used, or {@code null} if no member should be used.
      */
-    abstract T handlePossibleBridgeMethod(List<T> members);
+    final T handlePossibleShadowedMember(List<T> members) {
+        for (int i = members.size() - 1; i >=0; i--) {
+            T otherMember = members.get(i);
+            if (isShadowedBy(otherMember)) {
+                if (otherMember.isBridgeMethod()) {
+                    /*
+                     *  We need to return the previously-encountered bridge method
+                     *  because JUnit won't be able to call the parent method,
+                     *  because the parent class isn't public.
+                     */
+                    members.remove(i);
+                    return otherMember;
+                }
+                // We found a shadowed member that isn't a bridge method. Ignore it.
+                return null;
+            }
+        }
+        // No shadow or bridge method found. The caller should add *this* member.
+        return self();
+    }
+
+    @SuppressWarnings("unchecked")
+    private T self() {
+        return (T) this;
+    }
 
     abstract boolean isBridgeMethod();
 

src/main/java/org/junit/runners/model/FrameworkMethod.java

@@ -141,33 +141,8 @@ public void validateNoTypeParametersOnArgs(List<Throwable> errors) {
         new NoGenericTypeParametersValidator(method).validate(errors);
     }
 
-    @Override
-    FrameworkMethod handlePossibleBridgeMethod(List<FrameworkMethod> methods) {
-        for (int i = methods.size() - 1; i >=0; i--) {
-            FrameworkMethod otherMethod = methods.get(i);
-            if (isShadowedBy(otherMethod)) {
-                if (otherMethod.isBridgeMethod()) {
-                    /*
-                     *  We need to return the previously-encountered bridge method
-                     *  because JUnit won't be able to call the parent method,
-                     *  because the parent class isn't public.
-                     */
-                    methods.remove(i);
-                    return otherMethod;
-                }
-                // We found a shadowed member that isn't a bridge method. Ignore it.
-                return null;
-            }
-        }
-        // No shadow or bridge method found. The caller should add *this* member.
-        return this;
-    }
-
     @Override
     public boolean isShadowedBy(FrameworkMethod other) {
-        if (isStatic()) {
-            return false;
-        }
         if (!other.getName().equals(getName())) {
             return false;
         }

src/main/java/org/junit/runners/model/TestClass.java

@@ -84,7 +84,7 @@ protected static <T extends FrameworkMember<T>> void addToAnnotationLists(T memb
         for (Annotation each : member.getAnnotations()) {
             Class<? extends Annotation> type = each.annotationType();
             List<T> members = getAnnotatedMembers(map, type, true);
-            T memberToAdd = member.handlePossibleBridgeMethod(members);
+            T memberToAdd = member.handlePossibleShadowedMember(members);
             if (memberToAdd == null) {
                 return;
             }

src/test/java/org/junit/tests/running/methods/AnnotationTest.java

@@ -586,13 +586,13 @@ public void test() {
         }
     }
 
-    public void testStaticMethodsNeverTreatedAsShadowed() throws Exception {
+    public void testStaticMethodsCanBeTreatedAsShadowed() throws Exception {
         log = "";
         assertThat(testResult(SubStaticMethodShadowing.class), isSuccessful());
         assertEquals(
-                "super.rule().before() sub.rule().before() "
+                "sub.rule().before() "
                 + "Test "
-                + "sub.rule().after() super.rule().after() ",
+                + "sub.rule().after() ",
                 log);
     }
 
@@ -680,13 +680,13 @@ public void test() {
         }
     }
 
-    public void testStaticFieldsNeverTreatedAsShadowed() throws Exception {
+    public void testStaticFieldsCanBeTreatedAsShadowed() throws Exception {
         log = "";
         assertThat(testResult(SubStaticFieldShadowing.class), isSuccessful());
         assertEquals(
-                "super.rule.before() sub.rule.before() "
+                "sub.rule.before() "
                 + "Test "
-                + "sub.rule.after() super.rule.after() ",
+                + "sub.rule.after() ",
                 log);
     }