eclipse-jdt / eclipse.jdt.ui

Eclipse Public License 2.0
36 stars 86 forks source link

Optional requiring a cast yields strange quickfixes #759

Open laeubi opened 1 year ago

laeubi commented 1 year ago

Assume the following code

public <T> T hmmmm(Callable<?> callme) throws Exception {
     return callme.call();
}

JDT offers me to cast the type to T:

grafik

but as soon as this is wrapped in an Optional call this does not work anymore:

grafik

Choosing then first or second quickfix simply wraps the optional but does not resolve the compile error

public <T> Optional<T> hmmmm(Callable<?> callme) {
    try {
        return Optional.ofNullable(Optional.of(callme.call()));
    } catch(Exception e) {
        return Optional.empty();
    }
}

Choosing the third option simply deletes all code an replace it with empty optional:

public <T> Optional<T> hmmmm(Callable<?> callme) {
    try {
        return Optional.empty();
    } catch(Exception e) {
        return Optional.empty();
    }
}

Choosing the last option cast the optional instead of the argument:

public <T> Optional<T> hmmmm(Callable<?> callme) {
    try {
        return (Optional<T>)Optional.of(callme.call());
    } catch(Exception e) {
        return Optional.empty();
    }
}

Instead something like this is desired:

public <T> Optional<T> hmmmm(Callable<?> callme) {

    try {
        return Optional.of((T)callme.call());
    } catch(Exception e) {
        return Optional.empty();
    }
}
stephan-herrmann commented 1 year ago

Choosing the last option cast the optional instead of the argument:

Technically, the compile error affects the entire optional expression, which resolves to Optional<captureof ?>. Since this is not compatible to the return type Optional<T> it is the canonical solution to add the cast to the outer expression.

It requires extra knowledge to see that an inner cast would change the inferred type of that expression in the desired way. The algorithm to employ would be akin to reverse type inference ("assuming type inference would produce type X, what input would that algorithm need to come to that conclusion?" - if inference is a hard nut, that's one magnitude harder, even).

Should JDT suppress the cast proposal, if it is not what the user needs? How would we recognize this situation where it's not helpful?

Or are you asking for special case proposals, that are geared only at improving the workflow wrt Optional?

laeubi commented 1 year ago

For me it seems there are already special handling for optionals (even though I can't imagine when it is useful to delete the whole code and replace it with Optional.empty()), but the problem can be reproduced with other similar cases as well I think in general it is the following pattern:

  1. I have a "target type" that could either be explicit (eg String x = ...) or implicit (eg like here return ... ) of course could also be generic.
  2. I have some "dynamic" return type that is generic and this return type is deduced by one or more of the parameters of the function, e.g. List.of(...), Optional.of(...), public <T> T whatever(Function<String, T> converter, String paramX), ... where the type is automatically inferred
  3. Now the item that influences the value is wrong, needs a cast or itself depends on another dynamic type inference and everything goes bad

Of course my example here are simplified to the absolute minimum I can think of, but it can really drive you crazy in a larger expression as it often falls into two categories:

in both cases this usually marks the whole expression, that could span multiple lines and many function calls as errornours, autocompletion is completely disabled and most of the time quickfixes do either not work or do strange stuff.

In the end the user either happily "see" the missing pice or has to extract everything into local variables, convert lamdas to anonymous classes and so on to see what is really going on.

laeubi commented 1 year ago

@stephan-herrmann I described another maybe related problem here:

even though JDT seem to know that a Supplier< ..> is required and its lamda variant has to return a value (even though it thinks its of type U) I get some kind of not helpful quickfixes that can even produce more errors.