From 514357f43c126fb14a0cc96149dbc020eaa452ac Mon Sep 17 00:00:00 2001 From: Stephen Colebourne Date: Sun, 30 Nov 2025 21:47:49 +0000 Subject: [PATCH 1/3] Extend fallback String-Object converter Extends the algorithm to exclude deprecated methods. This extension occurs after the existing logic completes. Fixes #4996 --- .../asciidoc/user-guide/writing-tests.adoc | 20 +- .../FallbackStringToObjectConverter.java | 36 ++- .../FallbackStringToObjectConverterTests.java | 274 +++++++++++++++++- 3 files changed, 310 insertions(+), 20 deletions(-) diff --git a/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc b/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc index 920b418c997e..b1308c3b72e7 100644 --- a/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc +++ b/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc @@ -2540,8 +2540,8 @@ integral types: `byte`, `short`, `int`, `long`, and their boxed counterparts. In addition to implicit conversion from strings to the target types listed in the above table, JUnit Jupiter also provides a fallback mechanism for automatic conversion from a -`String` to a given target type if the target type declares exactly one suitable _factory -method_ or a _factory constructor_ as defined below. +`String` to a given target type if the target type declares a suitable _factory method_ +or _factory constructor_ as defined below. - __factory method__: a non-private, `static` method declared in the target type that accepts either a single `String` argument or a single `CharSequence` argument and @@ -2551,9 +2551,19 @@ method_ or a _factory constructor_ as defined below. either a single `String` argument or a single `CharSequence` argument. Note that the target type must be declared as either a top-level class or as a `static` nested class. -NOTE: If multiple _factory methods_ are discovered, they will be ignored. If a _factory -method_ and a _factory constructor_ are discovered, the factory method will be used -instead of the constructor. +If there are multiple _factory methods_ or _factory constructors_, matching proceeds in the +following order: + +1. A single _factory method_ accepting a `String` argument. +2. A single _factory constructor_ accepting a `String` argument. +3. A single _factory method_ accepting a `CharSequence` argument. +4. A single _factory constructor_ accepting a `CharSequence` argument. +5. A single _factory method_ accepting a `String` argument once all `@Deprecated` factory + methods have been removed from the set of methods being considered. +6. A single _factory method_ accepting a `CharSequence` argument once all `@Deprecated` factory +methods have been removed from the set of methods being considered. + +NOTE: If multiple _factory methods_ are discovered, no match occurs. For example, in the following `@ParameterizedTest` method, the `Book` argument will be created by invoking the `Book.fromTitle(String)` factory method and passing `"42 Cats"` diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java index 80be72128211..06ec6dc89128 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java @@ -32,8 +32,8 @@ /** * {@code FallbackStringToObjectConverter} is a {@link StringToObjectConverter} * that provides a fallback conversion strategy for converting from a - * {@link String} to a given target type by invoking a static factory method - * or factory constructor defined in the target type. + * {@link String} or {@link CharSequence} to a given target type by invoking a + * static factory method or factory constructor defined in the target type. * *

Search Algorithm

* @@ -101,6 +101,11 @@ public boolean canConvertTo(Class targetType) { if (factory != null) { return factory; } + // Third, exclude deprecated methods + factory = findFactoryExcludeDeprecated(type); + if (factory != null) { + return factory; + } // Else, nothing found. return NULL_EXECUTABLE; }); @@ -109,7 +114,7 @@ public boolean canConvertTo(Class targetType) { private static @Nullable Function findFactoryExecutable(Class targetType, Class parameterType) { - Method factoryMethod = findFactoryMethod(targetType, parameterType); + Method factoryMethod = findFactoryMethod(targetType, parameterType, false); if (factoryMethod != null) { return source -> invokeMethod(factoryMethod, null, source); } @@ -120,9 +125,22 @@ public boolean canConvertTo(Class targetType) { return null; } - private static @Nullable Method findFactoryMethod(Class targetType, Class parameterType) { - List factoryMethods = findMethods(targetType, new IsFactoryMethod(targetType, parameterType), - BOTTOM_UP); + private static @Nullable Function findFactoryExcludeDeprecated(Class targetType) { + Method factoryMethodStr = findFactoryMethod(targetType, String.class, true); + if (factoryMethodStr != null) { + return source -> invokeMethod(factoryMethodStr, null, source); + } + Method factoryMethodChSeq = findFactoryMethod(targetType, CharSequence.class, true); + if (factoryMethodChSeq != null) { + return source -> invokeMethod(factoryMethodChSeq, null, source); + } + return null; + } + + private static @Nullable Method findFactoryMethod(Class targetType, Class parameterType, + boolean excludeDeprecated) { + List factoryMethods = findMethods(targetType, + new IsFactoryMethod(targetType, parameterType, excludeDeprecated), BOTTOM_UP); if (factoryMethods.size() == 1) { return factoryMethods.get(0); } @@ -143,7 +161,8 @@ public boolean canConvertTo(Class targetType) { * {@link #test(Method)} is a non-private static factory method for the * supplied {@link #targetType} and {@link #parameterType}. */ - record IsFactoryMethod(Class targetType, Class parameterType) implements Predicate { + record IsFactoryMethod(Class targetType, Class parameterType, boolean excludeDeprecated) + implements Predicate { @Override public boolean test(Method method) { @@ -154,6 +173,9 @@ public boolean test(Method method) { if (isNotStatic(method)) { return false; } + if (excludeDeprecated && method.getAnnotation(Deprecated.class) != null) { + return false; + } return isFactoryCandidate(method, this.parameterType); } } diff --git a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java index 64b028cb6808..e440c0d260b2 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java @@ -32,7 +32,7 @@ */ class FallbackStringToObjectConverterTests { - private static final IsFactoryMethod isBookFactoryMethod = new IsFactoryMethod(Book.class, String.class); + private static final IsFactoryMethod isBookFactoryMethod = new IsFactoryMethod(Book.class, String.class, false); private static final FallbackStringToObjectConverter converter = new FallbackStringToObjectConverter(); @@ -41,7 +41,8 @@ void isNotFactoryMethodForWrongParameterType() { assertThat(isBookFactoryMethod).rejects(bookMethod("factory", Object.class)); assertThat(isBookFactoryMethod).rejects(bookMethod("factory", Number.class)); assertThat(isBookFactoryMethod).rejects(bookMethod("factory", StringBuilder.class)); - assertThat(new IsFactoryMethod(Record2.class, String.class)).rejects(record2Method("from")); + assertThat(new IsFactoryMethod(Record2.class, String.class, false)).rejects(record2Method("from")); + assertThat(new IsFactoryMethod(Record2.class, String.class, true)).rejects(record2Method("from")); } @Test @@ -55,17 +56,54 @@ void isNotFactoryMethodForNonStaticMethod() { } @Test - void isFactoryMethodForValidMethods() { - assertThat(new IsFactoryMethod(Book.class, String.class))// + void isFactoryMethodForValidMethodsNoDeprecated() { + assertThat(new IsFactoryMethod(Book.class, String.class, false))// .accepts(bookMethod("factory", String.class)); - assertThat(new IsFactoryMethod(Book.class, CharSequence.class))// + assertThat(new IsFactoryMethod(Book.class, String.class, true))// + .accepts(bookMethod("factory", String.class)); + + assertThat(new IsFactoryMethod(Book.class, CharSequence.class, false))// + .accepts(bookMethod("factory", CharSequence.class)); + assertThat(new IsFactoryMethod(Book.class, CharSequence.class, true))// .accepts(bookMethod("factory", CharSequence.class)); - assertThat(new IsFactoryMethod(Newspaper.class, String.class))// + + assertThat(new IsFactoryMethod(Newspaper.class, String.class, false))// + .accepts(newspaperMethod("from"), newspaperMethod("of")); + assertThat(new IsFactoryMethod(Newspaper.class, String.class, true))// .accepts(newspaperMethod("from"), newspaperMethod("of")); - assertThat(new IsFactoryMethod(Magazine.class, String.class))// + + assertThat(new IsFactoryMethod(Magazine.class, String.class, false))// + .accepts(magazineMethod("from"), magazineMethod("of")); + assertThat(new IsFactoryMethod(Magazine.class, String.class, true))// .accepts(magazineMethod("from"), magazineMethod("of")); - assertThat(new IsFactoryMethod(Record2.class, CharSequence.class))// + + assertThat(new IsFactoryMethod(Record2.class, CharSequence.class, false))// .accepts(record2Method("from")); + assertThat(new IsFactoryMethod(Record2.class, CharSequence.class, true))// + .accepts(record2Method("from")); + } + + @Test + void isFactoryMethodForValidMethodsWithDeprecated() { + assertThat(new IsFactoryMethod(Book2.class, String.class, false))// + .accepts(bookWithDeprecatedMethod("factory", String.class)); + assertThat(new IsFactoryMethod(Book2.class, String.class, true))// + .accepts(bookWithDeprecatedMethod("factory", String.class)); + + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, false))// + .accepts(bookWithDeprecatedMethod("factory", CharSequence.class)); + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, true))// + .accepts(bookWithDeprecatedMethod("factory", CharSequence.class)); + + assertThat(new IsFactoryMethod(Book2.class, String.class, false))// + .accepts(bookWithDeprecatedMethod("factoryDeprecated", String.class)); + assertThat(new IsFactoryMethod(Book2.class, String.class, true))// + .rejects(bookWithDeprecatedMethod("factoryDeprecated", String.class)); + + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, false))// + .accepts(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, true))// + .rejects(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); } @Test @@ -100,6 +138,12 @@ void convertsStringToBookViaStaticFactoryMethod() throws Exception { assertConverts("enigma", Book.class, new Book("factory(String): enigma")); } + @Test + void convertsStringToBookWithDeprecatedViaConstructor() throws Exception { + // constructor takes precedence over factory method when there are two factory methods, and one is deprecated + assertConverts("enigma", Book2.class, new Book2("enigma")); + } + @Test void convertsStringToRecord2ViaStaticFactoryMethodAcceptingCharSequence() throws Exception { assertConvertsRecord2("enigma", Record2.from(new StringBuffer("enigma"))); @@ -120,6 +164,24 @@ void convertsStringToNewspaperViaConstructorIgnoringMultipleFactoryMethods() thr assertConverts("enigma", Newspaper.class, new Newspaper("enigma")); } + @Test + void convertsDeprecatedToNewspaper() throws Exception { + // when only one method @Deprecated is irrelevant, @Deprecated from(String) > @Deprecated from(CharSequence) + assertConverts("enigma", Newspaper1.class, new Newspaper1("from(String): enigma")); + } + + @Test + void convertsToNewspaperPreferNonDeprecatedToDeprecated() throws Exception { + // when two String factories: parse(String) > @Deprecated from(String) + assertConverts("enigma", Newspaper2.class, new Newspaper2("parse(String): enigma")); + } + + @Test + void convertsToNewspaperPreferOnlyCharSequenceToNonDeprecatedString() throws Exception { + // when two String and one CharSequence factories: parse(CharSequence) > parse(String)/@Deprecated from(String) + assertConverts("enigma", Newspaper3.class, new Newspaper3("parse(CharSequence): enigma")); + } + @Test @DisplayName("Cannot convert String to Diary because Diary has neither a static factory method nor a factory constructor") void cannotConvertStringToDiary() { @@ -147,6 +209,10 @@ private static Method bookMethod(String methodName, Class parameterType) { return findMethod(Book.class, methodName, parameterType).orElseThrow(); } + private static Method bookWithDeprecatedMethod(String methodName, Class parameterType) { + return findMethod(Book2.class, methodName, parameterType).orElseThrow(); + } + private static Method newspaperMethod(String methodName) { return findMethod(Newspaper.class, methodName, String.class).orElseThrow(); } @@ -352,4 +418,196 @@ static Record2 from(CharSequence title) { static class Diary { } + static class Book2 { + + private final String title; + + Book2(String title) { + this.title = title; + } + + static Book2 factory(String title) { + return new Book2("factory(String): " + title); + } + + @Deprecated + static Book2 factoryDeprecated(String title) { + return new Book2("factoryDeprecated(String): " + title); + } + + /** + * Static and non-private, but intentionally overloads {@link #factory(String)} + * with a {@link CharSequence} argument to ensure that we don't introduce a + * regression in 6.0, since the String-based factory method should take + * precedence over a CharSequence-based factory method. + */ + static Book2 factory(CharSequence title) { + return new Book2("factory(CharSequence): " + title); + } + + @Deprecated + static Book2 factoryDeprecated(CharSequence title) { + return new Book2("factoryDeprecated(CharSequence): " + title); + } + + // wrong parameter type + static Book2 factory(Object obj) { + throw new UnsupportedOperationException(); + } + + // wrong parameter type + static Book2 factory(Number number) { + throw new UnsupportedOperationException(); + } + + /** + * Wrong parameter type, intentionally a subtype of {@link CharSequence} + * other than {@link String}. + */ + static Book2 factory(StringBuilder builder) { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("unused") + private static Book2 privateFactory(String title) { + return new Book2(title); + } + + Book2 nonStaticFactory(String title) { + return new Book2(title); + } + + @Override + public boolean equals(Object obj) { + return (this == obj) || (obj instanceof Book2 that && Objects.equals(this.title, that.title)); + } + + @Override + public int hashCode() { + return Objects.hash(title); + } + + @Override + public String toString() { + return "Book2 [title=" + this.title + "]"; + } + } + + static class Newspaper1 { + + private final String title; + + private Newspaper1(String title) { + // constructor must be private for factory/deprecated logic to kick in + this.title = title; + } + + // only String factory, thus being deprecated is irrelevant + @Deprecated + static Newspaper1 from(String title) { + return new Newspaper1("from(String): " + title); + } + + // only CharSequence factory, thus being deprecated is irrelevant, but String version takes precedence + @Deprecated + static Newspaper1 from(CharSequence title) { + return new Newspaper1("from(CharSequence): " + title); + } + + @Override + public boolean equals(Object obj) { + return (this == obj) || (obj instanceof Newspaper1 that && Objects.equals(this.title, that.title)); + } + + @Override + public int hashCode() { + return Objects.hash(title); + } + + @Override + public String toString() { + return "Newspaper1 [title=" + this.title + "]"; + } + } + + static class Newspaper2 { + + private final String title; + + private Newspaper2(String title) { + // constructor must be private for factory/deprecated logic to kick in + this.title = title; + } + + @Deprecated + static Newspaper2 from(String title) { + return new Newspaper2("from(String): " + title); + } + + @Deprecated + static Newspaper2 other(String title) { + return new Newspaper2("parse(CharSequence): " + title); + } + + // String factory without deprecated has precedence over String factory with deprecated + static Newspaper2 parse(String title) { + return new Newspaper2("parse(String): " + title); + } + + @Override + public boolean equals(Object obj) { + return (this == obj) || (obj instanceof Newspaper2 that && Objects.equals(this.title, that.title)); + } + + @Override + public int hashCode() { + return Objects.hash(title); + } + + @Override + public String toString() { + return "Newspaper2 [title=" + this.title + "]"; + } + } + + static class Newspaper3 { + + private final String title; + + private Newspaper3(String title) { + // constructor must be private for factory/deprecated logic to kick in + this.title = title; + } + + @Deprecated + static Newspaper3 from(String title) { + return new Newspaper3("from(String): " + title); + } + + static Newspaper3 parse(String title) { + return new Newspaper3("parse(String): " + title); + } + + // CharSequence factory without deprecated alternative has precedence + // over String factory with deprecated alternative + static Newspaper3 parse(CharSequence title) { + return new Newspaper3("parse(CharSequence): " + title); + } + + @Override + public boolean equals(Object obj) { + return (this == obj) || (obj instanceof Newspaper3 that && Objects.equals(this.title, that.title)); + } + + @Override + public int hashCode() { + return Objects.hash(title); + } + + @Override + public String toString() { + return "Newspaper3 [title=" + this.title + "]"; + } + } + } From b6b982bf3d0d4c8fa232de1ab5bac7817d95ba6d Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Wed, 10 Dec 2025 18:18:49 +0100 Subject: [PATCH 2/3] Match structure of findFactoryExecutable to functional description --- .../FallbackStringToObjectConverter.java | 61 +++++++++++-------- .../FallbackStringToObjectConverterTests.java | 45 +++++++------- 2 files changed, 60 insertions(+), 46 deletions(-) diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java index 06ec6dc89128..b5960eaed539 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java @@ -15,6 +15,8 @@ import static org.junit.platform.commons.support.ModifierSupport.isNotStatic; import static org.junit.platform.commons.support.ReflectionSupport.findMethods; import static org.junit.platform.commons.support.ReflectionSupport.invokeMethod; +import static org.junit.platform.commons.support.conversion.FallbackStringToObjectConverter.DeprecationStatus.EXCLUDE_DEPRECATED; +import static org.junit.platform.commons.support.conversion.FallbackStringToObjectConverter.DeprecationStatus.INCLUDE_DEPRECATED; import static org.junit.platform.commons.util.ReflectionUtils.findConstructors; import static org.junit.platform.commons.util.ReflectionUtils.newInstance; @@ -92,17 +94,29 @@ public boolean canConvertTo(Class targetType) { private static Function findFactoryExecutable(Class targetType) { return factoryExecutableCache.computeIfAbsent(targetType, type -> { // First, search for exact String argument matches. - Function factory = findFactoryExecutable(type, String.class); + var factory = findFactoryMethodExecutable(type, String.class, INCLUDE_DEPRECATED); + if (factory != null) { + return factory; + } + factory = findFactoryConstructorExecutable(type, String.class); if (factory != null) { return factory; } // Second, fall back to CharSequence argument matches. - factory = findFactoryExecutable(type, CharSequence.class); + factory = findFactoryMethodExecutable(type, CharSequence.class, INCLUDE_DEPRECATED); + if (factory != null) { + return factory; + } + factory = findFactoryConstructorExecutable(type, CharSequence.class); if (factory != null) { return factory; } - // Third, exclude deprecated methods - factory = findFactoryExcludeDeprecated(type); + // Third, try factory methods again, but exclude deprecated methods + factory = findFactoryMethodExecutable(type, String.class, EXCLUDE_DEPRECATED); + if (factory != null) { + return factory; + } + factory = findFactoryMethodExecutable(type, CharSequence.class, EXCLUDE_DEPRECATED); if (factory != null) { return factory; } @@ -111,36 +125,28 @@ public boolean canConvertTo(Class targetType) { }); } - private static @Nullable Function findFactoryExecutable(Class targetType, - Class parameterType) { - - Method factoryMethod = findFactoryMethod(targetType, parameterType, false); + private static @Nullable Function findFactoryMethodExecutable(Class targetType, + Class parameterType, DeprecationStatus deprecationStatus) { + Method factoryMethod = findFactoryMethod(targetType, parameterType, deprecationStatus); if (factoryMethod != null) { return source -> invokeMethod(factoryMethod, null, source); } - Constructor constructor = findFactoryConstructor(targetType, parameterType); - if (constructor != null) { - return source -> newInstance(constructor, source); - } return null; } - private static @Nullable Function findFactoryExcludeDeprecated(Class targetType) { - Method factoryMethodStr = findFactoryMethod(targetType, String.class, true); - if (factoryMethodStr != null) { - return source -> invokeMethod(factoryMethodStr, null, source); - } - Method factoryMethodChSeq = findFactoryMethod(targetType, CharSequence.class, true); - if (factoryMethodChSeq != null) { - return source -> invokeMethod(factoryMethodChSeq, null, source); + private static @Nullable Function findFactoryConstructorExecutable(Class targetType, + Class parameterType) { + Constructor constructor = findFactoryConstructor(targetType, parameterType); + if (constructor != null) { + return source -> newInstance(constructor, source); } return null; } private static @Nullable Method findFactoryMethod(Class targetType, Class parameterType, - boolean excludeDeprecated) { - List factoryMethods = findMethods(targetType, - new IsFactoryMethod(targetType, parameterType, excludeDeprecated), BOTTOM_UP); + DeprecationStatus deprecationStatus) { + var isFactoryMethod = new IsFactoryMethod(targetType, parameterType, deprecationStatus); + List factoryMethods = findMethods(targetType, isFactoryMethod, BOTTOM_UP); if (factoryMethods.size() == 1) { return factoryMethods.get(0); } @@ -156,12 +162,16 @@ public boolean canConvertTo(Class targetType) { return null; } + enum DeprecationStatus { + INCLUDE_DEPRECATED, EXCLUDE_DEPRECATED + } + /** * {@link Predicate} that determines if the {@link Method} supplied to * {@link #test(Method)} is a non-private static factory method for the * supplied {@link #targetType} and {@link #parameterType}. */ - record IsFactoryMethod(Class targetType, Class parameterType, boolean excludeDeprecated) + record IsFactoryMethod(Class targetType, Class parameterType, DeprecationStatus deprecationStatus) implements Predicate { @Override @@ -173,7 +183,8 @@ public boolean test(Method method) { if (isNotStatic(method)) { return false; } - if (excludeDeprecated && method.getAnnotation(Deprecated.class) != null) { + if (deprecationStatus == DeprecationStatus.EXCLUDE_DEPRECATED + && method.getAnnotation(Deprecated.class) != null) { return false; } return isFactoryCandidate(method, this.parameterType); diff --git a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java index e440c0d260b2..d6c0263eaa0c 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java @@ -12,6 +12,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.platform.commons.support.ReflectionSupport.findMethod; +import static org.junit.platform.commons.support.conversion.FallbackStringToObjectConverter.DeprecationStatus.EXCLUDE_DEPRECATED; +import static org.junit.platform.commons.support.conversion.FallbackStringToObjectConverter.DeprecationStatus.INCLUDE_DEPRECATED; import static org.junit.platform.commons.util.ReflectionUtils.getDeclaredConstructor; import java.lang.reflect.Constructor; @@ -32,7 +34,8 @@ */ class FallbackStringToObjectConverterTests { - private static final IsFactoryMethod isBookFactoryMethod = new IsFactoryMethod(Book.class, String.class, false); + private static final IsFactoryMethod isBookFactoryMethod = new IsFactoryMethod(Book.class, String.class, + INCLUDE_DEPRECATED); private static final FallbackStringToObjectConverter converter = new FallbackStringToObjectConverter(); @@ -41,8 +44,8 @@ void isNotFactoryMethodForWrongParameterType() { assertThat(isBookFactoryMethod).rejects(bookMethod("factory", Object.class)); assertThat(isBookFactoryMethod).rejects(bookMethod("factory", Number.class)); assertThat(isBookFactoryMethod).rejects(bookMethod("factory", StringBuilder.class)); - assertThat(new IsFactoryMethod(Record2.class, String.class, false)).rejects(record2Method("from")); - assertThat(new IsFactoryMethod(Record2.class, String.class, true)).rejects(record2Method("from")); + assertThat(new IsFactoryMethod(Record2.class, String.class, INCLUDE_DEPRECATED)).rejects(record2Method("from")); + assertThat(new IsFactoryMethod(Record2.class, String.class, EXCLUDE_DEPRECATED)).rejects(record2Method("from")); } @Test @@ -57,52 +60,52 @@ void isNotFactoryMethodForNonStaticMethod() { @Test void isFactoryMethodForValidMethodsNoDeprecated() { - assertThat(new IsFactoryMethod(Book.class, String.class, false))// + assertThat(new IsFactoryMethod(Book.class, String.class, INCLUDE_DEPRECATED))// .accepts(bookMethod("factory", String.class)); - assertThat(new IsFactoryMethod(Book.class, String.class, true))// + assertThat(new IsFactoryMethod(Book.class, String.class, EXCLUDE_DEPRECATED))// .accepts(bookMethod("factory", String.class)); - assertThat(new IsFactoryMethod(Book.class, CharSequence.class, false))// + assertThat(new IsFactoryMethod(Book.class, CharSequence.class, INCLUDE_DEPRECATED))// .accepts(bookMethod("factory", CharSequence.class)); - assertThat(new IsFactoryMethod(Book.class, CharSequence.class, true))// + assertThat(new IsFactoryMethod(Book.class, CharSequence.class, EXCLUDE_DEPRECATED))// .accepts(bookMethod("factory", CharSequence.class)); - assertThat(new IsFactoryMethod(Newspaper.class, String.class, false))// + assertThat(new IsFactoryMethod(Newspaper.class, String.class, INCLUDE_DEPRECATED))// .accepts(newspaperMethod("from"), newspaperMethod("of")); - assertThat(new IsFactoryMethod(Newspaper.class, String.class, true))// + assertThat(new IsFactoryMethod(Newspaper.class, String.class, EXCLUDE_DEPRECATED))// .accepts(newspaperMethod("from"), newspaperMethod("of")); - assertThat(new IsFactoryMethod(Magazine.class, String.class, false))// + assertThat(new IsFactoryMethod(Magazine.class, String.class, INCLUDE_DEPRECATED))// .accepts(magazineMethod("from"), magazineMethod("of")); - assertThat(new IsFactoryMethod(Magazine.class, String.class, true))// + assertThat(new IsFactoryMethod(Magazine.class, String.class, EXCLUDE_DEPRECATED))// .accepts(magazineMethod("from"), magazineMethod("of")); - assertThat(new IsFactoryMethod(Record2.class, CharSequence.class, false))// + assertThat(new IsFactoryMethod(Record2.class, CharSequence.class, INCLUDE_DEPRECATED))// .accepts(record2Method("from")); - assertThat(new IsFactoryMethod(Record2.class, CharSequence.class, true))// + assertThat(new IsFactoryMethod(Record2.class, CharSequence.class, EXCLUDE_DEPRECATED))// .accepts(record2Method("from")); } @Test void isFactoryMethodForValidMethodsWithDeprecated() { - assertThat(new IsFactoryMethod(Book2.class, String.class, false))// + assertThat(new IsFactoryMethod(Book2.class, String.class, INCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factory", String.class)); - assertThat(new IsFactoryMethod(Book2.class, String.class, true))// + assertThat(new IsFactoryMethod(Book2.class, String.class, EXCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factory", String.class)); - assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, false))// + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, INCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factory", CharSequence.class)); - assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, true))// + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, EXCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factory", CharSequence.class)); - assertThat(new IsFactoryMethod(Book2.class, String.class, false))// + assertThat(new IsFactoryMethod(Book2.class, String.class, INCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factoryDeprecated", String.class)); - assertThat(new IsFactoryMethod(Book2.class, String.class, true))// + assertThat(new IsFactoryMethod(Book2.class, String.class, EXCLUDE_DEPRECATED))// .rejects(bookWithDeprecatedMethod("factoryDeprecated", String.class)); - assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, false))// + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, INCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); - assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, true))// + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, EXCLUDE_DEPRECATED))// .rejects(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); } From 417f9a11e504f114fed32600f6e36bf8b20822fc Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Wed, 10 Dec 2025 19:08:41 +0100 Subject: [PATCH 3/3] Add missing test coverage --- .../FallbackStringToObjectConverterTests.java | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java index d6c0263eaa0c..041556934a26 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java @@ -97,6 +97,8 @@ void isFactoryMethodForValidMethodsWithDeprecated() { .accepts(bookWithDeprecatedMethod("factory", CharSequence.class)); assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, EXCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factory", CharSequence.class)); + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, EXCLUDE_DEPRECATED))// + .rejects(bookWithDeprecatedMethod("factory", StringBuilder.class)); assertThat(new IsFactoryMethod(Book2.class, String.class, INCLUDE_DEPRECATED))// .accepts(bookWithDeprecatedMethod("factoryDeprecated", String.class)); @@ -107,6 +109,8 @@ void isFactoryMethodForValidMethodsWithDeprecated() { .accepts(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, EXCLUDE_DEPRECATED))// .rejects(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); + assertThat(new IsFactoryMethod(Book2.class, CharSequence.class, EXCLUDE_DEPRECATED))// + .rejects(bookWithDeprecatedMethod("factoryDeprecated", CharSequence.class)); } @Test @@ -120,6 +124,8 @@ void isNotFactoryConstructorForWrongParameterType() { .rejects(getDeclaredConstructor(Record1.class)); assertThat(new IsFactoryConstructor(Record2.class, String.class))// .rejects(getDeclaredConstructor(Record2.class)); + assertThat(new IsFactoryConstructor(Record3.class, String.class))// + .rejects(getDeclaredConstructor(Record3.class)); } @Test @@ -185,6 +191,12 @@ void convertsToNewspaperPreferOnlyCharSequenceToNonDeprecatedString() throws Exc assertConverts("enigma", Newspaper3.class, new Newspaper3("parse(CharSequence): enigma")); } + @Test + void convertsToNewspaperPreferOnlyCharSequenceToDeprecatedString() throws Exception { + // when two String and two CharSequence factories: parse(CharSequence) > @Deprecated parse(String)/@Deprecated from(String)/@Deprecated from(CharSequence) + assertConverts("enigma", Newspaper4.class, new Newspaper4("parse(CharSequence): enigma")); + } + @Test @DisplayName("Cannot convert String to Diary because Diary has neither a static factory method nor a factory constructor") void cannotConvertStringToDiary() { @@ -418,6 +430,13 @@ static Record2 from(CharSequence title) { } } + record Record3(StringBuilder title) { + + static Record2 from(StringBuilder title) { + return new Record2("Record2(StringBuilder): " + title); + } + } + static class Diary { } @@ -613,4 +632,50 @@ public String toString() { } } + static class Newspaper4 { + + private final String title; + + private Newspaper4(String title) { + // constructor must be private for factory/deprecated logic to kick in + this.title = title; + } + + @Deprecated + static Newspaper4 from(String title) { + return new Newspaper4("from(String): " + title); + } + + @Deprecated + static Newspaper4 parse(String title) { + return new Newspaper4("parse(String): " + title); + } + + @Deprecated + static Newspaper4 from(CharSequence title) { + return new Newspaper4("from(CharSequence): " + title); + } + + // CharSequence factory with deprecated alternative has precedence + // over deprecated String factory + static Newspaper4 parse(CharSequence title) { + return new Newspaper4("parse(CharSequence): " + title); + } + + @Override + public boolean equals(Object obj) { + return (this == obj) || (obj instanceof Newspaper4 that && Objects.equals(this.title, that.title)); + } + + @Override + public int hashCode() { + return Objects.hash(title); + } + + @Override + public String toString() { + return "Newspaper4 [title=" + this.title + "]"; + } + } + }