Re: Bug: java/text/MessageFormat.java

[Mark Wielaard]

Hi,

On Tue, 2004-06-22 at 23:53, Ito Kazumitsu wrote:
> >>>>> ":" == Ito Kazumitsu <address@hidden> writes:
> :> As reported to the Kaffe mailing list:
> :> http://www.kaffe.org/pipermail/kaffe/2004-June/046657.html
> :> java/text/MessageFormat.java has a bug.
> :> 
> :> This is my suggested patch.
> 
> In addition to that, I found another difference between the API
> document and Classpath's implementation.
> 
> I made some test cases for checking these bugs and updated the
> mauve test: gnu/testlet/java/text/MessageFormat/format.java.
> 
> My patch follows.
> 
> Changelog entry:
> 2004-06-22  Ito Kazumitsu  <address@hidden>
> 
>       * java/text/MessageFormat.java
>       (formatInternal): Append "{n}" if argument n is unavailable.
>       (format(Object, StringBuffer, FieldPosition)): This should be
>       equivalent to format(Object[], StringBuffer, FieldPosition).

I have just checked this in with a small API update for that last method
since at first I thought the original implementation was correct. And I
updated the copyright year list. (diff attached).

Sorry for the long delay. I had already reviewed it but forgot to apply
it. With this almost all Mauve java.text tests succeed! And no new
regressions were introduced.

Thanks,

Mark

Index: java/text/MessageFormat.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/text/MessageFormat.java,v
retrieving revision 1.10
diff -u -r1.10 MessageFormat.java
--- java/text/MessageFormat.java        28 Apr 2004 19:17:09 -0000      1.10
+++ java/text/MessageFormat.java        8 Jul 2004 10:26:02 -0000
@@ -1,5 +1,5 @@
 /* MessageFormat.java - Localized message formatting.
-   Copyright (C) 1999, 2001, 2002 Free Software Foundation, Inc.
+   Copyright (C) 1999, 2001, 2002, 2004 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -157,7 +157,7 @@
      * This is the attribute set for all characters produced
      * by MessageFormat during a formatting.
      */
-    public static final MessageFormat.Field ARGUMENT = new Field("argument");
+    public static final MessageFormat.Field ARGUMENT = new 
MessageFormat.Field("argument");
 
     // For deserialization
     private Field()
@@ -414,10 +414,13 @@
 
     for (int i = 0; i < elements.length; ++i)
       {
-       if (elements[i].argNumber >= arguments.length)
-         throw new IllegalArgumentException("Not enough arguments given");
+       Object thisArg = null;
+       boolean unavailable = false;
+       if (arguments == null || elements[i].argNumber >= arguments.length)
+         unavailable = true;
+       else
+         thisArg = arguments[elements[i].argNumber];
 
-       Object thisArg = arguments[elements[i].argNumber];
        AttributedCharacterIterator iterator = null;
 
        Format formatter = null;
@@ -425,22 +428,27 @@
        if (fp != null && i == fp.getField() && fp.getFieldAttribute() == 
Field.ARGUMENT)
          fp.setBeginIndex(appendBuf.length());
 
-       if (elements[i].setFormat != null)
-         formatter = elements[i].setFormat;
-       else if (elements[i].format != null)
+       if (unavailable)
+         appendBuf.append("{" + elements[i].argNumber + "}");
+       else
          {
-           if (elements[i].formatClass != null
-               && ! elements[i].formatClass.isInstance(thisArg))
-             throw new IllegalArgumentException("Wrong format class");
+           if (elements[i].setFormat != null)
+             formatter = elements[i].setFormat;
+           else if (elements[i].format != null)
+             {
+               if (elements[i].formatClass != null
+                   && ! elements[i].formatClass.isInstance(thisArg))
+                 throw new IllegalArgumentException("Wrong format class");
            
-           formatter = elements[i].format;
+               formatter = elements[i].format;
+             }
+           else if (thisArg instanceof Number)
+             formatter = NumberFormat.getInstance(locale);
+           else if (thisArg instanceof Date)
+             formatter = DateFormat.getTimeInstance(DateFormat.DEFAULT, 
locale);
+           else
+             appendBuf.append(thisArg);
          }
-       else if (thisArg instanceof Number)
-         formatter = NumberFormat.getInstance(locale);
-       else if (thisArg instanceof Date)
-         formatter = DateFormat.getTimeInstance(DateFormat.DEFAULT, locale);
-       else
-         appendBuf.append(thisArg);
 
        if (fp != null && fp.getField() == i && fp.getFieldAttribute() == 
Field.ARGUMENT)
          fp.setEndIndex(appendBuf.length());
@@ -496,29 +504,18 @@
   }
 
   /**
-   * Returns the pattern with the formatted objects.
+   * Returns the pattern with the formatted objects.  The first argument
+   * must be a array of Objects.
+   * This is equivalent to format((Object[]) objectArray, appendBuf, fpos)
    *
-   * @param source The object to be formatted.
-   * @param result The StringBuffer where the text is appened.
+   * @param objectArray The object array to be formatted.
+   * @param appendBuf The StringBuffer where the text is appened.
    * @param fpos A FieldPosition object (it is ignored).
    */
-  public final StringBuffer format (Object singleArg, StringBuffer appendBuf,
+  public final StringBuffer format (Object objectArray, StringBuffer appendBuf,
                                    FieldPosition fpos)
   {
-    Object[] args;
-
-    if (singleArg instanceof Object[])
-      {
-       // This isn't specified in any manual, but it follows the
-       // JDK implementation.
-       args = (Object[]) singleArg;
-      }
-    else
-      {
-       args = new Object[1];
-       args[0] = singleArg;
-      }
-    return format (args, appendBuf, fpos);
+    return format ((Object[])objectArray, appendBuf, fpos);
   }
 
   /**

signature.asc
Description: This is a digitally signed message part