Bug 103667 - [quick fix] Should offer Quick Fix for problem 'Inexact type match for varargs argument'
Summary: [quick fix] Should offer Quick Fix for problem 'Inexact type match for vararg...
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: All All
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-07-13 11:36 EDT by Markus Keller CLA
Modified: 2020-08-27 16:32 EDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2005-07-13 11:36:42 EDT
3.1

We should offer 2 Quick Fixes for the problem 'Inexact type match for varargs
argument':

(1) Cast argument 'null' to 'Object[]'
(2) Wrap argument 'null' into 'new Object[]{...}'

public class VarargExample {
    static void addElements(Object... elements) {
        System.out.println(Arrays.deepToString(elements));
    }
    
    public static void main(String[] args) {
        addElements("A", 12);
        addElements(new Object[] {"Hello Object"});
            //no warning
        
        addElements(null);
            //Varargs argument null should be cast to Object[] etc.
        
        addElements(new String[] {"Hello String"});
            //Varargs argument String[] should be cast to Object[] etc.
    }
}

The quick fix (2) should produce:
        addElements(new Object[] {null});
        addElements(new Object[] {new String[] {"Hello String"}});
Comment 1 Markus Keller CLA 2005-07-13 12:04:46 EDT
Bug 103672 is a request for a better error message for this problem.
Comment 2 Dirk Baeumer CLA 2005-07-13 18:01:00 EDT
Makes sense to me.
Comment 3 Markus Keller CLA 2006-06-08 10:13:13 EDT
A third quick fix that would make sense:
(3) Cast argument 'null' to 'Object' for varargs invocation

See also the test case in bug 103672 comment 1 (that one is a bit more realistic than the test case here...).
Comment 4 Dani Megert CLA 2013-08-19 09:39:39 EDT
We should only offer to cast to the array and implement it as multi-fix.
Comment 5 Markus Keller CLA 2013-08-19 12:08:14 EDT
(In reply to comment #4)
> We should only offer to cast to the array and implement it as multi-fix.

Dani and I quickly discussed this before, but it looks like we were too quick.


Casting to the array type is the solution that just hides the warning but doesn't affect the semantics of the code in other ways. However, this is usually not the "right" solution and just bloats the code. A better solution is most of the time to turn the invocation into a real varargs invocation, and let the compiler produce the array instance.

(In reply to comment #0)
> (2) Wrap argument 'null' into 'new Object[]{...}'
> [..]
> The quick fix (2) should produce:
>         addElements(new Object[] {null});
>         addElements(new Object[] {new String[] {"Hello String"}});

This fix would remove the warning, but it's most certainly not the right solution. This should never be offered as a quick fix.

Let me re-label the available options to get rid of this warning:

a) Cast argument to 'varargType[]':

        // 1st solution proposed by the compiler warning
        addElements((Object[]) null);
        addElements((Object[]) new String[] {"Hello String"});

b) Cast argument to 'varargType' for varargs invocation:

        // 2nd proposal; not semantically preserving; incorrect in this case
        // incorrect for code written before the m(Object[]) became m(Object...)
        addElements((Object) null);
            // => caller gets an Object[1] with a null element
        addElements((Object) new String[] {"Hello String"});
            // => an Object[1] that contains a String[]? Hardly correct...

c) Change the method argument to be of the exact type 'varargType[]':

        addElements(new Object[] {"Hello String"});

d) Just pass the individual arguments and actually use a varargs invocation as intended by the varargs method declaration:

        addElements("Hello String");

    => Caveat: For this to be correct in the 1-arg case, the argument must not be of an array type that is assignable to 'varargsType[]'.
Comment 6 Markus Keller CLA 2013-08-19 12:48:33 EDT
I forgot to mention solution d) for addElements(null):

This is actually compiled into "invoke addElements(Object... elements) by passing 'null' for 'elements'". Real implementations of varags APIs like java.text.MessageFormat#format(Object[], ..) typically interpret a 'null' argument to a varargs parameter the same as an empty array. Therefore, the shortest solution in varargs-capable code is usually just this:

        addElements();


The diagnostic message is problematic, since it doesn't even tell what the actual problem is!

"The argument of type String[] should explicitly be cast to Object[] for the invocation of the varargs method addElements(Object...) from type VarargExample. It could alternatively be cast to Object for a varargs invocation"

The problem message should be changed to:

"Type String[] of last argument to method addElements(Object...) doesn't exactly match the vararg parameter type. Cast to Object[] to confirm the non-varargs call, or pass individual arguments of type Object for a varargs invocation."

I'll take this to bug 103672 unless someone has a better proposal.
Comment 7 Adtc Rulez CLA 2013-09-10 02:24:47 EDT
Surprised that several good suggestions have been suggested way back in 2006, but still haven't been implemented.

It's very obvious that you can offer quick-fixes here to change null to (varargType[])null or (varargType)null, since this is what the programmer would most often do to resolve the warning. And besides, if there is a @SuppressWarnings tag for this, it could also be offered as a quickfix.

Anyway the 'Wrap argument null into new Object[]{...} must NOT be offered as a quick-fix because this actually changes the semantics of the code, whereas casting null to one of the types doesn't - it still remains null, just that it is now casted safely for varargs invocation. Wrapping null to an empty varargType[] array should be left for the programmer to decide to do manually.

Here is what I would like to see:

========================================================
The argument of type null should explicitly be cast to 
Object[] for the invocation of the varargs method 
invoke(Object, Object...) from type Method. It could 
alternatively be cast to Object for a varargs invocation
--------------------------------------------------------
3 quick fixes available:
 - Cast null to Object[]
 - Cast null to Object
 @ Add @SuppressWarnings 'varagsinvocation' to invoke()
========================================================
Comment 8 Lukas Eder CLA 2020-08-27 03:51:08 EDT
I frequently run into this in test code where I want to test all the possible things that can be passed to my APIs.

A use-case that has not been discussed here yet is when an array parameter is refactored into a varargs parameter like this:

// Before
void m(String[] args);

// After
void m(String... args);

This is a binary compatible change and adds some convenience to some call sites, while not affecting other call sites. However, if the call site used to pass null:

m(null);

Then we get a warning now. Which makes sense, but it would be nice to not have to think about this too hard and just get offered the quick fix of casting the null to the array type, explicitly:

m((String[]) null);

In summary, I really only see two possible quick fixes here:

// 1. Cast the null to the inferred array type, 
// because that's what the code currently does: 
m((String[]) null);

// 2. Cast the null to the inferred array element type,
// because that's what the user might have wanted to do,
// and what's being done if there are more than one element:
m((String) null);
m(null, null); // No cast needed now.

I don't think that offering to wrap the argument in an explicit array instanciation is really what anyone wants to see as a result of a quick fix.