Bugzilla – Full Text Bug Listing
| Summary: | [quick fix] for qualified enum constants in switch-case labels | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Markus Keller <markus.kell.r> | ||||||||
| Component: | UI | Assignee: | sandra lions <sandra.lions-piron> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | daniel_megert, hauser, jerome.cambon, manju656, markus.kell.r, noopur_gupta, sandra.lions-piron | ||||||||
| Version: | 3.1 | Flags: | noopur_gupta:
review+
|
||||||||
| Target Milestone: | 4.5 M1 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Markus Keller
Example from the news.
The code below does not compile: "The enum constant EnumTester.color.black
reference cannot be qualified in a case label".
We could offer a quick fix which just removes the qualifier.
class EnumTester {
public enum color {black, white}
public void test(color c) {
switch (c) {
case color.black:
System.out.println("Black");
break;
case color.white:
System.out.println("White");
break;
}
}
}
Martin, please resolve as later if out of scope for 3.1 *** Bug 296413 has been marked as a duplicate of this bug. *** (In reply to bug 296413 comment #0) > dropping "MyEnumClass." would be so easy Implementation would be similar to LocalCorrectionsSubProcessor#addUnnecessaryInstanceofProposal(..) Tests for this are in org.eclipse.jdt.ui.tests, in LocalCorrectionsQuickFixTest. You can assign this one to me. Not sure to be clear about which part of this code should be removed ? Actually I already started to work on this one. I updated our internal wiki but forget to add a comment in bugzilla. Sorry for that :-( You can assign the issue to me. Created attachment 243292 [details] Unit Test + Patch This contribution complies with http://www.eclipse.org/legal/CoO.php (In reply to sandra lions from comment #7) > Created attachment 243292 [details] [diff] > Unit Test + Patch > > This contribution complies with http://www.eclipse.org/legal/CoO.php Patch looks good. Some review comments: - Use the given template for updating the contributors list in the file header (https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions): Your Name <email@example.com> - Bug Title - https://bugs.eclipse.org/BUG_NUMBER - Local variable 'ast' can be inlined as it is used only once. - Since enum constants cannot be surrounded by parenthesis, a case like "case ((color.white)): ..." would not be legal. Still, the quick fix can handle the parenthesis removal also in such a case. => Extract the qualifiedName from ParenthesizedExpression (see LocalCorrectionsSubProcessor#addUnnecessaryInstanceofProposal(..)) and replace the original coveringNode with simpleName. Created attachment 243645 [details] Unit Test + Patch version 2 Take into account Noopur Gupta comments (comment 8): - fixed contributors list - handle quick fix for enum surrounded with parenthesis. Note that I have a question in the java code (see LocalCorrectionsSubProcessor.addEnumConstantsCannotBeSurroundedByParenthesisProposal) - not sure to understand what you mean by : Local variable 'ast' can be inlined as it is used only once. Do you mean add some linked proposals in order to rename the enum ? Just like the Assign parameter to existing field quick fix ? Created attachment 243673 [details] Updated Patch (In reply to sandra lions from comment #9) I have updated your fix and tests with changes required in comment #8. Please have a look. - Contributors list was still not updated in all the changed files. Also, notice the exact template format. - "Local variable 'ast' can be inlined as it is used only once." => Referred to the local variable 'ast' that was present in LocalCorrectionsSubProcessor#addIllegalQualifiedEnumConstantLabelProposal. - "the quick fix can handle the parenthesis case" => Referred to updating the same quick fix so that it is also available in examples like: "case (color.white): ..." Hi Noopur, I tried your updated patch and have 2 questions : - when we get a qualified enum surrounded with parenthesis: case ((color.black)): we do not get the light icon in the left margin. I thought this icon was needed to indicate a quick fix was available, is it ? That’s the reason I added the case IProblem.EnumConstantsCannotBeSurroundedByParenthesis in the QuickFixPorcessor.java. - when we get an unqualified enum surrounded with parenthesis: case ((black)): we do not get the quick fix anymore (in order to simply remove the parenthesis). I first wanted to propose a quick fix for both cases (qualified enum AND/OR enum surrounded with parenthesis), but I agree, the parenthesis quick fix is larger than the scope of the current issue. Do you prefer I log another issue to track the enum parenthesis quick fix ? Thanks for your feedback, sandra (In reply to sandra lions from comment #11) > - when we get a qualified enum surrounded with parenthesis: > case ((color.black)): > we do not get the light icon in the left margin. In such a case, we have multiple markers (errors) at that line - You can see that when you hover on the error marker in the vertical ruler. Hence, though the light bulb exists, it is hidden behind the other error marker, and is not directly visible. You can view the light bulb if you check the preference option: Java > Editor > Hovers > Expand vertical ruler icons upon hovering. Then, re-open the editor and hover on the vertical ruler. > - when we get an unqualified enum surrounded with parenthesis: > case ((black)): > we do not get the quick fix anymore (in order to simply remove the > parenthesis). > but I agree, the parenthesis quick fix is larger than the scope of the > current issue. > Do you prefer I log another issue to track the enum parenthesis quick fix ? The other quick fix (that only removes the parenthesis) is not required as part of this bug. Also, it is a rare scenario and not of much relevance. If required, we can log another bug for that later. |