Cherry-pick: ticket:13477 Tweak the DecimalFormat mapping function to correctly handle positive prefix and suffix overrides.

http://bugs.icu-project.org/trac/changeset/40672

Change-Id: I82a3e4156654d2ae8a36e85775977af47abd2ce5

2 files changed, 89 insertions, 54 deletions

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 a64c3d708..bf3245146 100644
--- 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 9d681b56c..5c6a08c01 100644
--- 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);
         }
     }