From 6ca8fbc2daa4f64d960088882b8b3e8e7e4f7425 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sat, 13 Jul 2024 14:34:24 +0200 Subject: [PATCH] Merge ArgumentsMatchInfo into ArgumentsMatchKind in SpEL The internal ArgumentsMatchInfo type seems to have once had additional methods beyond the functionality provided by the ArgumentsMatchKind enum; however, that is no longer the case. Consequently, there is no need to maintain both types. This commit therefore merges the convenience methods from ArgumentsMatchInfo into the ArgumentsMatchKind enum and removes the unnecessary ArgumentsMatchInfo type. --- .../spel/support/ReflectionHelper.java | 50 ++++++++----------- .../ReflectiveConstructorResolver.java | 15 +++--- .../support/ReflectiveMethodResolver.java | 15 +++--- .../spel/support/ReflectionHelperTests.java | 26 +++++----- 4 files changed, 50 insertions(+), 56 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index bb2c188d519..0868bb2fbe7 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -58,7 +58,7 @@ public abstract class ReflectionHelper { * or {@code null} if it was not a match */ @Nullable - static ArgumentsMatchInfo compareArguments( + static ArgumentsMatchKind compareArguments( List expectedArgTypes, List suppliedArgTypes, TypeConverter typeConverter) { Assert.isTrue(expectedArgTypes.size() == suppliedArgTypes.size(), @@ -88,7 +88,7 @@ public abstract class ReflectionHelper { } } } - return (match != null ? new ArgumentsMatchInfo(match) : null); + return match; } /** @@ -143,11 +143,11 @@ public abstract class ReflectionHelper { * @param expectedArgTypes the types the method/constructor is expecting * @param suppliedArgTypes the types that are being supplied at the point of invocation * @param typeConverter a registered type converter - * @return an {@code ArgumentsMatchInfo} object indicating what kind of match it was, + * @return an {@code ArgumentsMatchKind} object indicating what kind of match it was, * or {@code null} if it was not a match */ @Nullable - static ArgumentsMatchInfo compareArgumentsVarargs( + static ArgumentsMatchKind compareArgumentsVarargs( List expectedArgTypes, List suppliedArgTypes, TypeConverter typeConverter) { Assert.isTrue(!CollectionUtils.isEmpty(expectedArgTypes), @@ -231,7 +231,7 @@ public abstract class ReflectionHelper { } } - return (match != null ? new ArgumentsMatchInfo(match) : null); + return match; } /** @@ -507,45 +507,37 @@ public abstract class ReflectionHelper { /** - * Arguments match kinds. + * {@code ArgumentsMatchKind} describes what kind of match was achieved between two sets + * of arguments: the set that a method/constructor is expecting and the set that is being + * supplied at the point of invocation. */ enum ArgumentsMatchKind { - /** An exact match is where the parameter types exactly match what the method/constructor is expecting. */ + /** + * An exact match is where the parameter types exactly match what the method/constructor is expecting. + */ EXACT, - /** A close match is where the parameter types either exactly match or are assignment-compatible. */ + /** + * A close match is where the parameter types either exactly match or are assignment-compatible. + */ CLOSE, - /** A conversion match is where the type converter must be used to transform some of the parameter types. */ - REQUIRES_CONVERSION - } - - - /** - * An instance of {@code ArgumentsMatchInfo} describes what kind of match was achieved - * between two sets of arguments - the set that a method/constructor is expecting - * and the set that is being supplied at the point of invocation. - * - * @param kind the kind of match that was achieved - */ - record ArgumentsMatchInfo(ArgumentsMatchKind kind) { + /** + * A conversion match is where the type converter must be used to transform some of the parameter types. + */ + REQUIRES_CONVERSION; public boolean isExactMatch() { - return (this.kind == ArgumentsMatchKind.EXACT); + return (this == EXACT); } public boolean isCloseMatch() { - return (this.kind == ArgumentsMatchKind.CLOSE); + return (this == CLOSE); } public boolean isMatchRequiringConversion() { - return (this.kind == ArgumentsMatchKind.REQUIRES_CONVERSION); - } - - @Override - public String toString() { - return "ArgumentsMatchInfo: " + this.kind; + return (this == REQUIRES_CONVERSION); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorResolver.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorResolver.java index ba55a8ae83e..ea2c5b054ad 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorResolver.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveConstructorResolver.java @@ -30,6 +30,7 @@ import org.springframework.expression.ConstructorResolver; import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; import org.springframework.expression.TypeConverter; +import org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind; import org.springframework.lang.Nullable; /** @@ -75,7 +76,7 @@ public class ReflectiveConstructorResolver implements ConstructorResolver { for (int i = 0; i < paramCount; i++) { paramDescriptors.add(new TypeDescriptor(new MethodParameter(ctor, i))); } - ReflectionHelper.ArgumentsMatchInfo matchInfo = null; + ArgumentsMatchKind matchKind = null; if (ctor.isVarArgs() && argumentTypes.size() >= paramCount - 1) { // *sigh* complicated // Basically.. we have to have all parameters match up until the varargs one, then the rest of what is @@ -83,20 +84,20 @@ public class ReflectiveConstructorResolver implements ConstructorResolver { // the same type whilst the final argument to the method must be an array of that (oh, how easy...not) - // or the final parameter // we are supplied does match exactly (it is an array already). - matchInfo = ReflectionHelper.compareArgumentsVarargs(paramDescriptors, argumentTypes, typeConverter); + matchKind = ReflectionHelper.compareArgumentsVarargs(paramDescriptors, argumentTypes, typeConverter); } else if (paramCount == argumentTypes.size()) { // worth a closer look - matchInfo = ReflectionHelper.compareArguments(paramDescriptors, argumentTypes, typeConverter); + matchKind = ReflectionHelper.compareArguments(paramDescriptors, argumentTypes, typeConverter); } - if (matchInfo != null) { - if (matchInfo.isExactMatch()) { + if (matchKind != null) { + if (matchKind.isExactMatch()) { return new ReflectiveConstructorExecutor(ctor); } - else if (matchInfo.isCloseMatch()) { + else if (matchKind.isCloseMatch()) { closeMatch = ctor; } - else if (matchInfo.isMatchRequiringConversion()) { + else if (matchKind.isMatchRequiringConversion()) { matchRequiringConversion = ctor; } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java index dbf696c13a2..b643ef8f8e3 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java @@ -39,6 +39,7 @@ import org.springframework.expression.MethodResolver; import org.springframework.expression.TypeConverter; import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; +import org.springframework.expression.spel.support.ReflectionHelper.ArgumentsMatchKind; import org.springframework.lang.Nullable; /** @@ -169,20 +170,20 @@ public class ReflectiveMethodResolver implements MethodResolver { for (int i = 0; i < paramCount; i++) { paramDescriptors.add(new TypeDescriptor(new MethodParameter(method, i))); } - ReflectionHelper.ArgumentsMatchInfo matchInfo = null; + ArgumentsMatchKind matchKind = null; if (method.isVarArgs() && argumentTypes.size() >= (paramCount - 1)) { // *sigh* complicated - matchInfo = ReflectionHelper.compareArgumentsVarargs(paramDescriptors, argumentTypes, typeConverter); + matchKind = ReflectionHelper.compareArgumentsVarargs(paramDescriptors, argumentTypes, typeConverter); } else if (paramCount == argumentTypes.size()) { // Name and parameter number match, check the arguments - matchInfo = ReflectionHelper.compareArguments(paramDescriptors, argumentTypes, typeConverter); + matchKind = ReflectionHelper.compareArguments(paramDescriptors, argumentTypes, typeConverter); } - if (matchInfo != null) { - if (matchInfo.isExactMatch()) { + if (matchKind != null) { + if (matchKind.isExactMatch()) { return new ReflectiveMethodExecutor(method, type); } - else if (matchInfo.isCloseMatch()) { + else if (matchKind.isCloseMatch()) { if (this.useDistance) { int matchDistance = ReflectionHelper.getTypeDifferenceWeight(paramDescriptors, argumentTypes); if (closeMatch == null || matchDistance < closeMatchDistance) { @@ -198,7 +199,7 @@ public class ReflectiveMethodResolver implements MethodResolver { } } } - else if (matchInfo.isMatchRequiringConversion()) { + else if (matchKind.isMatchRequiringConversion()) { if (matchRequiringConversion != null) { multipleOptions = true; } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java index 8a2884e8f11..00b1ec3bc47 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectionHelperTests.java @@ -450,22 +450,22 @@ class ReflectionHelperTests extends AbstractExpressionTests { * Used to validate the match returned from a compareArguments call. */ private void checkMatch(Class[] inputTypes, Class[] expectedTypes, StandardTypeConverter typeConverter, ArgumentsMatchKind expectedMatchKind) { - ReflectionHelper.ArgumentsMatchInfo matchInfo = ReflectionHelper.compareArguments(typeDescriptors(expectedTypes), typeDescriptors(inputTypes), typeConverter); + ArgumentsMatchKind matchKind = ReflectionHelper.compareArguments(typeDescriptors(expectedTypes), typeDescriptors(inputTypes), typeConverter); if (expectedMatchKind == null) { - assertThat(matchInfo).as("Did not expect them to match in any way").isNull(); + assertThat(matchKind).as("Did not expect them to match in any way").isNull(); } else { - assertThat(matchInfo).as("Should not be a null match").isNotNull(); + assertThat(matchKind).as("Should not be a null match").isNotNull(); } if (expectedMatchKind == EXACT) { - assertThat(matchInfo.isExactMatch()).isTrue(); + assertThat(matchKind.isExactMatch()).isTrue(); } else if (expectedMatchKind == CLOSE) { - assertThat(matchInfo.isCloseMatch()).isTrue(); + assertThat(matchKind.isCloseMatch()).isTrue(); } else if (expectedMatchKind == REQUIRES_CONVERSION) { - assertThat(matchInfo.isMatchRequiringConversion()).as("expected to be a match requiring conversion, but was " + matchInfo).isTrue(); + assertThat(matchKind.isMatchRequiringConversion()).as("expected to be a match requiring conversion, but was " + matchKind).isTrue(); } } @@ -475,18 +475,18 @@ class ReflectionHelperTests extends AbstractExpressionTests { private static void checkMatchVarargs(Class[] inputTypes, Class[] expectedTypes, StandardTypeConverter typeConverter, ArgumentsMatchKind expectedMatchKind) { - ReflectionHelper.ArgumentsMatchInfo matchInfo = + ArgumentsMatchKind matchKind = ReflectionHelper.compareArgumentsVarargs(typeDescriptors(expectedTypes), typeDescriptors(inputTypes), typeConverter); if (expectedMatchKind == null) { - assertThat(matchInfo).as("Did not expect them to match in any way: " + matchInfo).isNull(); + assertThat(matchKind).as("Did not expect them to match in any way: " + matchKind).isNull(); } else { - assertThat(matchInfo).as("Should not be a null match").isNotNull(); + assertThat(matchKind).as("Should not be a null match").isNotNull(); switch (expectedMatchKind) { - case EXACT -> assertThat(matchInfo.isExactMatch()).isTrue(); - case CLOSE -> assertThat(matchInfo.isCloseMatch()).isTrue(); - case REQUIRES_CONVERSION -> assertThat(matchInfo.isMatchRequiringConversion()) - .as("expected to be a match requiring conversion, but was " + matchInfo).isTrue(); + case EXACT -> assertThat(matchKind.isExactMatch()).isTrue(); + case CLOSE -> assertThat(matchKind.isCloseMatch()).isTrue(); + case REQUIRES_CONVERSION -> assertThat(matchKind.isMatchRequiringConversion()) + .as("expected to be a match requiring conversion, but was " + matchKind).isTrue(); } } }