Bug 545259 - [12][quick fix][switch statement] 'Add missing case statements'
Summary: [12][quick fix][switch statement] 'Add missing case statements'
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.11   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: BETA J12   Edit
Assignee: Noopur Gupta CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 545988 (view as bug list)
Depends on:
Blocks: 545120
  Show dependency tree
 
Reported: 2019-03-11 07:29 EDT by Noopur Gupta CLA
Modified: 2019-04-04 06:53 EDT (History)
5 users (show)

See Also:

Attachments
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 Noopur Gupta CLA 2019-03-11 07:29:20 EDT 
public class SS_MissingCase {
	public static void bar1(Day day) {
		switch (day) {
		case MONDAY, FRIDAY -> System.out.println(Day.SUNDAY);
		case TUESDAY                -> System.out.println(7);
		case THURSDAY, SATURDAY     -> System.out.println(8);
		}
	}
}

- In the above example, the QF 'Add missing case statements' adds the following resulting in compile error:

		case SUNDAY:
			break;
		case WEDNESDAY:
			break;
		default:
			break;

- For default, we can add:
default -> throw new IllegalArgumentException("Unexpected value: " + day);

- For case statements, what should be added? An empty block i.e. {}? Probably with a //TODO comment? 

- Should all missing cases be added in a single statement i.e. 
case SUNDAY, WEDNESDAY -> ...  or separately?
Comment 1 Stephan Herrmann CLA 2019-03-11 07:54:30 EDT 
(In reply to Noopur Gupta from comment #0)
> - For case statements, what should be added? An empty block i.e. {}?
> Probably with a //TODO comment? 

Quick idea: throw new UnsupportedOperation("Unimplemented switch case for "+day);

> - Should all missing cases be added in a single statement i.e. 
> case SUNDAY, WEDNESDAY -> ...  or separately?

My gut feelings says, explicitly enumerated cases (vs. default) should normally be separate.

Do you have plans for quick fixes "split case labels" / "join case labels"?
Once that is provided, the above question will have low relevance :)
Comment 2 Noopur Gupta CLA 2019-03-11 08:21:26 EDT 
(In reply to Stephan Herrmann from comment #1)
> (In reply to Noopur Gupta from comment #0)
> > - For case statements, what should be added? An empty block i.e. {}?
> > Probably with a //TODO comment? 
> 
> Quick idea: throw new UnsupportedOperation("Unimplemented switch case for
> "+day);
This will be fine. It can be shortened as:
throw new UnsupportedOperationException("Unimplemented case: "+ day);

We can have any number of missing cases being added by this QF, along with the default case. I can check if we can put them in linked mode so that they are selected by default and the user can edit each case quickly.

> > - Should all missing cases be added in a single statement i.e. 
> > case SUNDAY, WEDNESDAY -> ...  or separately?
> My gut feelings says, explicitly enumerated cases (vs. default) should
> normally be separate.
> 
> Do you have plans for quick fixes "split case labels" / "join case labels"?
Yes, we can provide these quick assists. But it can take some time until they are available. So, until then, we can create separate case statements. We can revisit it after those QAs are available.
Comment 3 Noopur Gupta CLA 2019-03-13 06:25:53 EDT 
(In reply to Noopur Gupta from comment #2)
> (In reply to Stephan Herrmann from comment #1)
> > (In reply to Noopur Gupta from comment #0)
> > Do you have plans for quick fixes "split case labels" / "join case labels"?
> Yes, we can provide these quick assists. 
Created bug 545261 and bug 545262.
Comment 4 Eclipse Genie CLA 2019-03-14 16:02:54 EDT 
New Gerrit change created: https://git.eclipse.org/r/138786
Comment 5 Noopur Gupta CLA 2019-03-18 01:39:19 EDT 
Roland, I already have a WIP patch for this which I will complete and upload soon. The fix looks different in both so I will let you compare what is missing in each of the two patches.
Comment 6 Noopur Gupta CLA 2019-04-01 02:12:03 EDT 
*** Bug 545988 has been marked as a duplicate of this bug. ***
Comment 7 Eclipse Genie CLA 2019-04-03 08:42:00 EDT 
New Gerrit change created: https://git.eclipse.org/r/139951
Comment 8 Noopur Gupta CLA 2019-04-03 08:47:59 EDT 
(In reply to Eclipse Genie from comment #7)
> New Gerrit change created: https://git.eclipse.org/r/139951
TODO: Add tests.
Comment 9 Eclipse Genie CLA 2019-04-04 04:27:40 EDT 
New Gerrit change created: https://git.eclipse.org/r/140008
Comment 12 Noopur Gupta CLA 2019-04-04 06:23:05 EDT 
(In reply to Eclipse Genie from comment #11)
> Gerrit change https://git.eclipse.org/r/140008 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=b8dab194984f6314e0eb6c93c4c62f92b009940f
Added tests. Need to add N&N entry for M1.
Comment 13 Eclipse Genie CLA 2019-04-04 06:33:30 EDT 
New Gerrit change created: https://git.eclipse.org/r/140023
Comment 14 Noopur Gupta CLA 2019-04-04 06:39:49 EDT 
(In reply to Eclipse Genie from comment #13)
> New Gerrit change created: https://git.eclipse.org/r/140023
Gerrit for BETA_JAVA_12 branch.

It is failing with the following error and I am not sure how to fix it:

Caused by: org.eclipse.equinox.p2.core.ProvisionException: No repository found at http://download.eclipse.org/eclipse/updates/4.11-Y-builds.

Releasing without successful Gerrit build...
Comment 15 Eclipse Genie CLA 2019-04-04 06:40:43 EDT 
Gerrit change https://git.eclipse.org/r/140023 was merged to [BETA_JAVA_12].
Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=efd2ae1bba42f2edf0f56a93bbf9a155ad9a340a
Comment 16 Eclipse Genie CLA 2019-04-04 06:52:57 EDT 
New Gerrit change created: https://git.eclipse.org/r/140025