Bug 70631 - [content assist] Parameter Hints should infer the overloaded method version
Summary: [content assist] Parameter Hints should infer the overloaded method version
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-Text-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-07-22 11:41 EDT by Markus Keller CLA
Modified: 2021-08-03 11:17 EDT (History)
4 users (show)

See Also:
daniel_megert: review-


Attachments
An untested fix. (16.07 KB, patch)
2012-10-11 14:28 EDT, Tristan Hume CLA
no flags Details | Diff
mylyn/context/zip (8.25 KB, application/octet-stream)
2012-10-11 14:28 EDT, Tristan Hume CLA
no flags Details
Fixed version of previous patch. Counts parameters in call to sort method list. (17.63 KB, patch)
2012-10-12 14:03 EDT, Tristan Hume CLA
daniel_megert: review-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2004-07-22 11:41:08 EDT
Eclipse 3.0

Parameter Hints should infer the overloaded method version from the invocation
around the caret location.

Example:

void method(String name) { }
void method(String name, String displayName) { }
void call() {
	method("Hello", "World");
}

- set caret into argument list of method(..) call
- press Ctrl+Shift+Space
-> expected: popup with both methods, the version with *2* parameters selected
-> was: popup, 1st line selected
Comment 1 Dani Megert CLA 2004-07-23 05:54:41 EDT
Important step to reproduce was missing: the file needed to be closed and
reopened because content assist remembers which version is used and from then on
directly displays the param hint for the last used variant. e.g. if you use
content assist to insert 1-param variant it will show the param hint with one
argument.

*** This bug has been marked as a duplicate of 58719 ***
Comment 2 Markus Keller CLA 2010-01-26 06:34:47 EST
(In reply to bug 58719 comment #7)
> >- pre-select the best-matching version in the proposal list
> This is currently not possible since we don't know which method it is [..]

This actually works fine for the scenario from comment 0 because on Ctrl+Shift+Space, the proposals list is now ordered with decreasing parameter count (I don't really know why -- content assist is still ordered with less parameters first).

But even without resolving, this could be improved by pre-selecting a method with matching parameter count, e.g. select the middle method here:

	void method(String name) { }
	void method(String name, String displayName) { }
	void method(String name, String displayName, boolean toggle) { }
	void call() {
		method("Hello", "World");
	}

Finding the parameter count should now be too expensive, since we already do similar syntactical parsing to embolden the current parameter in the context information popup.
Comment 3 Deepak Azad CLA 2010-01-26 08:52:28 EST
I was trying 'String.valueOf(10)', there are a number of overloaded one
parameter methods. I would like to see the correct match based on the exact
parameter type.
Comment 4 Tristan Hume CLA 2012-10-02 15:27:59 EDT
There seems to already be code that can do this somewhere. The Javadoc popup correctly identifies the correct overloaded method.

Example:
Hovering over String.valueOf(10) will show the Javadoc for String.valueOf(int)

The Javadoc popup uses org.eclipse.jdt.core.ICodeAssist.codeSelect(int, int). Using this in combination with something like org.eclipse.jdt.internal.ui.text.java.JavaCompletionProposalComputer.guessMethodContextInformationPosition(ContentAssistInvocationContext) it should be possible to extract the correct overloaded method.
Comment 5 Tristan Hume CLA 2012-10-03 13:36:45 EDT
I just read the comments on bug 58719 and saw the comment stating why this feature was not originally implemented.

Why can't codeSelect be used? Is there some bad side effect of doing a reconcile that I am unaware of? I am a fairly new contributor so I do not have that good of an understanding of the underlying architecture.
Comment 6 Markus Keller CLA 2012-10-04 06:41:24 EDT
codeSelect can be used, but not in the UI thread, since it can take too long to complete the necessary reconcile for huge classes. The Javadoc view and the Javadoc hover both compute their content in a background thread. Even the Open Declaration action uses SelectionConverter#codeResolveForked(..) to ensure the UI doesn't freeze.
Comment 7 Tristan Hume CLA 2012-10-04 13:49:14 EDT
Ok I have found the parsing code that detects what parameter number is selected. It is huge and complex due to having to handle nesting like func1(func2(7,9),8).

It is currently a private method in JavaParameterListValidator called getCharCount. It does not use any state and is completely generalized. It really belongs in some common location for parsing code as a static method.

Is there some common class for parsing helpers that I can move it to?
Comment 8 Tristan Hume CLA 2012-10-09 15:44:12 EDT
So far I'm thinking of moving the getCharCount method to the JavaHeuristicScanner class. Then I will use it to put methods with the right number of parameters on top in filterAndSortContextInformation.
Comment 9 Tristan Hume CLA 2012-10-11 14:28:26 EDT
Created attachment 222193 [details]
An untested fix.

I have written a fix that should work but when I went to test it, I discovered that I could not get any parameter hints. I thought it was a bug in my fix but then I discovered that they did not work in my main version of Eclipse either.

I suspect the culprit is the Code Recommenders plugin I installed since parameter hints worked before I installed it. I will work on fixing this.

Meanwhile I would really appreciate it if someone could test this patch.
Comment 10 Tristan Hume CLA 2012-10-11 14:28:30 EDT
Created attachment 222194 [details]
mylyn/context/zip
Comment 11 Tristan Hume CLA 2012-10-12 14:03:51 EDT
Created attachment 222245 [details]
Fixed version of previous patch. Counts parameters in call to sort method list.

I managed to get my parameter hints working again and test my fix.
I fixed one bug and now it works well, including in the case of nested calls.

I also added my info to the contributor headers. If a commiter could review this patch it should be ready for contribution.
Comment 12 Dani Megert CLA 2013-04-18 06:21:56 EDT
I'll try to look at the patch during M7. Otherwise it has to move to 4.4.
Comment 13 Dani Megert CLA 2014-04-23 06:40:23 EDT
Comment on attachment 222245 [details]
Fixed version of previous patch. Counts parameters in call to sort method list.

I tried the patch and it produces an IEA on the example from comment 0 [1]. I addition, the patch looks huge to me for solving such a small problem.


[1]
Caused by: java.lang.IllegalArgumentException: 
	at org.eclipse.core.runtime.Assert.isLegal(Assert.java:63)
	at org.eclipse.core.runtime.Assert.isLegal(Assert.java:47)
	at org.eclipse.jdt.internal.ui.text.JavaHeuristicScanner.scanForward(JavaHeuristicScanner.java:956)
	at org.eclipse.jdt.internal.ui.text.JavaHeuristicScanner.findClosingPeer(JavaHeuristicScanner.java:567)
	at org.eclipse.jdt.internal.ui.text.JavaHeuristicScanner.countParameters(JavaHeuristicScanner.java:647)
	at org.eclipse.jdt.internal.ui.text.java.ContentAssistProcessor.computeContextInformation(ContentAssistProcessor.java:366)
	at org.eclipse.jface.text.contentassist.ContentAssistant.computeContextInformation(ContentAssistant.java:1885)
	at org.eclipse.jface.text.contentassist.ContentAssistSubjectControlAdapter.computeContextInformation(ContentAssistSubjectControlAdapter.java:390)
	at org.eclipse.jface.text.contentassist.ContextInformationPopup.computeContextInformation(ContextInformationPopup.java:397)
	at org.eclipse.jface.text.contentassist.ContextInformationPopup.access$1(ContextInformationPopup.java:396)
	at org.eclipse.jface.text.contentassist.ContextInformationPopup$1.run(ContextInformationPopup.java:194)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.jface.text.contentassist.ContextInformationPopup.showContextProposals(ContextInformationPopup.java:189)
	at org.eclipse.jface.text.contentassist.ContentAssistant.showContextInformation(ContentAssistant.java:1739)
	at org.eclipse.jface.text.source.SourceViewer.doOperation(SourceViewer.java:935)
	at org.eclipse.jface.text.source.projection.ProjectionViewer.doOperation(ProjectionViewer.java:1507)
	at org.eclipse.jdt.internal.ui.javaeditor.JavaSourceViewer.doOperation(JavaSourceViewer.java:191)
	at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor$AdaptedSourceViewer.doOperation(CompilationUnitEditor.java:200)
	at org.eclipse.ui.texteditor.TextOperationAction$1.run(TextOperationAction.java:128)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.ui.texteditor.TextOperationAction.run(TextOperationAction.java:126)
	at org.eclipse.jface.action.Action.runWithEvent(Action.java:519)
	at org.eclipse.jface.commands.ActionHandler.execute(ActionHandler.java:122)
	... 60 more
Comment 14 Markus Keller CLA 2016-03-17 11:36:55 EDT
*** Bug 485542 has been marked as a duplicate of this bug. ***