Bug 350716

Comment 1 Olivier Thomann 2011-06-29 11:43:05 EDT

I would say that the type inferred in foo1 is a raw type which lead to unchecked warning. In foo2, the type inferred is ArrayList<Object> and this is not compatible with List<String>.
I will let Srikanth give a better explanation.

Note that javac b146 also reports an error for foo2.

Comment 2 Srikanth Sankaran 2011-06-30 02:19:56 EDT

(In reply to comment #0)

Don't see a bug here. Here is an explanation of what is going on:

>     private List<String> bar() {
>         return new ArrayList<>(); // no error
>     }

In this case the inferred type argument is String - The allocation expression
has an expected type of List<String> and that influences the inference per
15.12.2.8. 

Note that in this case, the wording in 15.12.2.8 

... "If the method result occurs in a context where it will be subject to
assignment conversion (§5.2) to a type S, then let R be the declared result
type of the method ..."

holds good while in the latter two methods it doesn't hold. As a result the
notion of expected type i.e List<String> applies to the *entire* conditional
construct in the case of foo1 and foo2 and the individual allocation expressions
don't have an (expected) declared result type.

>     private List<String> foo1(int a) {
>         return a > 0 ? new ArrayList<>() : new ArrayList(); // no error
>     }

As a result, the type argument if inferred to be Object here, so we have
ArrayList<Object> in conditional arm and RAW ArrayList in the other arm.
The type of the whole conditional expression is per 15.25 
lub(ArrayList<Object>, ArrayList#RAW) which comes to be
ArrayList#RAW which is acceptable in this context.

>     private List<String> foo2(int a) {
>         return a > 0 ? new ArrayList<>() : new ArrayList<>(); // error
>     }
> -----------------------------------------------------------------------------

In this case the type of the conditional expression is
lub(ArrayList<Object>, ArrayList<Object>) which is 
 ArrayList<Object> which is not convertible to List<String>.

I guess the key observation here is that the declared result type
doesn't constitute the expected type of the allocation expressions
when the allocation expressions are part of a conditional ? expression.

Does this make sense ?

Comment 3 Deepak Azad 2011-06-30 02:30:49 EDT

(In reply to comment #2)
> I guess the key observation here is that the declared result type
> doesn't constitute the expected type of the allocation expressions
> when the allocation expressions are part of a conditional ? expression.

I see. The compiler behavior indicated that this was the case but I was not sure if this was indeed the correct behavior.

Thanks for the detailed explanation!

Comment 4 Deepak Azad 2011-06-30 09:33:35 EDT

- In foo2 use the quick assist 'Replace if-else with conditional'
=> result is the code in foo1, which has a compile error. 
-------------------------------------------------------------------------------
	private List<String> foo1(int a) {
		return a > 0 ? new ArrayList<>() : new ArrayList<>(); //error
	}
	
	private List<String> foo2(int a) {
		if (a > 0)
			return new ArrayList<>();
		else
			return new ArrayList<>();
	}
------------------------------------------------------------------------------

There could be other cases of code transformations where the end result has a compile error or there is a change in semantics. (I do not know any other case yet)

Not sure what would be the right solution here...

Comment 5 Olivier Thomann 2011-06-30 09:37:25 EDT

I would prevent that quick assist from being proposed in the presence of diamond.

Comment 6 Deepak Azad 2011-06-30 10:38:20 EDT

More fun :)

1.
	List<String> l;
	l = a > 0 ? new ArrayList<>() : new ArrayList();
        	     // ArrayList<Object>

-> convert to if else

	if (a > 0)
		l = new ArrayList<>();  // ArrayList<String>
	else
		l = new ArrayList();

=> No error, but the result of type inference is changed

2. 
	List<String> l;
	ArrayList<Object> arrayList = new ArrayList<>();
	l = a > 0 ? arrayList : new ArrayList();

-> convert to if else

	if (a > 0)
		l = arrayList;  // error
	else
		l = new ArrayList();

=> Compile error, but this case does not involve diamond, just generics and conditional.

I think for cases involving diamond (comment 5 and example 1 above) we should just add explicit type arguments in the result to try ensure that semantics are not changed. If we do that then example 1 will be same as example 2 and I don't know if we can do anything there.

Comment 7 Markus Keller 2011-06-30 11:56:30 EDT

Regarding compile errors as in comment 0, you can make this arbitrarily complex, and I don't think we can do much about it. Eventually, programmers have to learn that the inference is not always perfect. Another similar example without conditionals:
	private String bingo() {
		return new ArrayList<>().iterator().next();
	}

I think the only concrete help we can provide for foo2() in comment 0, is to fix bug 112443 (allow converting to if-else even if there is an error).


For the problems with quick assists that create new compile errors, I think we need something similar to bug 174436, since the 15.12.2.8 rules are now also played for constructor invocations.

With a new API ClassInstanceCreation#isResolvedTypeInferredFromExpectedType(), I think we could do the same as we already do for invocations of parameterized methods.

Comment 8 Markus Keller 2011-06-30 12:15:41 EDT

In the pre-1.7 world, the analogon to foo2 from comment 4 is this:

    private List<String> foo4(int a) {
        if (a > 0)
            return Collections.emptyList();
        else
            return Collections.emptyList();
    }

Comment 9 Deepak Azad 2011-07-01 02:23:35 EDT

(In reply to comment #7)
> I think the only concrete help we can provide for foo2() in comment 0, is to
> fix bug 112443 (allow converting to if-else even if there is an error).
Agree.

> With a new API ClassInstanceCreation#isResolvedTypeInferredFromExpectedType(),
> I think we could do the same as we already do for invocations of parameterized
> methods.
Makes sense. I opened bug 350897.

Comment 10 Markus Keller 2011-07-12 14:01:16 EDT

Created attachment 199521 [details]
Fix with tests

Comment 11 Markus Keller 2011-07-12 14:06:42 EDT

This fixes the examples from comment 4, comment 8, and bug 350897 comment 3.

Released to BETA_JAVA7.

The cases from comment 6 could be handled in a new bug, but I think they're OK: Code that mixes raw and parameterized types that way deserves "garbage in - garbage out" treatment.

Comment 12 Satyam Kandula 2011-07-19 09:57:20 EDT

Verified that the if-else quick fix is getting proposed and it works. 
Verified using patch 1.0.0.v20110714-1400