Skip to content

Introduce Converter in junit-platform-commons #4219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apiguardian.api.API;
import org.jspecify.annotations.Nullable;
import org.junit.jupiter.params.converter.DefaultArgumentConverter;
import org.junit.platform.commons.support.conversion.TypeDescriptor;
import org.junit.platform.commons.util.ClassUtils;
import org.junit.platform.commons.util.Preconditions;

Expand All @@ -45,7 +46,8 @@ public static DefaultArgumentsAccessor create(int invocationIndex, ClassLoader c
Preconditions.notNull(classLoader, "ClassLoader must not be null");

BiFunction<@Nullable Object, Class<?>, @Nullable Object> converter = (source,
targetType) -> DefaultArgumentConverter.INSTANCE.convert(source, targetType, classLoader);
targetType) -> DefaultArgumentConverter.INSTANCE.convert(source, TypeDescriptor.forClass(targetType),
classLoader);
return new DefaultArgumentsAccessor(converter, invocationIndex, arguments);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.params.support.FieldContext;
import org.junit.platform.commons.support.conversion.ConversionContext;
import org.junit.platform.commons.support.conversion.ConversionException;
import org.junit.platform.commons.support.conversion.ConversionSupport;
import org.junit.platform.commons.support.conversion.TypeDescriptor;
import org.junit.platform.commons.util.ReflectionUtils;

/**
Expand All @@ -41,7 +43,7 @@
* {@link File}, {@link BigDecimal}, {@link BigInteger}, {@link Currency},
* {@link Locale}, {@link URI}, {@link URL}, {@link UUID}, etc.
*
* <p>If the source and target types are identical the source object will not
* <p>If the source and target types are identical, the source object will not
* be modified.
*
* @since 5.0
Expand All @@ -58,49 +60,42 @@ private DefaultArgumentConverter() {

@Override
public final @Nullable Object convert(@Nullable Object source, ParameterContext context) {
Class<?> targetType = context.getParameter().getType();
ClassLoader classLoader = getClassLoader(context.getDeclaringExecutable().getDeclaringClass());
return convert(source, targetType, classLoader);
return convert(source, TypeDescriptor.forParameter(context.getParameter()), classLoader);
}

@Override
public final @Nullable Object convert(@Nullable Object source, FieldContext context)
throws ArgumentConversionException {

Class<?> targetType = context.getField().getType();
ClassLoader classLoader = getClassLoader(context.getField().getDeclaringClass());
return convert(source, targetType, classLoader);
return convert(source, TypeDescriptor.forField(context.getField()), classLoader);
}

public final @Nullable Object convert(@Nullable Object source, Class<?> targetType, ClassLoader classLoader) {
public final @Nullable Object convert(@Nullable Object source, TypeDescriptor targetType, ClassLoader classLoader) {
if (source == null) {
if (targetType.isPrimitive()) {
throw new ArgumentConversionException(
"Cannot convert null to primitive value of type " + targetType.getTypeName());
"Cannot convert null to primitive value of type " + targetType.getType().getTypeName());
}
return null;
}

if (ReflectionUtils.isAssignableTo(source, targetType)) {
if (ReflectionUtils.isAssignableTo(source, targetType.getType())) {
return source;
}

if (source instanceof String string) {
try {
return convert(string, targetType, classLoader);
}
catch (ConversionException ex) {
throw new ArgumentConversionException(ex.getMessage(), ex);
}
try {
ConversionContext context = new ConversionContext(source, targetType, classLoader);
return delegateConversion(source, context);
}
catch (ConversionException ex) {
throw new ArgumentConversionException(ex.getMessage(), ex);
}

throw new ArgumentConversionException("No built-in converter for source type %s and target type %s".formatted(
source.getClass().getTypeName(), targetType.getTypeName()));
}

@Nullable
Object convert(@Nullable String source, Class<?> targetType, ClassLoader classLoader) {
return ConversionSupport.convert(source, targetType, classLoader);
Object delegateConversion(@Nullable Object source, ConversionContext context) {
return ConversionSupport.convert(source, context);
}

}
1 change: 1 addition & 0 deletions junit-platform-commons/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@
org.junit.platform.suite.engine,
org.junit.platform.testkit,
org.junit.vintage.engine;
uses org.junit.platform.commons.support.conversion.Converter;
uses org.junit.platform.commons.support.scanning.ClasspathScanner;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2015-2025 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.platform.commons.support.conversion;

import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.junit.platform.commons.util.ClassLoaderUtils.getDefaultClassLoader;

import org.apiguardian.api.API;
import org.jspecify.annotations.Nullable;

/**
* {@code ConversionContext} encapsulates the <em>context</em> in which the
* current conversion is being executed.
*
* <p>{@link Converter Converters} are provided an instance of
* {@code ConversionContext} to perform their work.
*
* @param sourceType the descriptor of the source type
* @param targetType the descriptor of the type the source should be converted into
* @param classLoader the {@code ClassLoader} to use
*
* @since 6.0
* @see Converter
*/
@API(status = EXPERIMENTAL, since = "6.0")
public record ConversionContext(TypeDescriptor sourceType, TypeDescriptor targetType, ClassLoader classLoader) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the constructors need to be public so they can be used in unit tests for Converter implementations, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept them public for any code invoking ConversionSupport.convert(), which is not only DefaultArgumentConverter but potentially user code as ConversionSupport is a public API.


/**
* Create a new {@code ConversionContext}, expecting an instance of the
* source instead of its type descriptor.
*
* @param source the source instance; may be {@code null}
* @param targetType the descriptor of the type the source should be converted into
* @param classLoader the {@code ClassLoader} to use; may be {@code null} to
* use the default {@code ClassLoader}
*/
public ConversionContext(@Nullable Object source, TypeDescriptor targetType, @Nullable ClassLoader classLoader) {
this(TypeDescriptor.forInstance(source), targetType,
classLoader != null ? classLoader : getDefaultClassLoader());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.Serial;

import org.apiguardian.api.API;
import org.jspecify.annotations.Nullable;
import org.junit.platform.commons.JUnitException;

/**
Expand All @@ -33,7 +34,7 @@ public ConversionException(String message) {
super(message);
}

public ConversionException(String message, Throwable cause) {
public ConversionException(String message, @Nullable Throwable cause) {
super(message, cause);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@

package org.junit.platform.commons.support.conversion;

import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.apiguardian.api.API.Status.MAINTAINED;
import static org.junit.platform.commons.util.ReflectionUtils.getWrapperType;

import java.util.List;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import org.apiguardian.api.API;
import org.jspecify.annotations.Nullable;
import org.junit.platform.commons.util.ClassLoaderUtils;

/**
* {@code ConversionSupport} provides static utility methods for converting a
Expand All @@ -29,17 +30,6 @@
@API(status = MAINTAINED, since = "1.13.3")
public final class ConversionSupport {

private static final List<StringToObjectConverter> stringToObjectConverters = List.of( //
new StringToBooleanConverter(), //
new StringToCharacterConverter(), //
new StringToNumberConverter(), //
new StringToClassConverter(), //
new StringToEnumConverter(), //
new StringToJavaTimeConverter(), //
new StringToCommonJavaTypesConverter(), //
new FallbackStringToObjectConverter() //
);

private ConversionSupport() {
/* no-op */
}
Expand All @@ -48,43 +38,6 @@ private ConversionSupport() {
* Convert the supplied source {@code String} into an instance of the specified
* target type.
*
* <p>If the target type is {@code String}, the source {@code String} will not
* be modified.
*
* <p>Some forms of conversion require a {@link ClassLoader}. If none is
* provided, the {@linkplain ClassLoaderUtils#getDefaultClassLoader() default
* ClassLoader} will be used.
*
* <p>This method is able to convert strings into primitive types and their
* corresponding wrapper types ({@link Boolean}, {@link Character}, {@link Byte},
* {@link Short}, {@link Integer}, {@link Long}, {@link Float}, and
* {@link Double}), enum constants, date and time types from the
* {@code java.time} package, as well as common Java types such as {@link Class},
* {@link java.io.File}, {@link java.nio.file.Path}, {@link java.nio.charset.Charset},
* {@link java.math.BigDecimal}, {@link java.math.BigInteger},
* {@link java.util.Currency}, {@link java.util.Locale}, {@link java.util.UUID},
* {@link java.net.URI}, and {@link java.net.URL}.
*
* <p>If the target type is not covered by any of the above, a convention-based
* conversion strategy will be used to convert the source {@code String} into the
* given target type by invoking a static factory method or factory constructor
* defined in the target type. The search algorithm used in this strategy is
* outlined below.
*
* <h4>Search Algorithm</h4>
*
* <ol>
* <li>Search for a single, non-private static factory method in the target
* type that converts from a String to the target type. Use the factory method
* if present.</li>
* <li>Search for a single, non-private constructor in the target type that
* accepts a String. Use the constructor if present.</li>
* </ol>
*
* <p>If multiple suitable factory methods are discovered they will be ignored.
* If neither a single factory method nor a single constructor is found, the
* convention-based conversion strategy will not apply.
*
* @param source the source {@code String} to convert; may be {@code null}
* but only if the target type is a reference type
* @param targetType the target type the source should be converted into;
Expand All @@ -96,49 +49,44 @@ private ConversionSupport() {
* type is a reference type
*
* @since 1.11
* @see DefaultConverter
* @deprecated Use {@link #convert(Object, ConversionContext)} instead.
*/
@SuppressWarnings("unchecked")
@Deprecated
@API(status = DEPRECATED, since = "6.0")
public static <T> @Nullable T convert(@Nullable String source, Class<T> targetType,
@Nullable ClassLoader classLoader) {
if (source == null) {
if (targetType.isPrimitive()) {
throw new ConversionException(
"Cannot convert null to primitive value of type " + targetType.getTypeName());
}
return null;
}

if (String.class.equals(targetType)) {
return (T) source;
}
ConversionContext context = new ConversionContext(source, TypeDescriptor.forClass(targetType), classLoader);
return convert(source, context);
}

Class<?> targetTypeToUse = toWrapperType(targetType);
Optional<StringToObjectConverter> converter = stringToObjectConverters.stream().filter(
candidate -> candidate.canConvertTo(targetTypeToUse)).findFirst();
if (converter.isPresent()) {
try {
ClassLoader classLoaderToUse = classLoader != null ? classLoader
: ClassLoaderUtils.getDefaultClassLoader();
return (T) converter.get().convert(source, targetTypeToUse, classLoaderToUse);
}
catch (Exception ex) {
if (ex instanceof ConversionException conversionException) {
// simply rethrow it
throw conversionException;
}
// else
throw new ConversionException(
"Failed to convert String \"%s\" to type %s".formatted(source, targetType.getTypeName()), ex);
}
}
/**
* Convert the supplied source object into an instance of the specified
* target type.
*
* @param source the source object to convert; may be {@code null}
* but only if the target type is a reference type
* @param context the context for the conversion
* @param <T> the type of the target
* @return the converted object; may be {@code null} but only if the target
* type is a reference type
* @since 6.0
*/
@API(status = EXPERIMENTAL, since = "6.0")
@SuppressWarnings({ "unchecked", "rawtypes", "TypeParameterUnusedInFormals" })
public static <T> @Nullable T convert(@Nullable Object source, ConversionContext context) {
ServiceLoader<Converter> serviceLoader = ServiceLoader.load(Converter.class, context.classLoader());

throw new ConversionException(
"No built-in converter for source type java.lang.String and target type " + targetType.getTypeName());
}
Converter converter = Stream.concat( //
StreamSupport.stream(serviceLoader.spliterator(), false), //
Stream.of(DefaultConverter.INSTANCE)) //
Comment on lines +80 to +82
Copy link
Member

@marcphilipp marcphilipp Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether this is better. From its Javadoc and a quick look at the code the Iterator backing ServiceLoader.spliterator is also lazy. I'll leave it up to you.

Suggested change
Converter converter = Stream.concat( //
StreamSupport.stream(serviceLoader.spliterator(), false), //
Stream.of(DefaultConverter.INSTANCE)) //
Converter converter = Stream.<Supplier<Converter>> concat( //
serviceLoader.stream(), //
Stream.of(() -> DefaultConverter.INSTANCE)) //
.map(Supplier::get) //

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, I originally drew inspiration from other parts of JUnit here.

I'll double-check both options.

.filter(candidate -> candidate.canConvert(context)) //
.findFirst() //
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to solve this now, but I'm fully expecting us to get requests for some kind of ordering/precedence support in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an int order() default method in Converter could be done already now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hold off doing that for now to keep the PR size down.

.orElseThrow(() -> new ConversionException(
"No registered or built-in converter for source '%s' and target type %s".formatted( //
source, context.targetType())));

private static Class<?> toWrapperType(Class<?> targetType) {
Class<?> wrapperType = getWrapperType(targetType);
return wrapperType != null ? wrapperType : targetType;
return (T) converter.convert(source, context);
}

}
Loading
Loading