Bug 246627 - [ast rewrite] Wrong indentation for statement inserted before SwitchCase
Summary: [ast rewrite] Wrong indentation for statement inserted before SwitchCase
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M6   Edit
Assignee: David Audel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-08 13:47 EDT by Markus Keller CLA
Modified: 2009-03-10 09:38 EDT (History)
3 users (show)

See Also:

Attachments
Proposed fix (4.64 KB, patch)
2009-02-04 07:01 EST, David Audel CLA 		no flags Details | Diff
Correct patch (35.79 KB, patch)
2009-02-04 07:03 EST, David Audel CLA 		no flags Details | Diff
Proposed patch for JDTUI tests (2.90 KB, patch)
2009-02-04 07:13 EST, David Audel CLA 		no flags Details | Diff
Show Obsolete (1) 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 Markus Keller CLA 2008-09-08 13:47:57 EDT 
I20080617-2000 and HEAD

ASTRewrite produces wrong indentation for a statement inserted before a SwitchCase:

- have:
		switch (1) {
		case 1:
			System.out.println();
		case 2:
			System.out.println();
		}

- format with Eclipse [built-in] formatter profile
- enable warning for "'switch' case fall-through"
- quick fix "Insert 'break' statement"

=> result:
		switch (1) {
		case 1:
			System.out.println();
		break;
			case 2:
			System.out.println();
		}

=> expected: same as when I format the result again:
		switch (1) {
		case 1:
			System.out.println();
			break;
		case 2:
			System.out.println();
		}

The implementation in LocalCorrectionsSubProcessor.addFallThroughProposals(..) is trivial:
    listRewrite.insertBefore(ast.newBreakStatement(), switchCaseNode, null);
Comment 1 Philipe Mulet CLA 2008-11-27 10:05:22 EST 
This one is annoying...
Comment 2 Markus Keller CLA 2008-12-04 08:27:33 EST 
Same problem when you remove a statement in front of a SwitchCase, e.g. have:

	void m() {
		switch (1) {
		case 1:
			System.out.println("Hello");
			break;
			System.out.println("Unreachable");
			break;
		case 2:
			System.out.println("Hello3");
			return;
		default:
			System.out.println("Hell4");
			break;
		}
	}

Quick Fix to remove unreachable code gives:

	void m() {
		switch (1) {
		case 1:
			System.out.println("Hello");
			break;
			case 2:
			System.out.println("Hello3");
			return;
		default:
			System.out.println("Hell4");
			break;
		}
	}

=> "case 2:" should keep original indentation. See bug 257524 for a problem in the formatter with unreachable code.
Comment 3 David Audel CLA 2009-02-04 07:01:29 EST 
Created attachment 124661 [details]
Proposed fix

All statements in a switch statement do not have the same indentation and  ASTRewriteAnalyzer does not update this indentation when necessary.

This patch improve ASTRewriteAnalyzer to produce correct indentation.
Comment 4 David Audel CLA 2009-02-04 07:03:45 EST 
Created attachment 124662 [details]
Correct patch

I attached the wrong patch, the correct patch is this one.
Comment 5 David Audel CLA 2009-02-04 07:13:12 EST 
Created attachment 124665 [details]
Proposed patch for JDTUI tests

Some JDTUI regressions test failed with the fix. This is a patch for these tests.
Comment 6 Dani Megert CLA 2009-02-04 08:53:12 EST 
OK, will look at the patch once you've committed the JDT Core side.
Comment 7 David Audel CLA 2009-02-04 08:58:07 EST 
Released for 3.5M6.

Released jdtcore patch and added tests:
  ASTRewritingStatementsTest#testSwitchStatement2() -> testSwitchStatement10()
Comment 8 Markus Keller CLA 2009-02-04 08:59:28 EST 
Looks good, I released the UI patch.
Comment 9 Olivier Thomann CLA 2009-03-10 09:38:03 EDT 
Verified for 3.5M6 using I20090310-0100.