Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 265744

Summary: Enum switch should warn about missing default
Product: [Eclipse Project] JDT Reporter: Stephan Herrmann <stephan.herrmann>
Component: CoreAssignee: Stephan Herrmann <stephan.herrmann>
Status: VERIFIED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: amj87.iitr, deepakazad, markus.kell.r, Olivier_Thomann, srikanth_sankaran
Version: 3.4.1   
Target Milestone: 3.8 M6   
Hardware: Other   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 372818, 372840    

Description Stephan Herrmann CLA 2009-02-21 16:02:06 EST 
When reading bug 149562 and bug 207915 and bug 97790 
it seems that analysis of enum switch is not 100% what
people (including me) naivly expect.

Indeed JLS 16.2.9 requires a default label no matter what is 
the switch type (int or enum).
However, JLS 14.11 additionally has this:

 "Compilers are encouraged (but not required) to provide a
  warning if a switch on an enum-valued expression lacks a
  default case and lacks cases for one or more of the enum
  type's constants..."

Of this only the second part is implemented, right?
I think implementing the first warning would actually help 
in the above situations, because than we have a place where
we can give a hint on the motivation. With no diagnostic
on the switch statement, the compiler can't tell the user
why (s)he should add a default branch.

s.t. along these lines:
"Switch statement should contain a default case to improve
binary compatibility."
Comment 1 Stephan Herrmann CLA 2011-04-19 12:17:36 EDT 
Olivier, this should be a trivial fix. Do you think it makes sense for 3.7?

Questions are only regarding configurability & suppress warning token.
Comment 2 Olivier Thomann CLA 2011-04-19 12:25:58 EDT 
Unfortunately this will require new options and this would add new API field on IProblem.
So I would defer to 3.8.
Comment 3 Stephan Herrmann CLA 2012-01-29 07:40:21 EST 
Let's no miss API freeze this time. As mentioned, fix should be trivial.

We do need a new IProblem but this would fit into existing options:
- Irritant: IncompleteEnumSwitch
- token: "incomplete-switch"
- option: "org.eclipse.jdt.core.compiler.problem.incompleteEnumSwitch"

Question: is the default "ignore" for IncompleteEnumSwitch really justified? E.g., in bug 207915 the new warning would help explaining another not-configurable error. If the warning is ignored the error (re blank final field) remains unexplained, hm?
Comment 4 Stephan Herrmann CLA 2012-02-23 07:08:39 EST 
Srikanth, the fix would be easy, but I don't think another off-by-default warning would help a lot, as I argued in comment 3.

What do you think of changing the default for IncompleteEnumSwitch to warning? After all this warning *is* encouraged by the JLS.
Comment 5 Srikanth Sankaran CLA 2012-02-24 00:01:35 EST 
(In reply to comment #4)
> Srikanth, the fix would be easy, but I don't think another off-by-default
> warning would help a lot, as I argued in comment 3.
> 
> What do you think of changing the default for IncompleteEnumSwitch to warning?
> After all this warning *is* encouraged by the JLS.

Sounds like a good idea to me.
Comment 6 Stephan Herrmann CLA 2012-02-28 12:32:04 EST 
Resolved for 3.8 M6 via commit d5a32e245b27c645dcf70347396d7253765682d3

I'm leaving the bug open for the corresponding doc-update.
Comment 7 Stephan Herrmann CLA 2012-02-28 12:50:54 EST 
(In reply to comment #6)
> I'm leaving the bug open for the corresponding doc-update.

Looking at the doc I find that CompilerOptions.IncompleteEnumSwitch is rendered as "Enum type constant not covered on 'switch'"

Therefore I propose to rephrase the UI label to s.t. like
   "Incomplete enum switch"
:)

Alternatively, we could of course create yet another irritant/option, but that sounds like overkill to me.

For easy reference: I made the new warning say: 
    The switch on the enum type {0} should have a default case
Comment 8 Markus Keller CLA 2012-02-29 07:19:40 EST 
> I'm leaving the bug open for the corresponding doc-update.

That includes Javadoc of JavaCore.COMPILER_PB_INCOMPLETE_ENUM_SWITCH, right?
It needs updates in the description and for the default value.
Comment 9 Deepak Azad CLA 2012-02-29 07:22:19 EST 
FYI: I also opened bug 372840 to provide a quick fix for the new problem.
Comment 10 Stephan Herrmann CLA 2012-03-01 17:43:54 EST 
(In reply to comment #8)
> > I'm leaving the bug open for the corresponding doc-update.
> 
> That includes Javadoc of JavaCore.COMPILER_PB_INCOMPLETE_ENUM_SWITCH, right?
> It needs updates in the description and for the default value.

Sure. 
To make the dependency between both situations clear I'd make it:

 When enabled, the compiler will issue an error or a warning whenever
 an enum switch statement lacks a default case;
 if no default case is given, additionally one error or warning is issued
 regarding each enum constant for which a corresponding case label is lacking.

OK?
Comment 11 Markus Keller CLA 2012-03-02 02:19:11 EST 
>  When enabled, the compiler will issue an error or a warning whenever
>  an enum switch statement lacks a default case;
>  if no default case is given, additionally one error or warning is issued
>  regarding each enum constant for which a corresponding case label is lacking.

Sounds good.
Comment 12 Stephan Herrmann CLA 2012-03-02 17:02:34 EST 
Javadoc update is in commit 4de8875392f1614ce3d9b4e720843df3cd1fa2b7
Help update in commit 8718c719fb8c1327d8aae9ff6af012dbe41775c6 (using the wording from bug 372818 comment 2)
Comment 13 Srikanth Sankaran CLA 2012-03-12 05:27:05 EDT 
Verified for 3.8 M6 using Build id: I20120306-0800
Comment 14 Ayushman Jain CLA 2012-03-12 05:38:06 EDT 
(In reply to comment #7)
> [..]
> Looking at the doc I find that CompilerOptions.IncompleteEnumSwitch is rendered
> as "Enum type constant not covered on 'switch'"
> 
> Therefore I propose to rephrase the UI label to s.t. like
>    "Incomplete enum switch"
> :)

Is this covered by some bug request? Or was this proposal intentionally dropped?
Comment 15 Deepak Azad CLA 2012-03-12 05:55:57 EDT 
(In reply to comment #14)
> (In reply to comment #7)
> > [..]
> > Looking at the doc I find that CompilerOptions.IncompleteEnumSwitch is rendered
> > as "Enum type constant not covered on 'switch'"
> > 
> > Therefore I propose to rephrase the UI label to s.t. like
> >    "Incomplete enum switch"
> > :)
> 
> Is this covered by some bug request? Or was this proposal intentionally
> dropped?

The UI label has already been updated.