Bug 578198 - Better variable name for lambda completion
Summary: Better variable name for lambda completion
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.22   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Gayan Perera CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-13 06:28 EST by Lars Vogel CLA
Modified: 2024-04-06 12:41 EDT (History)
5 users (show)

See Also:


Attachments
Screenshot (16.44 KB, image/png)
2022-01-13 06:28 EST, Lars Vogel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2022-01-13 06:28:48 EST
Created attachment 287828 [details]
Screenshot

I think it would be nicer if the lambda variable name would use the regular naming convention, e.g. using the Type.

See screenshot.In this example invoice should be used instead of t.
Comment 1 Gayan Perera CLA 2022-01-16 16:04:42 EST
(In reply to Lars Vogel from comment #0)
> Created attachment 287828 [details]
> Screenshot
> 
> I think it would be nicer if the lambda variable name would use the regular
> naming convention, e.g. using the Type.
> 
> See screenshot.In this example invoice should be used instead of t.

Today we use variable name of the functional interface. But what about primitive and primitive wrappers ? I think naming based type might be useful for non-primitive and non-wrapper types. WDYT ?
Comment 2 Lars Vogel CLA 2022-01-16 16:54:44 EST
(In reply to Gayan Perera from comment #1)
> (In reply to Lars Vogel from comment #0)
> > Created attachment 287828 [details]
> > Screenshot
> > 
> > I think it would be nicer if the lambda variable name would use the regular
> > naming convention, e.g. using the Type.
> > 
> > See screenshot.In this example invoice should be used instead of t.
> 
> Today we use variable name of the functional interface. But what about
> primitive and primitive wrappers ? I think naming based type might be useful
> for non-primitive and non-wrapper types. WDYT ?

+1, sounds good to me
Comment 3 Eclipse Genie CLA 2022-03-22 15:05:06 EDT
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/192163
Comment 4 Jay Arthanareeswaran CLA 2022-03-24 03:47:09 EDT
The assumption that all parameter names will not be meaningful is not correct, IMO. If someone names the lambda method parameter as "subject" instead of "t", we will no longer be proposing the name intended by the API provider.

Consider these example:

interface MyInterface<T> {
    void accept(T subject);
}
private void forEach(MyInterface<Invoice> in) {}
 (or)
private void forEach(MyInterface<?> in) {}


With the patch, I get "Invoice invoice" or "Object object"
Before the patch, I get "Invoice subject" or "Object subject"

I am not sure if we really want to override a given name by something else.

Also consider the fact that we will be inconsistent with the code we generate when we convert an anonymous with lambda.
Comment 5 Gayan Perera CLA 2022-03-24 14:11:49 EDT
(In reply to Jay Arthanareeswaran from comment #4)
> The assumption that all parameter names will not be meaningful is not
> correct, IMO. If someone names the lambda method parameter as "subject"
> instead of "t", we will no longer be proposing the name intended by the API
> provider.
> 
> Consider these example:
> 
> interface MyInterface<T> {
>     void accept(T subject);
> }
> private void forEach(MyInterface<Invoice> in) {}
>  (or)
> private void forEach(MyInterface<?> in) {}
> 
> 
> With the patch, I get "Invoice invoice" or "Object object"
> Before the patch, I get "Invoice subject" or "Object subject"
> 
> I am not sure if we really want to override a given name by something else.
> 
> Also consider the fact that we will be inconsistent with the code we
> generate when we convert an anonymous with lambda.

Yes @Jay, i had the same concern after using this patch on some of the own code. For example when i was using this on a more fine grained functional interface like a interceptor where i have given a proper name into.

I'm open for suggestions if we can improve this further without making things more complex and ambiguous.
Comment 6 Gayan Perera CLA 2022-03-24 14:41:52 EDT
@Jay what if we do this only for java.util.function Functional interfaces which are known to be general purpose use.
Comment 7 Gayan Perera CLA 2022-03-24 15:07:05 EDT
@Jay i implemented my suggestion, please check how it feels now. I got some inspiration from IJ.
Comment 8 Stephan Herrmann CLA 2022-03-24 19:19:19 EDT
@Gayan, is your change *replacing* the existing naming strategy?

It seems that the lambda-proposal uses linked mode after insertion. Can you offer additional heuristics as alternatives in linked mode? Since there's no one strategy that fits all, presenting alternatives would feel natural to me.

Disclaimer: I failed to pull your change because EGit reported: SshException: No more authentication methods available
Comment 9 Jay Arthanareeswaran CLA 2022-03-25 00:28:30 EDT
(In reply to Gayan Perera from comment #7)
> @Jay i implemented my suggestion, please check how it feels now. I got some
> inspiration from IJ.

Looks much better to me now. I like how we even take care of the case like below:

void accept(T t, double value)

to which we propose "object, value".

However, for the ToDoubleBiFunction#applyAsDouble(T t, U u), we are proposing "object, u".

Looks like, within java.util.function.*, there are lot of cases to take care of.
Comment 10 Gayan Perera CLA 2022-03-25 16:49:11 EDT
I found the issue, its a code mistake in using wrong index in my new code. I will fix and will also try to capture the type variables so the parameters could be much meaningful
Comment 11 Gayan Perera CLA 2022-03-26 07:26:27 EDT
(In reply to Jay Arthanareeswaran from comment #9)
> (In reply to Gayan Perera from comment #7)
> > @Jay i implemented my suggestion, please check how it feels now. I got some
> > inspiration from IJ.
> 
> Looks much better to me now. I like how we even take care of the case like
> below:
> 
> void accept(T t, double value)
> 
> to which we propose "object, value".
> 
> However, for the ToDoubleBiFunction#applyAsDouble(T t, U u), we are
> proposing "object, u".
> 
> Looks like, within java.util.function.*, there are lot of cases to take care
> of.

The latest patch set fixes the issue you mentioned. But in cases where we need to resolve the type variable from the previouse argument such as in 
java.util.stream.Stream.reduce(Object, BiFunction<Object, ? super BigDecimal, Object>, BinaryOperator<Object>)

I couldn't find a way to resolve the type variables in BiFunction from the first argument of the reduce method.

Also let me know what you think about handling String and Object type parameters, May be we can exclude them as well.
Comment 12 Gayan Perera CLA 2022-03-26 07:31:10 EDT
(In reply to Stephan Herrmann from comment #8)
> @Gayan, is your change *replacing* the existing naming strategy?

Yes we change the orignal naming strategy if we can find a better variable name. And that is only done for java.util.function.* functional interfaces.

> 
> It seems that the lambda-proposal uses linked mode after insertion. Can you
> offer additional heuristics as alternatives in linked mode? Since there's no
> one strategy that fits all, presenting alternatives would feel natural to me.
> 

yes we could do that, but then the user needs to always select the parameter from the list where we suggest the original and computed isn't it ?
Comment 13 Stephan Herrmann CLA 2022-03-26 10:20:21 EDT
(In reply to Gayan Perera from comment #12)
> (In reply to Stephan Herrmann from comment #8)
> > @Gayan, is your change *replacing* the existing naming strategy?
> 
> Yes we change the orignal naming strategy if we can find a better variable
> name. And that is only done for java.util.function.* functional interfaces.

It's this hard coding of java.util.function.* that made we wonder: is JDT in a good position to decide which strategy is better in which case? There are other libraries providing general purpose functional interfaces. Should they be included? Will users ask for a preference to list additional functional interfaces to use the alternative strategy?

> > It seems that the lambda-proposal uses linked mode after insertion. Can you
> > offer additional heuristics as alternatives in linked mode? Since there's no
> > one strategy that fits all, presenting alternatives would feel natural to me.
> > 
> 
> yes we could do that, but then the user needs to always select the parameter
> from the list where we suggest the original and computed isn't it ?

The first proposal from the list can be accepted without further interaction. Alternatively, a user may find a better proposal down in the list. If proposals are sorted in a useful way (e.g., using the very rules you have implemented), than in most cases users wouldn't have to do more, but in the remaining cases they could select an alternative with just one click or few keystrokes.

IOW: if JDT has difficulties deciding which strategy is best, why not pass that decision down to users?

I'm sorry I joined this discussion late, so feel free to ignore, if everybody is comfortable with the design.
Comment 14 Gayan Perera CLA 2022-03-27 06:23:35 EDT
(In reply to Stephan Herrmann from comment #13)
> (In reply to Gayan Perera from comment #12)
> > (In reply to Stephan Herrmann from comment #8)
> > > @Gayan, is your change *replacing* the existing naming strategy?
> > 
> > Yes we change the orignal naming strategy if we can find a better variable
> > name. And that is only done for java.util.function.* functional interfaces.
> 
> It's this hard coding of java.util.function.* that made we wonder: is JDT in
> a good position to decide which strategy is better in which case? There are
> other libraries providing general purpose functional interfaces. Should they
> be included? Will users ask for a preference to list additional functional
> interfaces to use the alternative strategy?

Yes this is something problematic with the current approach. I agree. Because if JDT start deciding things for users, then JDT needs to be more opinionated, from my experience thats not the Eclipse way of working which i love about Eclipse. So may be we in JDT Core should only resolve conflicts like

		decimals.stream().filter(t -> {
			
			Stream.of(args).filter(t -> ).findFirst().isPresent();
		})

So the second predicate can suggest t2 instead of t from JDT. 


> 
> > > It seems that the lambda-proposal uses linked mode after insertion. Can you
> > > offer additional heuristics as alternatives in linked mode? Since there's no
> > > one strategy that fits all, presenting alternatives would feel natural to me.
> > > 
> > 
> > yes we could do that, but then the user needs to always select the parameter
> > from the list where we suggest the original and computed isn't it ?
> 
> The first proposal from the list can be accepted without further
> interaction. Alternatively, a user may find a better proposal down in the
> list. If proposals are sorted in a useful way (e.g., using the very rules
> you have implemented), than in most cases users wouldn't have to do more,
> but in the remaining cases they could select an alternative with just one
> click or few keystrokes.
> 
> IOW: if JDT has difficulties deciding which strategy is best, why not pass
> that decision down to users?

Yes i now also feel its better to suggest better names that *JDT Thinks* and let user decide. may be if the original name is a single letter, the JDT suggestions could be moved top.

> I'm sorry I joined this discussion late, so feel free to ignore, if
> everybody is comfortable with the design.

I think you bring good insight Stephan, i really value that. Specially in a project like this which we need to look at the wholistic picture. You always surprise me with things i missed and i always learn from them.
Comment 15 Gayan Perera CLA 2022-03-27 06:24:48 EDT
@Jay if you also think we should only resolve conflicts in JDT Core and then move the better variable suggestion as a choice that user needs to make, Let me know so i can start changing the patch accordingly
Comment 16 Jay Arthanareeswaran CLA 2022-04-06 01:48:25 EDT
(In reply to Gayan Perera from comment #15)
> @Jay if you also think we should only resolve conflicts in JDT Core and then
> move the better variable suggestion as a choice that user needs to make, Let
> me know so i can start changing the patch accordingly

Yes, that would be the safest option, IMO.
Comment 17 Eclipse Genie CLA 2024-04-06 12:41:51 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.