Fix incorrect handling of format which should not produce any digit for zero

Also include the internally computed format-string when the resulting format causes an exception

One other case with question marks is still not handled correctly, though

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1874950 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/format/CellNumberFormatter.java

@@ -22,6 +22,7 @@ import java.text.FieldPosition;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Formatter;
+import java.util.IllegalFormatException;
 import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
@@ -232,15 +233,15 @@ public class CellNumberFormatter extends CellFormatter {
         integerSpecials.addAll(specials.subList(0, integerEnd()));
 
         if (exponent == null) {
-            StringBuilder fmtBuf = new StringBuilder("%");
-
             int integerPartWidth = calculateIntegerPartWidth();
             int totalWidth = integerPartWidth + fractionPartWidth;
 
-            fmtBuf.append('0').append(totalWidth).append('.').append(precision);
-
-            fmtBuf.append("f");
-            printfFmt = fmtBuf.toString();
+            // need to handle empty width specially as %00.0f fails during formatting
+            if(totalWidth == 0) {
+                printfFmt = "";
+            } else {
+                printfFmt = "%0" + totalWidth + '.' + precision + "f";
+            }
             decimalFmt = null;
         } else {
             StringBuffer fmtBuf = new StringBuffer();
@@ -278,7 +279,7 @@ public class CellNumberFormatter extends CellFormatter {
     private DecimalFormatSymbols getDecimalFormatSymbols() {
         return DecimalFormatSymbols.getInstance(locale);
     }
-    
+
     private static void placeZeros(StringBuffer sb, List<Special> specials) {
         for (Special s : specials) {
             if (isDigitFmt(s)) {
@@ -458,6 +459,8 @@ public class CellNumberFormatter extends CellFormatter {
             StringBuffer result = new StringBuffer();
             try (Formatter f = new Formatter(result, locale)) {
                 f.format(locale, printfFmt, value);
+            } catch (IllegalFormatException e) {
+                throw new IllegalArgumentException("Format: " + printfFmt, e);
             }
 
             if (numerator == null) {
@@ -695,7 +698,7 @@ public class CellNumberFormatter extends CellFormatter {
             LOG.log(POILogger.ERROR, "error while fraction evaluation", ignored);
         }
     }
-    
+
     private String localiseFormat(String format) {
         DecimalFormatSymbols dfs = getDecimalFormatSymbols();
         if(format.contains(",") && dfs.getGroupingSeparator() != ',') {
@@ -711,8 +714,8 @@ public class CellNumberFormatter extends CellFormatter {
         }
         return format;
     }
-    
-    
+
+
     private static String replaceLast(String text, String regex, String replacement) {
         return text.replaceFirst("(?s)(.*)" + regex, "$1" + replacement);
     }

src/testcases/org/apache/poi/hssf/usermodel/TestUnfixedBugs.java

@@ -24,6 +24,7 @@ import java.io.IOException;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.util.HSSFColor;
+import org.apache.poi.ss.format.CellFormat;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
@@ -32,7 +33,7 @@ import org.junit.Test;
 
 /**
  * @author aviks
- * 
+ *
  * This testcase contains tests for bugs that are yet to be fixed. Therefore,
  * the standard ant test target does not run these tests. Run this testcase with
  * the single-test target. The names of the tests usually correspond to the
@@ -94,7 +95,7 @@ public final class TestUnfixedBugs {
         Sheet sheet = wb.getSheet("Sheet1");
         Row row = sheet.getRow(0);
         Cell cell = row.getCell(0);
-        
+
         HSSFColor bgColor = (HSSFColor) cell.getCellStyle().getFillBackgroundColorColor();
         String bgColorStr = bgColor.getTriplet()[0]+", "+bgColor.getTriplet()[1]+", "+bgColor.getTriplet()[2];
         //System.out.println(bgColorStr);
@@ -106,4 +107,24 @@ public final class TestUnfixedBugs {
         assertEquals("0, 128, 128", fontColorStr);
         wb.close();
     }
+
+    @Test
+    public void testDataFormattingWithQuestionMark() {
+        // The question mark in the format should be replaced by blanks, but
+        // this is currently not handled when producing the Java formatting and
+        // so we end up with a trailing zero here
+        CellFormat cfUK  = CellFormat.getInstance("??");
+        assertEquals("  ", cfUK.apply((double) 0).text);
+    }
+
+    @Test
+    public void testDataFormattingWithQuestionMarkAndPound() {
+        char pound = '\u00A3';
+
+        // The question mark in the format should be replaced by blanks, but
+        // this is currently not handled when producing the Java formatting and
+        // so we end up with a trailing zero here
+        CellFormat cfUK  = CellFormat.getInstance("_-[$£-809]* \"-\"??_-");
+        assertEquals(" "+pound+"   -  ", cfUK.apply((double) 0).text);
+    }
 }

src/testcases/org/apache/poi/ss/format/TestCellFormat.java

@@ -943,34 +943,33 @@ public class TestCellFormat {
 
         // For +ve numbers, should be Space + currency symbol + spaces + whole number with commas + space
         // (Except French, which is mostly reversed...)
-        assertEquals(" $   12 ", cfDft.apply(Double.valueOf(12.33)).text);
-        assertEquals(" $   12 ",  cfUS.apply(Double.valueOf(12.33)).text);
-        assertEquals(" "+pound+"   12 ", cfUK.apply(Double.valueOf(12.33)).text);
-        assertEquals(" 12   "+euro+" ", cfFR.apply(Double.valueOf(12.33)).text);
+        assertEquals(" $   12 ", cfDft.apply(12.33).text);
+        assertEquals(" $   12 ",  cfUS.apply(12.33).text);
+        assertEquals(" "+pound+"   12 ", cfUK.apply(12.33).text);
+        assertEquals(" 12   "+euro+" ", cfFR.apply(12.33).text);
 
-        assertEquals(" $   16,789 ", cfDft.apply(Double.valueOf(16789.2)).text);
-        assertEquals(" $   16,789 ",  cfUS.apply(Double.valueOf(16789.2)).text);
-        assertEquals(" "+pound+"   16,789 ", cfUK.apply(Double.valueOf(16789.2)).text);
-        assertEquals(" 16,789   "+euro+" ", cfFR.apply(Double.valueOf(16789.2)).text);
+        assertEquals(" $   16,789 ", cfDft.apply(16789.2).text);
+        assertEquals(" $   16,789 ",  cfUS.apply(16789.2).text);
+        assertEquals(" "+pound+"   16,789 ", cfUK.apply(16789.2).text);
+        assertEquals(" 16,789   "+euro+" ", cfFR.apply(16789.2).text);
 
         // For -ve numbers, gets a bit more complicated...
-        assertEquals("-$   12 ", cfDft.apply(Double.valueOf(-12.33)).text);
-        assertEquals(" $   -12 ",  cfUS.apply(Double.valueOf(-12.33)).text);
-        assertEquals("-"+pound+"   12 ", cfUK.apply(Double.valueOf(-12.33)).text);
-        assertEquals("-12   "+euro+" ", cfFR.apply(Double.valueOf(-12.33)).text);
+        assertEquals("-$   12 ", cfDft.apply(-12.33).text);
+        assertEquals(" $   -12 ",  cfUS.apply(-12.33).text);
+        assertEquals("-"+pound+"   12 ", cfUK.apply(-12.33).text);
+        assertEquals("-12   "+euro+" ", cfFR.apply(-12.33).text);
 
-        assertEquals("-$   16,789 ", cfDft.apply(Double.valueOf(-16789.2)).text);
-        assertEquals(" $   -16,789 ",  cfUS.apply(Double.valueOf(-16789.2)).text);
-        assertEquals("-"+pound+"   16,789 ", cfUK.apply(Double.valueOf(-16789.2)).text);
-        assertEquals("-16,789   "+euro+" ", cfFR.apply(Double.valueOf(-16789.2)).text);
+        assertEquals("-$   16,789 ", cfDft.apply(-16789.2).text);
+        assertEquals(" $   -16,789 ",  cfUS.apply(-16789.2).text);
+        assertEquals("-"+pound+"   16,789 ", cfUK.apply(-16789.2).text);
+        assertEquals("-16,789   "+euro+" ", cfFR.apply(-16789.2).text);
 
         // For zero, should be Space + currency symbol + spaces + Minus + spaces
-        assertEquals(" $   - ", cfDft.apply(Double.valueOf(0)).text);
-        // TODO Fix the exception this incorrectly triggers
-        //assertEquals(" $   - ",  cfUS.apply(Double.valueOf(0)).text);
+        assertEquals(" $   - ", cfDft.apply((double) 0).text);
+        assertEquals(" $   - ", cfUS.apply((double) 0).text);
         // TODO Fix these to not have an incorrect bonus 0 on the end
-        //assertEquals(" "+pound+"   -  ", cfUK.apply(Double.valueOf(0)).text);
-        //assertEquals(" -    "+euro+"  ", cfFR.apply(Double.valueOf(0)).text);
+        //assertEquals(" "+pound+"   -  ", cfUK.apply((double) 0).text);
+        //assertEquals(" -    "+euro+"  ", cfFR.apply((double) 0).text);
     }
 
     @Test