Bug 310747

Comment 1 Srikanth Sankaran 2010-04-28 01:02:41 EDT

Ayush please investigate - What makes sense here ?
See also bug# 292087 comment# 9 -- Thanks.

Comment 2 Ayushman Jain 2010-05-13 07:19:13 EDT

Created attachment 168368 [details]
proposed fix v1.0 + regression tests

There are precisely 2 problems in this case:
1) Random strings from the comment line that follows the position where we're asking for completion are being proposed. This happens due to the incorrect modification of CompletionScanner$currentPosition and CompletionScanner$startPosition in AssistParser.moveRecoveryCheckPoint(). The root cause of this lies in the incorrect setting of CompletionParser$lastCheckPoint at CompletionParser.buildMoreCompletionContext : 878. 
What happens is - currently on encountering the completion token inside array initializer, the currentPosition points to the position of the token after the completion token ie. points to '/'. When the above mentioned line in buildMore.. sets the lastCheckPoint to this position and recovery restarts, AssistParser.moveRecoveryCheckPoint() advances the scanner by one token, and the old position is taken using lastCheckPoint. Due to this the scanner now points ahead of the first '/' of the line comment(It should actually point AT the '/'), in effect skipping the first '/' entirely. Because of this missed token the parser is not able to determine that the second '/' marks the starting of the line comment and goes ahead to ACTUALLY PARSE THE COMMENT ITSELF! 
So the fix for this is to just get rid of the offending line in buildMore...()

2) The proposals are not sorted according to the expected type, leading to useless proposals showing up first and useful ones down in the list. The fix for this is in CompletionEngine.computeExpectedTypes(). It is clear from line 3388 that we are ignoring adding expected types if the variable initialization is an array initializer. The fix takes care to add the relevant expected type in this case also.

Comment 3 Srikanth Sankaran 2010-05-19 05:23:19 EDT

Patch looks good. Targetting 3.7 M1.

Comment 4 Ayushman Jain 2011-02-03 11:41:11 EST

Created attachment 188253 [details]
patch updated for HEAD

Comment 5 Ayushman Jain 2011-02-03 11:43:51 EST

Released in HEAD for 3.7M6

Comment 6 Olivier Thomann 2011-03-07 11:19:40 EST

I still get some weird proposals. Reopening to get clarifications.

Comment 7 Ayushman Jain 2011-03-08 00:52:01 EST

(In reply to comment #6)
> I still get some weird proposals. Reopening to get clarifications.

The problem, when reported, was that in the case given in comment 0, we used to get bogus proposals, which included words from within the comment " // completion here brings up useless proposals". So one would see proposals like "completion", "here", "brings", etc.  With the fix, we don't get these anymore. There are some extra proposals here because there's nothing precise to propose in this case.
OTOH, if you try

public class Test {
	public Test memberGet() {
		return new Test();
	}
	public Test memberField;
    private Test member[] = {
    		
        // complete here
    };    
}

you would now get memberField, and memberGet as the topmost proposals, which is correct. Again, there are some other proposals of the type Object, but thats ok.

Let me know if you observed some other weird behaviour

Comment 8 Olivier Thomann 2011-03-08 08:49:53 EST

Then closing as fixed.

Comment 9 Olivier Thomann 2011-03-08 08:50:05 EST

Verified for 3.7M6.