Bug 343342 - [assist] Non constant variables, strings and methods are proposed inside case statements
Summary: [assist] Non constant variables, strings and methods are proposed inside case...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.7 M7   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-20 02:58 EDT by Ayushman Jain CLA
Modified: 2011-04-26 09:56 EDT (History)
3 users (show)

See Also:
Olivier_Thomann: review+

Attachments
proposed fix v1.0 + regression tests (12.74 KB, patch)
2011-04-20 10:23 EDT, Ayushman Jain CLA 		no flags Details | Diff
minor update (4.31 KB, patch)
2011-04-21 05:40 EDT, Ayushman Jain CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ayushman Jain CLA 2011-04-20 02:58:18 EDT 
HEAD

class Test {
    void f(int a) {
            String str = null;
	    final String ss = "";
	    switch (a){
		case s[CTRL-SPC]: break;
	   }
     }
}

In the above snippet str and ss are proposed. But theyre not allowed in java 1.6 and below
Comment 1 Ayushman Jain CLA 2011-04-20 08:38:43 EDT 
There are more cases that should be brought under the ambit of this bug. We should also not propose any non constant variable/field, or methods.
Comment 2 Ayushman Jain CLA 2011-04-20 10:23:28 EDT 
Created attachment 193702 [details]
proposed fix v1.0 + regression tests

The patch makes sure no non-constant variable, string, or method is proposed inside the case statement since JLS says only constant expressions can be used inside a case. It makes the use of CompletionEngine.isAssistNodeInsideCase field from bug 195346.
Comment 3 Ayushman Jain CLA 2011-04-20 10:25:17 EDT 
Olivier, can you please review? Its a small fix. Thanks!

The motivation behind this fix is to make sure strings are proposed only in 1.7 compliance. I'll open another bug for the BETA_JAVA7 branch for that once this is fixed.
Comment 4 Olivier Thomann CLA 2011-04-20 12:59:23 EDT 
Looks good. I guess the fix for compliance 1.7 will be different.
Comment 5 Ayushman Jain CLA 2011-04-21 01:48:47 EDT 
Released in HEAD for 3.7M7
Comment 6 Ayushman Jain CLA 2011-04-21 05:40:02 EDT 
Created attachment 193792 [details]
minor update

Just noticed that instead of calling the org.eclipse.jdt.internal.codeassist.CompletionEngine.assistNodeIsInsideCase(ASTNode, ASTNode) method in each of completionOn* methods, we can call it once at CompletionEngine#computeExpectedTypes(..). Also, the check for whether to find methods, instead of being done at the call site for findMethods(..) should be done inside findMethods(..) itself, so as to catch all cases where methods are being proposed inside case.

This patch consists of just these minor changes.
Comment 7 Ayushman Jain CLA 2011-04-21 05:40:50 EDT 
Released above patch also in HEAD.
Comment 8 Olivier Thomann CLA 2011-04-26 09:56:44 EDT 
Verified.