Process extensions registered on non-static fields earlier (#3895)

This PR changes extensions declared via @ExtendWith on non-static fields
to be registered before the test instance is instantiated. Since these 
declarative extensions are documented to be sorted along with 
programmatic extensions registered via @RegisterExtension the latter 
are also registered earlier. Since, however, non-static fields are not 
initialized until the test class is instantiated, this PR introduces the
internal concept of "uninitialized extensions" that are registered 
early and initialized whenever an instance of the test class is created.

Fixes #3437. Resolves #3463.

Co-authored-by: Sam Brannen <104798+sbrannen@users.noreply.github.com>

documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0-RC1.adoc

@@ -41,12 +41,15 @@ repository on GitHub.
 [[release-notes-5.11.0-RC1-junit-jupiter-bug-fixes]]
 ==== Bug Fixes
 
-* ❓
+* `TestInstancePostProcessor` extensions can now be registered via the `@ExtendWith`
+  annotation on non-static fields.
 
 [[release-notes-5.11.0-RC1-junit-jupiter-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes
 
-* ❓
+* The registration order of extensions was changed to allow non-static fields to be
+  processed earlier. This change may affect extensions that rely on the order of
+  registration.
 
 [[release-notes-5.11.0-RC1-junit-jupiter-new-features-and-improvements]]
 ==== New Features and Improvements

documentation/src/docs/asciidoc/user-guide/extensions.adoc

@@ -129,21 +129,9 @@ important to note which extension APIs are implemented and for what reasons.
 Specifically, `RandomNumberExtension` implements the following extension APIs:
 
 - `BeforeAllCallback`: to support static field injection
-- `BeforeEachCallback`: to support non-static field injection
+- `TestInstancePostProcessor`: to support non-static field injection
 - `ParameterResolver`: to support constructor and method injection
 
-[NOTE]
-====
-Ideally, the `RandomNumberExtension` would implement `TestInstancePostProcessor` instead
-of `BeforeEachCallback` in order to support non-static field injection immediately after
-the test class has been instantiated.
-
-However, JUnit Jupiter currently does not allow a `TestInstancePostProcessor` to be
-registered via `@ExtendWith` on a non-static field (see
-link:{junit5-repo}/issues/3437[issue 3437]). In light of that, the `RandomNumberExtension`
-implements `BeforeEachCallback` as an alternative approach.
-====
-
 [source,java,indent=0]
 ----
 include::{testDir}/example/extensions/RandomNumberExtension.java[tags=user_guide]
@@ -272,11 +260,8 @@ will be registered after the test class has been instantiated and after each reg
 (potentially injecting the instance of the extension to be used into the annotated
 field). Thus, if such an _instance extension_ implements class-level or instance-level
 extension APIs such as `BeforeAllCallback`, `AfterAllCallback`, or
-`TestInstancePostProcessor`, those APIs will not be honored. By default, an instance
-extension will be registered _after_ extensions that are registered at the method level
-via `@ExtendWith`; however, if the test class is configured with
-`@TestInstance(Lifecycle.PER_CLASS)` semantics, an instance extension will be registered
-_before_ extensions that are registered at the method level via `@ExtendWith`.
+`TestInstancePostProcessor`, those APIs will not be honored. Instance extensions will be
+registered _before_ extensions that are registered at the method level via `@ExtendWith`.
 
 In the following example, the `docs` field in the test class is initialized
 programmatically by invoking a custom `lookUpDocsDir()` method and supplying the result

documentation/src/test/java/example/extensions/RandomNumberExtension.java

@@ -18,17 +18,17 @@
 import java.util.function.Predicate;
 
 import org.junit.jupiter.api.extension.BeforeAllCallback;
-import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.ParameterContext;
 import org.junit.jupiter.api.extension.ParameterResolver;
+import org.junit.jupiter.api.extension.TestInstancePostProcessor;
 import org.junit.platform.commons.support.ModifierSupport;
 
 // end::user_guide[]
 // @formatter:off
 // tag::user_guide[]
 class RandomNumberExtension
-		implements BeforeAllCallback, BeforeEachCallback, ParameterResolver {
+		implements BeforeAllCallback, TestInstancePostProcessor, ParameterResolver {
 
 	private final java.util.Random random = new java.util.Random(System.nanoTime());
 
@@ -47,9 +47,8 @@ public void beforeAll(ExtensionContext context) {
 	 * {@code @Random} and can be assigned an integer value.
 	 */
 	@Override
-	public void beforeEach(ExtensionContext context) {
+	public void postProcessTestInstance(Object testInstance, ExtensionContext context) {
 		Class<?> testClass = context.getRequiredTestClass();
-		Object testInstance = context.getRequiredTestInstance();
 		injectFields(testClass, testInstance, ModifierSupport::isNotStatic);
 	}
 

junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/TestInstancePostProcessor.java

@@ -23,7 +23,9 @@
  * etc.
  *
  * <p>Extensions that implement {@code TestInstancePostProcessor} must be
- * registered at the class level.
+ * registered at the class level, {@linkplain ExtendWith declaratively} via a
+ * field of the test class, or {@linkplain RegisterExtension programmatically}
+ * via a <em>static</em> field of the test class.
  *
  * <h2>Constructor Requirements</h2>
  *

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java

@@ -15,7 +15,8 @@
 import static org.junit.jupiter.engine.descriptor.ExtensionUtils.populateNewExtensionRegistryFromExtendWithAnnotation;
 import static org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromConstructorParameters;
 import static org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromExecutableParameters;
-import static org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromFields;
+import static org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromInstanceFields;
+import static org.junit.jupiter.engine.descriptor.ExtensionUtils.registerExtensionsFromStaticFields;
 import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findAfterAllMethods;
 import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findAfterEachMethods;
 import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findBeforeAllMethods;
@@ -152,7 +153,7 @@ public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext conte
 
 		// Register extensions from static fields here, at the class level but
 		// after extensions registered via @ExtendWith.
-		registerExtensionsFromFields(registry, this.testClass, null);
+		registerExtensionsFromStaticFields(registry, this.testClass);
 
 		// Resolve the TestInstanceFactory at the class level in order to fail
 		// the entire class in case of configuration errors (e.g., more than
@@ -175,6 +176,7 @@ public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext conte
 		registerBeforeEachMethodAdapters(registry);
 		registerAfterEachMethodAdapters(registry);
 		this.afterAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
+		registerExtensionsFromInstanceFields(registry, this.testClass);
 
 		ThrowableCollector throwableCollector = createThrowableCollector();
 		ExecutableInvoker executableInvoker = new DefaultExecutableInvoker(context);
@@ -288,10 +290,10 @@ private TestInstances instantiateAndPostProcessTestInstance(JupiterEngineExecuti
 			throwableCollector);
 		throwableCollector.execute(() -> {
 			invokeTestInstancePostProcessors(instances.getInnermostInstance(), registry, extensionContext);
-			// In addition, we register extensions from instance fields here since the
-			// best time to do that is immediately following test class instantiation
-			// and post processing.
-			registerExtensionsFromFields(registrar, this.testClass, instances.getInnermostInstance());
+			// In addition, we initialize extension registered programmatically from instance fields here
+			// since the best time to do that is immediately following test class instantiation
+			// and post-processing.
+			registrar.initializeExtensions(this.testClass, instances.getInnermostInstance());
 		});
 		return instances;
 	}

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ExtensionUtils.java

@@ -35,6 +35,7 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.engine.extension.ExtensionRegistrar;
 import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
+import org.junit.platform.commons.PreconditionViolationException;
 import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.commons.util.ReflectionUtils;
 
@@ -71,60 +72,94 @@ static MutableExtensionRegistry populateNewExtensionRegistryFromExtendWithAnnota
 		Preconditions.notNull(parentRegistry, "Parent ExtensionRegistry must not be null");
 		Preconditions.notNull(annotatedElement, "AnnotatedElement must not be null");
 
-		return MutableExtensionRegistry.createRegistryFrom(parentRegistry, streamExtensionTypes(annotatedElement));
+		return MutableExtensionRegistry.createRegistryFrom(parentRegistry,
+			streamDeclarativeExtensionTypes(annotatedElement));
 	}
 
 	/**
-	 * Register extensions using the supplied registrar from fields in the supplied
-	 * class that are annotated with {@link ExtendWith @ExtendWith} or
-	 * {@link RegisterExtension @RegisterExtension}.
+	 * Register extensions using the supplied registrar from static fields in
+	 * the supplied class that are annotated with {@link ExtendWith @ExtendWith}
+	 * or {@link RegisterExtension @RegisterExtension}.
 	 *
 	 * <p>The extensions will be sorted according to {@link Order @Order} semantics
 	 * prior to registration.
 	 *
 	 * @param registrar the registrar with which to register the extensions; never {@code null}
 	 * @param clazz the class or interface in which to find the fields; never {@code null}
-	 * @param instance the instance of the supplied class; may be {@code null}
-	 * when searching for {@code static} fields in the class
+	 * @since 5.11
 	 */
-	static void registerExtensionsFromFields(ExtensionRegistrar registrar, Class<?> clazz, Object instance) {
-		Preconditions.notNull(registrar, "ExtensionRegistrar must not be null");
-		Preconditions.notNull(clazz, "Class must not be null");
+	static void registerExtensionsFromStaticFields(ExtensionRegistrar registrar, Class<?> clazz) {
+		streamExtensionRegisteringFields(clazz, ReflectionUtils::isStatic) //
+				.forEach(field -> {
+					List<Class<? extends Extension>> extensionTypes = streamDeclarativeExtensionTypes(field).collect(
+						toList());
+					boolean isExtendWithPresent = !extensionTypes.isEmpty();
 
-		Predicate<Field> predicate = (instance == null ? ReflectionUtils::isStatic : ReflectionUtils::isNotStatic);
+					if (isExtendWithPresent) {
+						extensionTypes.forEach(registrar::registerExtension);
+					}
+					if (isAnnotated(field, RegisterExtension.class)) {
+						Extension extension = readAndValidateExtensionFromField(field, null, extensionTypes);
+						registrar.registerExtension(extension, field);
+					}
+				});
+	}
 
-		streamFields(clazz, predicate, TOP_DOWN)//
-				.sorted(orderComparator)//
+	/**
+	 * Register extensions using the supplied registrar from instance fields in
+	 * the supplied class that are annotated with {@link ExtendWith @ExtendWith}
+	 * or {@link RegisterExtension @RegisterExtension}.
+	 *
+	 * <p>The extensions will be sorted according to {@link Order @Order} semantics
+	 * prior to registration.
+	 *
+	 * @param registrar the registrar with which to register the extensions; never {@code null}
+	 * @param clazz the class or interface in which to find the fields; never {@code null}
+	 * @since 5.11
+	 */
+	static void registerExtensionsFromInstanceFields(ExtensionRegistrar registrar, Class<?> clazz) {
+		streamExtensionRegisteringFields(clazz, ReflectionUtils::isNotStatic) //
 				.forEach(field -> {
-					List<Class<? extends Extension>> extensionTypes = streamExtensionTypes(field).collect(toList());
+					List<Class<? extends Extension>> extensionTypes = streamDeclarativeExtensionTypes(field).collect(
+						toList());
 					boolean isExtendWithPresent = !extensionTypes.isEmpty();
-					boolean isRegisterExtensionPresent = isAnnotated(field, RegisterExtension.class);
+
 					if (isExtendWithPresent) {
 						extensionTypes.forEach(registrar::registerExtension);
 					}
-					if (isRegisterExtensionPresent) {
-						tryToReadFieldValue(field, instance).ifSuccess(value -> {
-							Preconditions.condition(value instanceof Extension, () -> String.format(
-								"Failed to register extension via @RegisterExtension field [%s]: field value's type [%s] must implement an [%s] API.",
-								field, (value != null ? value.getClass().getName() : null), Extension.class.getName()));
-
-							if (isExtendWithPresent) {
-								Class<?> valueType = value.getClass();
-								extensionTypes.forEach(extensionType -> {
-									Preconditions.condition(!extensionType.equals(valueType),
-										() -> String.format("Failed to register extension via field [%s]. "
-												+ "The field registers an extension of type [%s] via @RegisterExtension and @ExtendWith, "
-												+ "but only one registration of a given extension type is permitted.",
-											field, valueType.getName()));
-								});
-							}
-
-							registrar.registerExtension((Extension) value, field);
-						});
+					if (isAnnotated(field, RegisterExtension.class)) {
+						registrar.registerUninitializedExtension(clazz, field,
+							instance -> readAndValidateExtensionFromField(field, instance, extensionTypes));
 					}
 				});
 	}
 
+	/**
+	 * @since 5.11
+	 */
+	private static Extension readAndValidateExtensionFromField(Field field, Object instance,
+			List<Class<? extends Extension>> declarativeExtensionTypes) {
+		Object value = tryToReadFieldValue(field, instance) //
+				.getOrThrow(e -> new PreconditionViolationException(
+					String.format("Failed to read @RegisterExtension field [%s]", field), e));
+
+		Preconditions.condition(value instanceof Extension, () -> String.format(
+			"Failed to register extension via @RegisterExtension field [%s]: field value's type [%s] must implement an [%s] API.",
+			field, (value != null ? value.getClass().getName() : null), Extension.class.getName()));
+
+		declarativeExtensionTypes.forEach(extensionType -> {
+			Class<?> valueType = value.getClass();
+			Preconditions.condition(!extensionType.equals(valueType),
+				() -> String.format(
+					"Failed to register extension via field [%s]. "
+							+ "The field registers an extension of type [%s] via @RegisterExtension and @ExtendWith, "
+							+ "but only one registration of a given extension type is permitted.",
+					field, valueType.getName()));
+		});
+
+		return (Extension) value;
+	}
+
 	/**
 	 * Register extensions using the supplied registrar from parameters in the
 	 * declared constructor of the supplied class that are annotated with
@@ -157,22 +192,34 @@ static void registerExtensionsFromExecutableParameters(ExtensionRegistrar regist
 		// @formatter:off
 		Arrays.stream(executable.getParameters())
 				.map(parameter -> findRepeatableAnnotations(parameter, index.getAndIncrement(), ExtendWith.class))
-				.flatMap(ExtensionUtils::streamExtensionTypes)
+				.flatMap(ExtensionUtils::streamDeclarativeExtensionTypes)
 				.forEach(registrar::registerExtension);
 		// @formatter:on
 	}
 
 	/**
-	 * @since 5.8
+	 * @since 5.11
 	 */
-	private static Stream<Class<? extends Extension>> streamExtensionTypes(AnnotatedElement annotatedElement) {
-		return streamExtensionTypes(findRepeatableAnnotations(annotatedElement, ExtendWith.class));
+	private static Stream<Field> streamExtensionRegisteringFields(Class<?> clazz, Predicate<Field> predicate) {
+		Predicate<Field> composedPredicate = predicate.and(
+			field -> isAnnotated(field, ExtendWith.class) || isAnnotated(field, RegisterExtension.class));
+		return streamFields(clazz, composedPredicate, TOP_DOWN)//
+				.sorted(orderComparator);
 	}
 
 	/**
-	 * @since 5.8
+	 * @since 5.11
+	 */
+	private static Stream<Class<? extends Extension>> streamDeclarativeExtensionTypes(
+			AnnotatedElement annotatedElement) {
+		return streamDeclarativeExtensionTypes(findRepeatableAnnotations(annotatedElement, ExtendWith.class));
+	}
+
+	/**
+	 * @since 5.11
 	 */
-	private static Stream<Class<? extends Extension>> streamExtensionTypes(List<ExtendWith> extendWithAnnotations) {
+	private static Stream<Class<? extends Extension>> streamDeclarativeExtensionTypes(
+			List<ExtendWith> extendWithAnnotations) {
 		return extendWithAnnotations.stream().map(ExtendWith::value).flatMap(Arrays::stream);
 	}
 

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java

@@ -74,7 +74,7 @@ public boolean mayRegisterTests() {
 	// --- Node ----------------------------------------------------------------
 
 	@Override
-	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) throws Exception {
+	public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) {
 		MutableExtensionRegistry registry = populateNewExtensionRegistryFromExtendWithAnnotation(
 			context.getExtensionRegistry(), getTestMethod());
 

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/ExtensionRegistrar.java

@@ -12,8 +12,12 @@
 
 import static org.apiguardian.api.API.Status.INTERNAL;
 
+import java.lang.reflect.Field;
+import java.util.function.Function;
+
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.extension.Extension;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 /**
  * An {@code ExtensionRegistrar} is used to register extensions.
@@ -45,11 +49,11 @@ public interface ExtensionRegistrar {
 	 * {@link org.junit.jupiter.api.extension.ExtendWith @ExtendWith}, the
 	 * {@code source} and the {@code extension} should be the same object.
 	 * However, if an extension is registered <em>programmatically</em> via
-	 * {@link org.junit.jupiter.api.extension.RegisterExtension @RegisterExtension},
-	 * the {@code source} object should be the {@link java.lang.reflect.Field}
-	 * that is annotated with {@code @RegisterExtension}. Similarly, if an
-	 * extension is registered <em>programmatically</em> as a lambda expression
-	 * or method reference, the {@code source} object should be the underlying
+	 * {@link RegisterExtension @RegisterExtension}, the {@code source} object
+	 * should be the {@link java.lang.reflect.Field} that is annotated with
+	 * {@code @RegisterExtension}. Similarly, if an extension is registered
+	 * <em>programmatically</em> as a lambda expression or method reference, the
+	 * {@code source} object should be the underlying
 	 * {@link java.lang.reflect.Method} that implements the extension API.
 	 *
 	 * @param extension the extension to register; never {@code null}
@@ -68,4 +72,34 @@ public interface ExtensionRegistrar {
 	 */
 	void registerSyntheticExtension(Extension extension, Object source);
 
+	/**
+	 * Register an uninitialized extension for the supplied {@code testClass} to
+	 * be initialized using the supplied {@code initializer} when an instance of
+	 * the test class is created.
+	 *
+	 * <p>Uninitialized extensions are typically registered for fields annotated
+	 * with {@link RegisterExtension @RegisterExtension} that cannot be
+	 * initialized until an instance of the test class is created. Until they
+	 * are initialized, such extensions are not available for use.
+	 *
+	 * @param testClass the test class for which the extension is registered;
+	 * never {@code null}
+	 * @param source the source of the extension; never {@code null}
+	 * @param initializer the initializer function to be used to create the
+	 * extension; never {@code null}
+	 */
+	void registerUninitializedExtension(Class<?> testClass, Field source,
+			Function<Object, ? extends Extension> initializer);
+
+	/**
+	 * Initialize all registered extensions for the supplied {@code testClass}
+	 * using the supplied {@code testInstance}.
+	 *
+	 * @param testClass the test class for which the extensions are initialized;
+	 * never {@code null}
+	 * @param testInstance the test instance to be used to initialize the
+	 * extensions; never {@code null}
+	 */
+	void initializeExtensions(Class<?> testClass, Object testInstance);
+
 }