`RemoveToStringCallsFromArrayInstances` should not use `Arrays.toString` on String/PrintStream/PrintWriter.format methods

[philippe-granet]

The following code:

    Object[] strings = new Object[]{"foo", "bar"};
    System.out.println(String.format("%s %s", strings));

display foo bar.
But RemoveToStringCallsFromArrayInstances recipe generate this code::

    Object[] strings = new Object[]{"foo", "bar"};
    System.out.println(String.format("%s %s", Arrays.toString(strings)));

that fail with this error:

Exception in thread "main" java.util.MissingFormatArgumentException: Format specifier '%s'
at java.base/java.util.Formatter.format(Formatter.java:2780)
at java.base/java.util.Formatter.format(Formatter.java:2717)
	at java.base/java.lang.String.format(String.java:4150)
	at HelloWorld.main(HelloWorld.java:8)

I think RemoveToStringCallsFromArrayInstances recipe should not use Arrays.toString on String/PrintStream/PrintWriter format methods.

[timtebeek]

Thanks for the clear and detailed report! That's indeed a bug that could be fixed with checking the surrounding method invocation. Is that also something you'd like to work on?

To be sure: we appreciate every report, but it can be hard to get to everything. Don't be discouraged from reporting more issues even if you don't have time to contribute! :)

[knutwannheden]

Good catch! I think the issue here is that the method being called is a varargs method (e.g. String#format(String, Object...)) and that the recipe probably doesn't correctly handle that case.

[philippe-granet]

Thanks for the clear and detailed report! That's indeed a bug that could be fixed with checking the surrounding method invocation. Is that also something you'd like to work on?

You're welcome 😀
Sorry, I'd would love to help but I don't have time available at the moment to work on this problem

[timtebeek]

The recipe has some handling to explicitly include arrays passed into one of various print/format methods:

rewrite-static-analysis/src/main/java/org/openrewrite/staticanalysis/RemoveToStringCallsFromArrayInstances.java

Lines 76 to 84 in 30b963b

	} else if (METHOD_MATCHERS.stream().anyMatch(matcher -> matcher.matches(mi))) {
	// deals with edge cases where .toString() is called implicitly
	List<Expression> arguments = mi.getArguments();
	for (Expression arg : arguments) {
	if (arg.getType() instanceof JavaType.Array) {
	getCursor().putMessage("METHOD_KEY", mi);
	break;
	}
	}

It seems as though that wasn't optimal though, as it produced additional perhaps unwanted changes. I've opened a PR with an alternative but would like your input on potential additional cases to cover:

• Do not adopt Arrays.toString for varargs arguments #246