ICU-13489 Merging #13477(r40672) DecimalFormat setPositivePrefix also changes negativ...

X-SVN-Rev: 40693

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/AffixUtils.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/AffixUtils.java
index a64c3d708cfad5ccff6ce9fd740948aef24ac8fa..bf32451468864b08ac4773c967e4d0e00e2adb6f 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/AffixUtils.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/AffixUtils.java
@@ -225,8 +225,9 @@ public class AffixUtils {
     return output.length() - startLength;
   }
 
-  /** Version of {@link #escape} that returns a String. */
+  /** Version of {@link #escape} that returns a String, or null if input is null. */
   public static String escape(CharSequence input) {
+    if (input == null) return null;
     StringBuilder sb = new StringBuilder();
     escape(input, sb);
     return sb.toString();

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java
index 9d681b56c7f64f992d026175e9c530f97aeeb96e..5c6a08c01032430e4b53508c554e0e5a35a3a0c2 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java
@@ -86,15 +86,7 @@ final class NumberPropertyMapper {
 
         AffixPatternProvider affixProvider;
         if (properties.getCurrencyPluralInfo() == null) {
-            affixProvider = new PropertiesAffixPatternProvider(
-                    properties.getPositivePrefix() != null ? AffixUtils.escape(properties.getPositivePrefix())
-                            : properties.getPositivePrefixPattern(),
-                    properties.getPositiveSuffix() != null ? AffixUtils.escape(properties.getPositiveSuffix())
-                            : properties.getPositiveSuffixPattern(),
-                    properties.getNegativePrefix() != null ? AffixUtils.escape(properties.getNegativePrefix())
-                            : properties.getNegativePrefixPattern(),
-                    properties.getNegativeSuffix() != null ? AffixUtils.escape(properties.getNegativeSuffix())
-                            : properties.getNegativeSuffixPattern());
+            affixProvider = new PropertiesAffixPatternProvider(properties);
         } else {
             affixProvider = new CurrencyPluralInfoAffixProvider(properties.getCurrencyPluralInfo());
         }
@@ -329,85 +321,127 @@ final class NumberPropertyMapper {
     }
 
     private static class PropertiesAffixPatternProvider implements AffixPatternProvider {
-        private final String posPrefixPattern;
-        private final String posSuffixPattern;
-        private final String negPrefixPattern;
-        private final String negSuffixPattern;
-
-        public PropertiesAffixPatternProvider(String ppp, String psp, String npp, String nsp) {
-            if (ppp == null)
-                ppp = "";
-            if (psp == null)
-                psp = "";
-            if (npp == null && nsp != null)
-                npp = "-"; // TODO: This is a hack.
-            if (nsp == null && npp != null)
-                nsp = "";
-            posPrefixPattern = ppp;
-            posSuffixPattern = psp;
-            negPrefixPattern = npp;
-            negSuffixPattern = nsp;
+        private final String posPrefix;
+        private final String posSuffix;
+        private final String negPrefix;
+        private final String negSuffix;
+
+        public PropertiesAffixPatternProvider(DecimalFormatProperties properties) {
+            // There are two ways to set affixes in DecimalFormat: via the pattern string (applyPattern), and via the
+            // explicit setters (setPositivePrefix and friends).  The way to resolve the settings is as follows:
+            //
+            // 1) If the explicit setting is present for the field, use it.
+            // 2) Otherwise, follows UTS 35 rules based on the pattern string.
+            //
+            // Importantly, the explicit setters affect only the one field they override.  If you set the positive
+            // prefix, that should not affect the negative prefix.  Since it is impossible for the user of this class
+            // to know whether the origin for a string was the override or the pattern, we have to say that we always
+            // have a negative subpattern and perform all resolution logic here.
+
+            // Convenience: Extract the properties into local variables.
+            // Variables are named with three chars: [p/n][p/s][o/p]
+            // [p/n] => p for positive, n for negative
+            // [p/s] => p for prefix, s for suffix
+            // [o/p] => o for escaped custom override string, p for pattern string
+            String ppo = AffixUtils.escape(properties.getPositivePrefix());
+            String pso = AffixUtils.escape(properties.getPositiveSuffix());
+            String npo = AffixUtils.escape(properties.getNegativePrefix());
+            String nso = AffixUtils.escape(properties.getNegativeSuffix());
+            String ppp = properties.getPositivePrefixPattern();
+            String psp = properties.getPositiveSuffixPattern();
+            String npp = properties.getNegativePrefixPattern();
+            String nsp = properties.getNegativeSuffixPattern();
+
+            if (ppo != null) {
+                posPrefix = ppo;
+            } else if (ppp != null) {
+                posPrefix = ppp;
+            } else {
+                // UTS 35: Default positive prefix is empty string.
+                posPrefix = "";
+            }
+
+            if (pso != null) {
+                posSuffix = pso;
+            } else if (psp != null) {
+                posSuffix = psp;
+            } else {
+                // UTS 35: Default positive suffix is empty string.
+                posSuffix = "";
+            }
+
+            if (npo != null) {
+                negPrefix = npo;
+            } else if (npp != null) {
+                negPrefix = npp;
+            } else {
+                // UTS 35: Default negative prefix is "-" with positive prefix.
+                // Important: We prepend the "-" to the pattern, not the override!
+                negPrefix = ppp == null ? "-" : "-" + ppp;
+            }
+
+            if (nso != null) {
+                negSuffix = nso;
+            } else if (nsp != null) {
+                negSuffix = nsp;
+            } else {
+                // UTS 35: Default negative prefix is the positive prefix.
+                negSuffix = psp == null ? "" : psp;
+            }
         }
 
         @Override
         public char charAt(int flags, int i) {
-            boolean prefix = (flags & Flags.PREFIX) != 0;
-            boolean negative = (flags & Flags.NEGATIVE_SUBPATTERN) != 0;
-            if (prefix && negative) {
-                return negPrefixPattern.charAt(i);
-            } else if (prefix) {
-                return posPrefixPattern.charAt(i);
-            } else if (negative) {
-                return negSuffixPattern.charAt(i);
-            } else {
-                return posSuffixPattern.charAt(i);
-            }
+            return getStringForFlags(flags).charAt(i);
         }
 
         @Override
         public int length(int flags) {
+            return getStringForFlags(flags).length();
+        }
+
+        private String getStringForFlags(int flags) {
             boolean prefix = (flags & Flags.PREFIX) != 0;
             boolean negative = (flags & Flags.NEGATIVE_SUBPATTERN) != 0;
             if (prefix && negative) {
-                return negPrefixPattern.length();
+                return negPrefix;
             } else if (prefix) {
-                return posPrefixPattern.length();
+                return posPrefix;
             } else if (negative) {
-                return negSuffixPattern.length();
+                return negSuffix;
             } else {
-                return posSuffixPattern.length();
+                return posSuffix;
             }
         }
 
         @Override
         public boolean positiveHasPlusSign() {
-            return AffixUtils.containsType(posPrefixPattern, AffixUtils.TYPE_PLUS_SIGN)
-                    || AffixUtils.containsType(posSuffixPattern, AffixUtils.TYPE_PLUS_SIGN);
+            return AffixUtils.containsType(posPrefix, AffixUtils.TYPE_PLUS_SIGN)
+                    || AffixUtils.containsType(posSuffix, AffixUtils.TYPE_PLUS_SIGN);
         }
 
         @Override
         public boolean hasNegativeSubpattern() {
-            return negPrefixPattern != null;
+            // See comments in the constructor for more information on why this is always true.
+            return true;
         }
 
         @Override
         public boolean negativeHasMinusSign() {
-            return AffixUtils.containsType(negPrefixPattern, AffixUtils.TYPE_MINUS_SIGN)
-                    || AffixUtils.containsType(negSuffixPattern, AffixUtils.TYPE_MINUS_SIGN);
+            return AffixUtils.containsType(negPrefix, AffixUtils.TYPE_MINUS_SIGN)
+                    || AffixUtils.containsType(negSuffix, AffixUtils.TYPE_MINUS_SIGN);
         }
 
         @Override
         public boolean hasCurrencySign() {
-            return AffixUtils.hasCurrencySymbols(posPrefixPattern) || AffixUtils.hasCurrencySymbols(posSuffixPattern)
-                    || AffixUtils.hasCurrencySymbols(negPrefixPattern)
-                    || AffixUtils.hasCurrencySymbols(negSuffixPattern);
+            return AffixUtils.hasCurrencySymbols(posPrefix) || AffixUtils.hasCurrencySymbols(posSuffix)
+                    || AffixUtils.hasCurrencySymbols(negPrefix) || AffixUtils.hasCurrencySymbols(negSuffix);
         }
 
         @Override
         public boolean containsSymbolType(int type) {
-            return AffixUtils.containsType(posPrefixPattern, type) || AffixUtils.containsType(posSuffixPattern, type)
-                    || AffixUtils.containsType(negPrefixPattern, type)
-                    || AffixUtils.containsType(negSuffixPattern, type);
+            return AffixUtils.containsType(posPrefix, type) || AffixUtils.containsType(posSuffix, type)
+                    || AffixUtils.containsType(negPrefix, type) || AffixUtils.containsType(negSuffix, type);
         }
     }
 

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
index 1cf29b4eb37638cabd484583d218af67c79655b6..74290f3b0b0ba5608d6666daabfeb96b4cf8dd37 100644 (file)
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
@@ -5902,4 +5902,18 @@ public class NumberFormatTest extends TestFmwk {
         assertEquals("Narrow currency symbol for USD in en_CA is $",
                 "$123.45", df.format(123.45));
     }
+
+    @Test
+    public void TestAffixOverrideBehavior() {
+        DecimalFormat df = (DecimalFormat) NumberFormat.getInstance(ULocale.ENGLISH);
+        expect2(df, 100, "100");
+        expect2(df, -100, "-100");
+        // This is not the right way to set an override plus sign, but we need to support it for compatibility.
+        df.setPositivePrefix("+");
+        expect2(df, 100, "+100");
+        expect2(df, -100, "-100"); // note: the positive prefix does not affect the negative prefix
+        df.applyPattern("a0");
+        expect2(df, 100, "a100");
+        expect2(df, -100, "-a100");
+    }
 }