Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 322469 - [Code Assist] hinting suggests the second parameter only, when both are optional
Summary: [Code Assist] hinting suggests the second parameter only, when both are optional
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: PDT (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P1 normal (vote)
Target Milestone: ---   Edit
Assignee: Zhongwei Zhao CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-12 03:27 EDT by Roy Ganor CLA
Modified: 2020-05-14 11:09 EDT (History)
2 users (show)

See Also:
ganoro: review-
zhaozhongwei: review?
zhaozhongwei: review? (zhaozhongwei)


Attachments
patch (3.47 KB, patch)
2010-09-02 23:23 EDT, Zhongwei Zhao CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roy Ganor CLA 2010-08-12 03:27:42 EDT
Given:

<?php
class ZwasComponents_SessionClustering_Model {
    const SCD_STATUS_OFF	= 0;
    const SCD_STATUS_ON	 = 1;
}

class Node_Heartbeat {
    
    /**
     * @param integer $status
     * @param string $signature
     * @param integer $scStatus
     */
    public function __construct($signature = '', $scStatus = ZwasComponents_SessionClustering_Model::SCD_STATUS_OFF){
        
    }
}

Check the code in the additional info section.
At the end of it try writing

$hb = new Nodes_<CTRL+SPACE>

and you get 
$hb = new Node_Heartbeat($scStatus)

Even though $scStatus is the second parameter, and is optional

If you delete the end part and try
$hb = new Node_Heartbeat(<CTRL+SPACE>

Sometime it is completed without the variable, sometimes the cursor jumps between the 'N' and the 'o' characters
Comment 1 Zhongwei Zhao CLA 2010-08-19 21:59:10 EDT
fixed in head and branch
Comment 2 Petyo Tanchev CLA 2010-08-31 11:00:14 EDT
Tested on 2.2.1.v20100829

Works still the same way as described by Roy:

$hb = new Node_Heartbeat($scStatus)
Comment 3 Zhongwei Zhao CLA 2010-09-02 23:23:04 EDT
Created attachment 178117 [details]
patch
Comment 4 Roy Ganor CLA 2010-09-03 17:20:16 EDT
For this use-case:

function foo ($signature = '', 
$scStatus = ZwasComponents_SessionClustering_Model::SCD_STATUS_OFF)
{}
foo| 

the problem still exists...


Please fix.
Comment 5 Roy Ganor CLA 2010-09-03 17:23:47 EDT
one more comment:

in functions and methods if an argument is optional than all arguments after this argument MUST be optional.

My recommendation is to search for the first argument that is not optional and stop looking after it.

so the loop: 

+ for (int a = 0; a < args.size(); a++) {
+			Argument arg = (Argument) args.get(a);
+			parameter[a] = arg.getName();
+			if (arg.getInitialization() != null) {

should be updated to a while loop

thanks!
Comment 6 Zhongwei Zhao CLA 2010-09-03 20:03:18 EDT
(In reply to comment #5)
> one more comment:
> 
> in functions and methods if an argument is optional than all arguments after
> this argument MUST be optional.
> 
> My recommendation is to search for the first argument that is not optional and
> stop looking after it.
> 
> so the loop: 
> 
> + for (int a = 0; a < args.size(); a++) {
> +            Argument arg = (Argument) args.get(a);
> +            parameter[a] = arg.getName();
> +            if (arg.getInitialization() != null) {
> 
> should be updated to a while loop
> 
> thanks!

yes,for this bug we should update for loop to while loop,but the default values are used by other places that need know the value if it is a scalar,right?So we have to go over all the arguments.
Comment 7 Q.S. Wang CLA 2010-09-05 20:43:49 EDT
I'd suggest to change the method
public boolean visitMethodDeclaration

to private. 

Or any reason to have it as public method?

Another minor thing is that the name is a little confusing. 

It sounds like

vist(method) calls visitMethod() :)
Comment 8 Zhongwei Zhao CLA 2010-09-06 01:51:44 EDT
(In reply to comment #7)
> I'd suggest to change the method
> public boolean visitMethodDeclaration
> 
> to private. 
> 
> Or any reason to have it as public method?
> 
> Another minor thing is that the name is a little confusing. 
> 
> It sounds like
> 
> vist(method) calls visitMethod() :)

I do not know how to name it,the code is copied from super.visit,and I will change public to private.
Comment 9 Zhongwei Zhao CLA 2010-09-07 03:06:47 EDT
fixed
Comment 10 Petyo Tanchev CLA 2010-10-06 09:02:06 EDT
Tested on 2.2.1.v20101001
Fixed