Bug 255640 - [spec] Methods Signature.toCharArray(..) have unclear precondition
Summary: [spec] Methods Signature.toCharArray(..) have unclear precondition
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4.1   Edit
Hardware: Other Linux
: P3 normal (vote)
Target Milestone: 3.6 M5   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-18 08:50 EST by Stephan Herrmann CLA
Modified: 2010-01-25 06:23 EST (History)
3 users (show)

See Also:


Attachments
Proposed fix (2.42 KB, patch)
2009-12-10 11:44 EST, 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 Stephan Herrmann CLA 2008-11-18 08:50:36 EST
Build ID: M20080911-1700

Looking at the javadocs it is not clear how the toCharArray family of
methods in org.eclipse.jdt.core.Signature handles empty char[] as input.
The docs only say:
  @exception IllegalArgumentException if the signature is not syntactically correct
One could argue that an empty string cannot contain syntax errors.

Digging into the implementation one hint is found in appendTypeSignature:
  // need a minimum 1 char

The following snippet from toCharArray(char[]) looks confusing to me:
  int sigLength = signature.length;
  if (sigLength == 0 || signature[0] == C_PARAM_START || signature[0] == C_GENERIC_START) {
    return toCharArray(signature, CharOperation.NO_CHAR, null, true, true);
  }

Note, that the second toCharArray-method is intended for parsing
method signatures only and it fails with an IllegalArgumentException
if no C_PARAM_START is found within the input. For length 0 this
will invariable happen. Without changing the behavior
the same could be achieved (in a less confusing way) by:
  int sigLength = signature.length;
  if (sigLength == 0)
    throw new IllegalArgumentException();
  if (signature[0] == C_PARAM_START || signature[0] == C_GENERIC_START) {
    return toCharArray(signature, CharOperation.NO_CHAR, null, true, true);
  }
If that's indeed the intended behavior I'd suggest to add to the docs that
the input must not be an empty string/char[].
Comment 1 Olivier Thomann CLA 2009-12-09 10:26:40 EST
(In reply to comment #0)
> If that's indeed the intended behavior I'd suggest to add to the docs that
> the input must not be an empty string/char[].
This is a good suggestion. I missed it.
Will be fixed for 3.6M5.
Comment 2 Olivier Thomann CLA 2009-12-10 11:44:47 EST
Created attachment 154253 [details]
Proposed fix
Comment 3 Olivier Thomann CLA 2009-12-10 11:47:00 EST
Released for 3.6M5.
Verification must be done by checking the javadoc.
Comment 4 Ayushman Jain CLA 2010-01-25 06:18:12 EST
Verified for 3.6M5 through code inspection.
Comment 5 Srikanth Sankaran CLA 2010-01-25 06:23:03 EST
Verified.