Bug 283133 - [formatter] IAE when pasting a snippet
Summary: [formatter] IAE when pasting a snippet
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M1   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-10 08:55 EDT by Frederic Fusier CLA
Modified: 2009-08-05 02:41 EDT (History)
4 users (show)

See Also:


Attachments
Stack trace (4.64 KB, text/plain)
2009-07-10 08:57 EDT, Frederic Fusier CLA
no flags Details
Proposed fix (1.38 KB, patch)
2009-07-14 15:52 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2009-07-10 08:55:31 EDT
Using I20090707-0800 but already happens in 3.5.0...

I wanted to create a test case for bug 281655 and for this copy the code snippet given in the bug description and pasted it in a project of my runtime workbench workspace. The I got an error dialog from the refactoring and from the error log the attached stack trace...

Investigating a little bit it seems that this is due to the fact that I configured my workspace to use the bug 281533 formatter profile which has 0 for tabs and indentation width...

I open this bug against JDT/UI instead of JDT/Core as it's clearly specified in the javadoc of method measureIndentUnits(CharSequence line, int tabWidth, int indentWidth) of org.eclipse.jdt.core.formatter.IndentManipulation that:
 * @exception IllegalArgumentException if:
 * <ul>
 * <li>the given <code>indentWidth</code> is lower or equals to zero</li>
 * <li>the given <code>tabWidth</code> is lower than zero</li>
 * <li>the given <code>line</code> is null</li>
 * </ul>

Hence callers,  org.eclipse.jdt.internal.corext.util.Strings.computeIndentUnits(Strings.java:294) in this peculiar case should not call this method when indentWidth is equals to 0

Note that the exception is raised whatever the pasted CU is, e.g.:
class X {}
Comment 1 Frederic Fusier CLA 2009-07-10 08:57:07 EDT
Created attachment 141289 [details]
Stack trace
Comment 2 Dani Megert CLA 2009-07-14 08:52:48 EDT
I would argue that this is better fixed in JDT Core by loosening the spec: clients call that API by feeding the corresponding JDT Core options and since JDT Core allows 0 for the indent it would be fair to also allow it here. Also, if not changed in JDT Core then all clients which followed that pattern have to add some special case code.

Moving back to JDT Core for consideration.
Comment 3 Olivier Thomann CLA 2009-07-14 10:08:15 EDT
I concur with Daniel's comment.
We have an inconsistency. If we allow 0 for the indentation level, we must loosen the spec of the method in measureIndentUnits.
If indentWidth is 0, we could return 0.
Comment 4 Olivier Thomann CLA 2009-07-14 15:52:32 EDT
Created attachment 141562 [details]
Proposed fix
Comment 5 Olivier Thomann CLA 2009-07-14 15:53:30 EDT
Released for 3.6M1.
Comment 6 Olivier Thomann CLA 2009-07-14 15:56:28 EDT
Added regression test org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests#test722
Comment 7 Markus Keller CLA 2009-08-04 08:52:00 EDT
Bug 285565 requests that the rest of the IndentManipulation methods should also accept 0 for indentWidth.
Comment 8 Srikanth Sankaran CLA 2009-08-05 02:41:30 EDT
Verified for 3.6M1 by code inspection. This point fix needs to be
applied to several other places to address bug 285565