TODO cleanup for 4.9 lead-up

src/main/java/org/junit/experimental/categories/Categories.java

@@ -62,6 +62,10 @@
  * </pre>
  */
 public class Categories extends Suite {
+	// TODO: the way filters are implemented makes this unnecessarily complicated,
+	//       buggy, and difficult to specify.  A new way of handling filters could
+	//       someday enable a better new implementation.
+
 	@Retention(RetentionPolicy.RUNTIME)
 	public @interface IncludeCategory {
 		public Class<?> value();
@@ -92,7 +96,6 @@ public String describe() {
 			return "category " + fIncluded;
 		}
 
-		// TODO: why do we have two CategoryFilters?
 		@Override
 		public boolean shouldRun(Description description) {
 			if (hasCorrectCategoryAnnotation(description))
@@ -123,8 +126,7 @@ private List<Class<?>> categories(Description description) {
 			return categories;
 		}
 
-		private Description parentDescription(Description description) {			
-			// TODO: how heavy are we cringing?
+		private Description parentDescription(Description description) {
 			Class<?> testClass= description.getTestClass();
 			if (testClass == null)
 				return null;
@@ -145,7 +147,6 @@ public Categories(Class<?> klass, RunnerBuilder builder)
 			throws InitializationError {
 		super(klass, builder);
 		try {
-			// TODO: too much work in constructors
 			filter(new CategoryFilter(getIncludedCategory(klass),
 					getExcludedCategory(klass)));
 		} catch (NoTestsRemainException e) {

src/main/java/org/junit/rules/ExternalResource.java

@@ -33,8 +33,6 @@
  * </pre>
  */
 public abstract class ExternalResource implements TestRule {
-	// TODO: validate that no field implements both BisectionRule and MethodRule?
-
 	public Statement apply(Statement base, Description description) {
 		return statement(base);
 	}

src/main/java/org/junit/runner/Result.java

@@ -16,7 +16,7 @@
 public class Result {
 	private AtomicInteger fCount = new AtomicInteger();
 	private AtomicInteger fIgnoreCount= new AtomicInteger();
-	private final List<Failure> fFailures= Collections.synchronizedList( new ArrayList<Failure>());
+	private final List<Failure> fFailures= Collections.synchronizedList(new ArrayList<Failure>());
 	private long fRunTime= 0;
 	private long fStartTime;
 

src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java

@@ -367,20 +367,31 @@ private Statement withRules(FrameworkMethod method, Object target,
 	@SuppressWarnings("deprecation")
 	private Statement withMethodRules(FrameworkMethod method, Object target,
 			Statement result) {
-		for (org.junit.rules.MethodRule each : getTestClass().getAnnotatedFieldValues(target,
-				Rule.class, org.junit.rules.MethodRule.class))
-			result= each.apply(result, method, target);
+		List<TestRule> testRules= getTestRules(target);
+		for (org.junit.rules.MethodRule each : getMethodRules(target))
+			if (! testRules.contains(each))
+				result= each.apply(result, method, target);
 		return result;
 	}
 
+	@SuppressWarnings("deprecation")
+	private List<org.junit.rules.MethodRule> getMethodRules(Object target) {
+		return getTestClass().getAnnotatedFieldValues(target,
+				Rule.class, org.junit.rules.MethodRule.class);
+	}
+
 	private Statement withTestRules(FrameworkMethod method, Object target,
 			Statement result) {
-		for (TestRule each : getTestClass().getAnnotatedFieldValues(target,
-				Rule.class, TestRule.class))
+		for (TestRule each : getTestRules(target))
 			result= each.apply(result, describeChild(method));
 		return result;
 	}
 
+	private List<TestRule> getTestRules(Object target) {
+		return getTestClass().getAnnotatedFieldValues(target,
+				Rule.class, TestRule.class);
+	}
+
 	private EachTestNotifier makeNotifier(FrameworkMethod method,
 			RunNotifier notifier) {
 		Description description= describeChild(method);

src/test/java/org/junit/tests/experimental/rules/TestRuleTest.java

@@ -19,6 +19,7 @@
 import org.junit.runner.Description;
 import org.junit.runner.JUnitCore;
 import org.junit.runner.Result;
+import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.Statement;
 
 public class TestRuleTest {
@@ -50,6 +51,36 @@ public void ruleIsIntroducedAndEvaluated() {
 		JUnitCore.runClasses(ExampleTest.class);
 		assertTrue(wasRun);
 	}
+
+	@SuppressWarnings("deprecation")
+	public static class BothKindsOfRule implements TestRule, org.junit.rules.MethodRule {
+		public int applications = 0;
+
+		public Statement apply(Statement base, FrameworkMethod method,
+				Object target) {
+			applications++;
+			return base;
+		}
+
+		public Statement apply(Statement base, Description description) {
+			applications++;
+			return base;
+		}
+	}
+
+	public static class OneFieldTwoKindsOfRule {
+		@Rule public BothKindsOfRule both = new BothKindsOfRule();
+
+		@Test public void onlyOnce() {
+			assertEquals(1, both.applications);
+		}
+	}
+
+
+	@Test
+	public void onlyApplyOnceEvenIfImplementsBothInterfaces() {
+		assertTrue(JUnitCore.runClasses(OneFieldTwoKindsOfRule.class).wasSuccessful());
+	}
 
 	public static class SonOfExampleTest extends ExampleTest {