Bug 331551

Comment 1 Dani Megert 2010-12-01 11:38:54 EST

Test Case 1:
1. paste
----------%<----------
	int foo() {
		return "".length();
	}
----------%<----------
2. place caret after '(' on the return line
3. press 'Enter'
4. Ctrl+I
==> indent gets destroyed


Test Case 2:
1. paste
----------%<----------
public class Test {
	private void helper2(boolean[] booleans) {
		if (booleans[0]) {

		}
	}
}
----------%<----------
2. place caret before ')' on the if line
3. press 'Enter'
4. Ctrl+I
==> indent gets destroyed

Comment 2 Rajesh 2011-02-04 10:21:45 EST

Created attachment 188325 [details]
Patch

Besides adding a new test, I have modified 'testAnonymousIndentation2' in JavaHeuristicScannerTest.java.

Comment 3 Dani Megert 2011-02-07 03:48:26 EST

Deepak, please make the first review.

Comment 4 Deepak Azad 2011-02-07 07:44:09 EST

Rajesh, is there a reason for this check "if ((line != -1) && (line != fLine))" ? 

The tests pass with the following code, and I cannot think of a case where this would fail.
-----------------------------------------------------------------------
if (matchParen) {
	if (skipScope(Symbols.TokenLPAREN, Symbols.TokenRPAREN)) {
		fIndent= fPrefs.prefContinuationIndent;
		return fPosition;
	} else {
-----------------------------------------------------------------------

Comment 5 Rajesh 2011-02-08 01:13:13 EST

Created attachment 188497 [details]
Patch

(In reply to comment #4)
> Rajesh, is there a reason for this check "if ((line != -1) && (line != fLine))"
> ? 
> 
> The tests pass with the following code, and I cannot think of a case where this
> would fail.
> -----------------------------------------------------------------------
> if (matchParen) {
>     if (skipScope(Symbols.TokenLPAREN, Symbols.TokenRPAREN)) {
>         fIndent= fPrefs.prefContinuationIndent;
>         return fPosition;
>     } else {
> -----------------------------------------------------------------------

Being over cautious and trying to be sure that it is a continuation scenario. But the following 2 lines (785/786) in the execution path already ensure this which is why none of the potential failure cases never occur - 

"if (isFirstTokenOnLine)
		matchParen= true;"

Thx. New patch removes the check.

Comment 6 Deepak Azad 2011-02-08 06:43:29 EST

(In reply to comment #5)
> Created attachment 188497 [details] [diff]
> Patch

Please attach a patch with tests included. 

+1 for the fix.

Comment 7 Rajesh 2011-02-08 08:15:52 EST

Created attachment 188512 [details]
Patch with tests.

Comment 8 Deepak Azad 2011-02-08 08:46:51 EST

(In reply to comment #7)
> Created attachment 188512 [details] [diff]
> Patch with tests.

Committed to HEAD.

Comment 9 Deepak Azad 2011-02-08 08:48:37 EST

.

Comment 10 Dani Megert 2011-02-08 11:25:32 EST

Thanks for the patch Rajesh.