From e11a419cc32a5c680070d7e7199e5c2a833cb94c Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sun, 4 Aug 2024 16:42:38 +0300 Subject: [PATCH] Revert "Support single String argument for varargs invocations in SpEL" This reverts commit d33f66d9b5865d7eb76cfde70e93a25f23c5e4fb. See gh-33013 See gh-33189 --- .../spel/support/ReflectionHelper.java | 6 +-- .../spel/MethodInvocationTests.java | 44 ------------------- .../expression/spel/SpelReproTests.java | 19 +++----- .../spel/testresources/Inventor.java | 5 --- 4 files changed, 9 insertions(+), 65 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 d7013b57ef0e..e9b62ec2adbb 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -298,11 +298,11 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Exe conversionOccurred = true; } } - // If the argument type is assignable to the varargs component type, there is no need to + // If the argument type is equal to the varargs element type, there is no need to // convert it or wrap it in an array. For example, using StringToArrayConverter to // convert a String containing a comma would result in the String being split and // repackaged in an array when it should be used as-is. - else if (!sourceType.isAssignableTo(targetType.getElementTypeDescriptor())) { + else if (!sourceType.equals(targetType.getElementTypeDescriptor())) { arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); } // Possible outcomes of the above if-else block: diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index c1293625d93e..7b59007195e2 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -294,50 +294,6 @@ public void testVarargsInvocation03() { evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class); } - @Test // gh-33013 - void testVarargsWithObjectArrayType() { - // Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) - - // No var-args and no conversion necessary - evaluate("formatObjectVarargs('x')", "x", String.class); - - // No var-args but conversion necessary - evaluate("formatObjectVarargs(9)", "9", String.class); - - // No conversion necessary - evaluate("formatObjectVarargs('x -> %s', '')", "x -> ", String.class); - evaluate("formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class); - evaluate("formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class); - evaluate("formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class); - evaluate("formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class); - evaluate("formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class); - evaluate("formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class); - evaluate("formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class); - - // The following assertions were cherry-picked from 6.1.x; however, they are expected - // to fail on 6.0.x and 5.3.x, since gh-32704 (Support varargs invocations in SpEL for - // varargs array subtype) was not backported. - // evaluate("formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class); - // evaluate("formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class); - // evaluate("formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class); - // evaluate("formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class); - - // Conversion necessary - evaluate("formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class); - evaluate("formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class); - - // Individual string contains a comma with multiple varargs arguments - evaluate("formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class); - evaluate("formatObjectVarargs('foo -> %s %s', 'bar', ',baz')", "foo -> bar ,baz", String.class); - evaluate("formatObjectVarargs('foo -> %s %s', 'bar,', 'baz')", "foo -> bar, baz", String.class); - - // Individual string contains a comma with single varargs argument. - evaluate("formatObjectVarargs('foo -> %s', ',')", "foo -> ,", String.class); - evaluate("formatObjectVarargs('foo -> %s', ',bar')", "foo -> ,bar", String.class); - evaluate("formatObjectVarargs('foo -> %s', 'bar,')", "foo -> bar,", String.class); - evaluate("formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", String.class); - } - @Test public void testVarargsOptionalInvocation() { // Calling 'public String optionalVarargsMethod(Optional... values)' diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java index f7b9c382bb3e..00d9581f3818 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java @@ -66,7 +66,6 @@ import static org.assertj.core.api.Assertions.assertThatException; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; -import static org.assertj.core.api.InstanceOfAssertFactories.list; /** * Reproduction tests cornering various reported SpEL issues. @@ -1443,20 +1442,14 @@ void SPR12502() { assertThat(expression.getValue(new NamedUser())).isEqualTo(NamedUser.class.getName()); } - @Test // gh-17127, SPR-12522 - void arraysAsListWithNoArguments() { - SpelExpressionParser parser = new SpelExpressionParser(); - Expression expression = parser.parseExpression("T(java.util.Arrays).asList()"); - List value = expression.getValue(List.class); - assertThat(value).isEmpty(); - } - - @Test // gh-33013 - void arraysAsListWithSingleEmptyStringArgument() { + @Test + @SuppressWarnings("rawtypes") + void SPR12522() { SpelExpressionParser parser = new SpelExpressionParser(); Expression expression = parser.parseExpression("T(java.util.Arrays).asList('')"); - List value = expression.getValue(List.class); - assertThat(value).asInstanceOf(list(String.class)).containsExactly(""); + Object value = expression.getValue(); + assertThat(value).isInstanceOf(List.class); + assertThat(((List) value).isEmpty()).isTrue(); } @Test diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index 3979d919e096..282622cf7d2d 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java @@ -213,11 +213,6 @@ public String aVarargsMethod3(String str1, String... strings) { return str1 + "-" + String.join("-", strings); } - public String formatObjectVarargs(String format, Object... args) { - return String.format(format, args); - } - - public Inventor(String... strings) { }