Fix array wrapping for varargs invocations in SpEL

[LeMikaelF]

There was a bug in SpEL where an array passed into a varargs method could be wrapped one too many times if the component type of the argument didn't match the component type of the parameter. For example:

@Test
void givenArrayFromVariable_whenInvokeVarargsMethod_thenCorrectArguments() {
	ExpressionParser parser = new SpelExpressionParser();
	StandardEvaluationContext context = new StandardEvaluationContext();
	Recorder recorder = new Recorder();
	context.setVariable("recorder", recorder);
	context.setVariable("myArray", new String[] { "a", "b" });

	parser.parseExpression("#recorder.record(#myArray)").getValue(context);

	assertThat(recorder.getArgs()).asInstanceOf(ARRAY).containsExactly("a", "b");
}

private static class Recorder {
	@Nullable private Object[] args;

        // test would pass with (String... args)
	public void record(Object... args) {
		this.args = args;
	}
	
	@Nullable
	public Object[] getArgs() {
		return args;
	}
}

This fails with the following error message:

Expecting actual:
  [["a", "b"]]
to contain exactly (and in same order):
  ["a", "b"]

I've added more exhaustive test cases for the method where the bug was, as well as a test in SpelCompilationCoverageTests. I've added this last test just next to another test that was commented out with a TODO, and had to comment out part of my test because of the same bug, but fixing this exposes another underlying bug that I've fixed in another branch, and I was able to uncomment the tests that were commented out. The fix is ready, I'm just waiting for this one to be merged.

I may also have accidentally uncovered another bug in SpEL. At line 4651 in SpelCompilationCoverageTests, the method seventeen() isn't actually invoked with (), yet it's still executed and its result value is available. I'm not sure this is desirable behaviour.

[sbrannen]

Hi @LeMikaelF,

This fails with the following error message:

I just copied and pasted the example from this issue's description into my IDE, and I cannot reproduce the error you've shown.

That test passes for me on main.

What versions of Spring Framework and Java are you using?

[LeMikaelF]

My bad, I had two similar tests and copy-pasted the wrong one. The tests above passes because the bug only affects arrays, not lists. This is the failing test:

@Test
void givenArrayFromVariable_whenInvokeVarargsMethod_thenCorrectArguments() {
	ExpressionParser parser = new SpelExpressionParser();
	StandardEvaluationContext context = new StandardEvaluationContext();
	Recorder recorder = new Recorder();
	context.setVariable("recorder", recorder);
	context.setVariable("myArray", new String[] { "a", "b" });

	parser.parseExpression("#recorder.record(#myArray)").getValue(context);

	assertThat(recorder.getArgs()).asInstanceOf(ARRAY).containsExactly("a", "b");
}

I will update the PR description.

[dcollins123]

Could you add * before the array var so the elements become individual args?

parser.parseExpression("#recorder.record(*#myArray)").getValue(context);

[sbrannen]

I may also have accidentally uncovered another bug in SpEL. At line 4651 in SpelCompilationCoverageTests, the method seventeen() isn't actually invoked with (), yet it's still executed and its result value is available. I'm not sure this is desirable behaviour.

That's to be expected: when seventeen is encountered in that expression without (), it is treated as a property reference that resolves to the seventeen() method in the root context object.

[sbrannen]

I've added this last test just next to another test that was commented out with a TODO, and had to comment out part of my test because of the same bug, but fixing this exposes another underlying bug that I've fixed in another branch, and I was able to uncomment the tests that were commented out. The fix is ready, I'm just waiting for this one to be merged.

Do you mean that you have a second fix for which you plan to submit a PR that will address the following TODO regarding compilation of varargs method invocations?

// TODO Determine why the String[] is passed as the first element of the Object... varargs array instead of the entire varargs array.

[sbrannen]

Could you add * before the array var so the elements become individual args?

@dcollins123, * is not proper syntax for a SpEL expression.

[sbrannen]

There was a bug in SpEL where an array passed into a varargs method could be wrapped one too many times if the component type of the argument didn't match the component type of the parameter.

SpEL has supported varargs invocations for methods and constructors for quite a while with the limitation that the supplied array must match the declared varargs type.

In light of that, we consider this an "enhancement" rather than a "bug", and we will include this in the upcoming 6.1.7. release.

[LeMikaelF]

Do you mean that you have a second fix for which you plan to submit a PR that will address the following TODO regarding compilation of varargs method invocations?

Yes, exactly.

[sbrannen]

Do you mean that you have a second fix for which you plan to submit a PR that will address the following TODO regarding compilation of varargs method invocations?

Yes, exactly.

@LeMikaelF, that would be great!