Porting fix for AYTF to C++, small comment updates.

cpp/src/phonenumbers/asyoutypeformatter.cc

@@ -146,7 +146,7 @@ AsYouTypeFormatter::AsYouTypeFormatter(const string& region_code)
       position_to_remember_(0),
       prefix_before_national_number_(),
       should_add_space_after_national_prefix_(false),
-      national_prefix_extracted_(),
+      extracted_national_prefix_(),
       national_number_(),
       possible_formats_() {
 }
@@ -316,7 +316,7 @@ void AsYouTypeFormatter::Clear() {
   last_match_position_ = 0;
   current_formatting_pattern_.clear();
   prefix_before_national_number_.clear();
-  national_prefix_extracted_.clear();
+  extracted_national_prefix_.clear();
   national_number_.clear();
   able_to_format_ = true;
   input_has_formatting_ = false;
@@ -414,9 +414,10 @@ void AsYouTypeFormatter::InputDigitWithOptionToRememberPosition(
     case 3:
       if (AttemptToExtractIdd()) {
         is_expecting_country_code_ = true;
+        // FALLTHROUGH_INTENDED
       } else {
         // No IDD or plus sign is found, might be entering in national format.
-        RemoveNationalPrefixFromNationalNumber(&national_prefix_extracted_);
+        RemoveNationalPrefixFromNationalNumber(&extracted_national_prefix_);
         AttemptToChooseFormattingPattern(phone_number);
         return;
       }
@@ -468,21 +469,25 @@ void AsYouTypeFormatter::AttemptToChoosePatternWithPrefixExtracted(
   AttemptToChooseFormattingPattern(formatted_number);
 }
 
+const string& AsYouTypeFormatter::GetExtractedNationalPrefix() const {
+  return extracted_national_prefix_;
+}
+
 bool AsYouTypeFormatter::AbleToExtractLongerNdd() {
-  if (national_prefix_extracted_.length() > 0) {
+  if (extracted_national_prefix_.length() > 0) {
     // Put the extracted NDD back to the national number before attempting to
     // extract a new NDD.
-    national_number_.insert(0, national_prefix_extracted_);
+    national_number_.insert(0, extracted_national_prefix_);
     // Remove the previously extracted NDD from prefixBeforeNationalNumber. We
     // cannot simply set it to empty string because people sometimes incorrectly
     // enter national prefix after the country code, e.g. +44 (0)20-1234-5678.
     int index_of_previous_ndd =
-        prefix_before_national_number_.find_last_of(national_prefix_extracted_);
+        prefix_before_national_number_.find_last_of(extracted_national_prefix_);
     prefix_before_national_number_.resize(index_of_previous_ndd);
   }
   string new_national_prefix;
   RemoveNationalPrefixFromNationalNumber(&new_national_prefix);
-  return national_prefix_extracted_ != new_national_prefix;
+  return extracted_national_prefix_ != new_national_prefix;
 }
 
 void AsYouTypeFormatter::AttemptToFormatAccruedDigits(
@@ -699,6 +704,9 @@ bool AsYouTypeFormatter::AttemptToExtractCountryCode() {
   }
   StrAppend(&prefix_before_national_number_, country_code);
   prefix_before_national_number_.push_back(kSeparatorBeforeNationalNumber);
+  // When we have successfully extracted the IDD, the previously extracted NDD
+  // should be cleared because it is no longer valid.
+  extracted_national_prefix_.clear();
 
   return true;
 }

cpp/src/phonenumbers/asyoutypeformatter.h

@@ -115,6 +115,8 @@ class AsYouTypeFormatter {
 
   void AttemptToChoosePatternWithPrefixExtracted(string* formatted_number);
 
+  const string& GetExtractedNationalPrefix() const;
+
   // Some national prefixes are a substring of others. If extracting the
   // shorter NDD doesn't result in a number we can format, we try to see if we
   // can extract a longer version here.
@@ -222,7 +224,7 @@ class AsYouTypeFormatter {
   bool should_add_space_after_national_prefix_;
   // This contains the national prefix that has been extracted. It contains only
   // digits without formatting.
-  string national_prefix_extracted_;
+  string extracted_national_prefix_;
   string national_number_;
 
   list<const NumberFormat*> possible_formats_;

cpp/test/phonenumbers/asyoutypeformatter_test.cc

@@ -43,6 +43,10 @@ class AsYouTypeFormatterTest : public testing::Test {
     return formatter_->current_metadata_;
   }
 
+  const string& GetExtractedNationalPrefix() const {
+    return formatter_->GetExtractedNationalPrefix();
+  }
+
   int ConvertUnicodeStringPosition(const UnicodeString& s, int pos) const {
     return AsYouTypeFormatter::ConvertUnicodeStringPosition(s, pos);
   }
@@ -1172,6 +1176,41 @@ TEST_F(AsYouTypeFormatterTest, AYTF_ShortNumberFormattingFix_US) {
   EXPECT_EQ("1 22", formatter_->InputDigit('2', &result_));
 }
 
+TEST_F(AsYouTypeFormatterTest, AYTF_ClearNDDAfterIDDExtraction) {
+  formatter_.reset(phone_util_.GetAsYouTypeFormatter(RegionCode::KR()));
+
+  // Check that when we have successfully extracted an IDD, the previously
+  // extracted NDD is cleared since it is no longer valid.
+  EXPECT_EQ("0", formatter_->InputDigit('0', &result_));
+  EXPECT_EQ("00", formatter_->InputDigit('0', &result_));
+  EXPECT_EQ("007", formatter_->InputDigit('7', &result_));
+  EXPECT_EQ("0070", formatter_->InputDigit('0', &result_));
+  EXPECT_EQ("00700", formatter_->InputDigit('0', &result_));
+  EXPECT_EQ("0", GetExtractedNationalPrefix());
+
+  // Once the IDD "00700" has been extracted, it no longer makes sense for the
+  // initial "0" to be treated as an NDD.
+  EXPECT_EQ("00700 1 ", formatter_->InputDigit('1', &result_));
+  EXPECT_EQ("", GetExtractedNationalPrefix());
+
+  EXPECT_EQ("00700 1 2", formatter_->InputDigit('2', &result_));
+  EXPECT_EQ("00700 1 23", formatter_->InputDigit('3', &result_));
+  EXPECT_EQ("00700 1 234", formatter_->InputDigit('4', &result_));
+  EXPECT_EQ("00700 1 234 5", formatter_->InputDigit('5', &result_));
+  EXPECT_EQ("00700 1 234 56", formatter_->InputDigit('6', &result_));
+  EXPECT_EQ("00700 1 234 567", formatter_->InputDigit('7', &result_));
+  EXPECT_EQ("00700 1 234 567 8", formatter_->InputDigit('8', &result_));
+  EXPECT_EQ("00700 1 234 567 89", formatter_->InputDigit('9', &result_));
+  EXPECT_EQ("00700 1 234 567 890", formatter_->InputDigit('0', &result_));
+  EXPECT_EQ("00700 1 234 567 8901", formatter_->InputDigit('1', &result_));
+  EXPECT_EQ("00700123456789012", formatter_->InputDigit('2', &result_));
+  EXPECT_EQ("007001234567890123", formatter_->InputDigit('3', &result_));
+  EXPECT_EQ("0070012345678901234", formatter_->InputDigit('4', &result_));
+  EXPECT_EQ("00700123456789012345", formatter_->InputDigit('5', &result_));
+  EXPECT_EQ("007001234567890123456", formatter_->InputDigit('6', &result_));
+  EXPECT_EQ("0070012345678901234567", formatter_->InputDigit('7', &result_));
+}
+
 TEST_F(AsYouTypeFormatterTest,
        NumberPatternsBecomingInvalidShouldNotResultInDigitLoss) {
   formatter_.reset(phone_util_.GetAsYouTypeFormatter(RegionCode::CN()));

java/libphonenumber/src/com/google/i18n/phonenumbers/AsYouTypeFormatter.java

@@ -181,7 +181,8 @@ public class AsYouTypeFormatter {
     for (NumberFormat format : formatList) {
       if (!nationalPrefixIsUsedByCountry || isCompleteNumber ||
           format.isNationalPrefixOptionalWhenFormatting() ||
-          phoneUtil.formattingRuleHasFirstGroupOnly(format.getNationalPrefixFormattingRule())) {
+          PhoneNumberUtil.formattingRuleHasFirstGroupOnly(
+              format.getNationalPrefixFormattingRule())) {
         if (isFormatEligible(format.getFormat())) {
           possibleFormats.add(format);
         }

java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java

@@ -539,8 +539,8 @@ public class PhoneNumberUtil {
   private final Map<Integer, List<String>> countryCallingCodeToRegionCodeMap;
 
   // The set of regions that share country calling code 1.
-  // There are roughly 26 regions and we set the initial capacity of the HashSet to 35 to offer a
-  // load factor of roughly 0.75.
+  // There are roughly 26 regions.
+  // We set the initial capacity of the HashSet to 35 to offer a load factor of roughly 0.75.
   private final Set<String> nanpaRegions = new HashSet<String>(35);
 
   // A mapping from a region code to the PhoneMetadata for that region.
@@ -577,7 +577,7 @@ public class PhoneNumberUtil {
   private final MetadataLoader metadataLoader;
 
   /**
-   * This class implements a singleton, so the only constructor is private.
+   * This class implements a singleton, the constructor is only visible to facilitate testing.
    */
   // @VisibleForTesting
   PhoneNumberUtil(String filePrefix, MetadataLoader metadataLoader,
@@ -803,11 +803,11 @@ public class PhoneNumberUtil {
   }
 
   /**
-   * Gets the length of the geographical area code from the {@code nationalNumber_} field of the
-   * PhoneNumber object passed in, so that clients could use it to split a national significant
-   * number into geographical area code and subscriber number. It works in such a way that the
-   * resultant subscriber number should be diallable, at least on some devices. An example of how
-   * this could be used:
+   * Gets the length of the geographical area code from the
+   * PhoneNumber object passed in, so that clients could use it
+   * to split a national significant number into geographical area code and subscriber number. It
+   * works in such a way that the resultant subscriber number should be diallable, at least on some
+   * devices. An example of how this could be used:
    *
    * <pre>
    * PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance();
@@ -838,9 +838,10 @@ public class PhoneNumberUtil {
    *    entities
    *  <li> some geographical numbers have no area codes.
    * </ul>
-   * @param number  the PhoneNumber object for which clients want to know the length of the area
-   *     code.
-   * @return  the length of area code of the PhoneNumber object passed in.
+   * @param number  the PhoneNumber object for which clients
+   *     want to know the length of the area code.
+   * @return  the length of area code of the PhoneNumber object
+   *     passed in.
    */
   public int getLengthOfGeographicalAreaCode(PhoneNumber number) {
     PhoneMetadata metadata = getMetadataForRegion(getRegionCodeForNumber(number));
@@ -861,11 +862,12 @@ public class PhoneNumberUtil {
   }
 
   /**
-   * Gets the length of the national destination code (NDC) from the PhoneNumber object passed in,
-   * so that clients could use it to split a national significant number into NDC and subscriber
-   * number. The NDC of a phone number is normally the first group of digit(s) right after the
-   * country calling code when the number is formatted in the international format, if there is a
-   * subscriber number part that follows. An example of how this could be used:
+   * Gets the length of the national destination code (NDC) from the
+   * PhoneNumber object passed in, so that clients could use it
+   * to split a national significant number into NDC and subscriber number. The NDC of a phone
+   * number is normally the first group of digit(s) right after the country calling code when the
+   * number is formatted in the international format, if there is a subscriber number part that
+   * follows. An example of how this could be used:
    *
    * <pre>
    * PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance();
@@ -888,8 +890,10 @@ public class PhoneNumberUtil {
    * Refer to the unittests to see the difference between this function and
    * {@link #getLengthOfGeographicalAreaCode}.
    *
-   * @param number  the PhoneNumber object for which clients want to know the length of the NDC.
-   * @return  the length of NDC of the PhoneNumber object passed in.
+   * @param number  the PhoneNumber object for which clients
+   *     want to know the length of the NDC.
+   * @return  the length of NDC of the PhoneNumber object
+   *     passed in.
    */
   public int getLengthOfNationalDestinationCode(PhoneNumber number) {
     PhoneNumber copiedProto;
@@ -1033,7 +1037,7 @@ public class PhoneNumberUtil {
     return new PhoneNumberUtil(META_DATA_FILE_PREFIX, metadataLoader,
         CountryCodeToRegionCodeMap.getCountryCodeToRegionCodeMap());
   }
-  
+
   /**
    * Helper function to check if the national prefix formatting rule has the first group only, i.e.,
    * does not start with the national prefix.
@@ -1047,6 +1051,11 @@ public class PhoneNumberUtil {
    * Tests whether a phone number has a geographical association. It checks if the number is
    * associated to a certain region in the country where it belongs to. Note that this doesn't
    * verify if the number is actually in use.
+   *
+   * A similar method is implemented as PhoneNumberOfflineGeocoder.canBeGeocoded, which performs a
+   * looser check, since it only prevents cases where prefixes overlap for geocodable and
+   * non-geocodable numbers. Also, if new phone number types were added, we should check if this
+   * other method should be updated too.
    */
   boolean isNumberGeographical(PhoneNumber phoneNumber) {
     PhoneNumberType numberType = getNumberType(phoneNumber);
@@ -1111,6 +1120,7 @@ public class PhoneNumberUtil {
     formattedNumber.setLength(0);
     int countryCallingCode = number.getCountryCode();
     String nationalSignificantNumber = getNationalSignificantNumber(number);
+
     if (numberFormat == PhoneNumberFormat.E164) {
       // Early exit for E164 case (even if the country calling code is invalid) since no formatting
       // of the national number needs to be applied. Extensions are not formatted.
@@ -1337,7 +1347,8 @@ public class PhoneNumberUtil {
             // CL fixed line numbers need the national prefix when dialing in the national format,
             // but don't have it when used for display. The reverse is true for mobile numbers.
             // As a result, we output them in the international format to make it work.
-            ((regionCode.equals("MX") || regionCode.equals("CL")) && isFixedLineOrMobile)) &&
+            ((regionCode.equals("MX") || regionCode.equals("CL")) &&
+             isFixedLineOrMobile)) &&
             canBeInternationallyDialled(numberNoExt)) {
           formattedNumber = format(numberNoExt, PhoneNumberFormat.INTERNATIONAL);
         } else {
@@ -1508,7 +1519,8 @@ public class PhoneNumberUtil {
         }
         // When the format we apply to this number doesn't contain national prefix, we can just
         // return the national format.
-        // TODO: Refactor the code below with the code in isNationalPrefixPresentIfRequired.
+        // TODO: Refactor the code below with the code in
+        // isNationalPrefixPresentIfRequired.
         String candidateNationalPrefixRule = formatRule.getNationalPrefixFormattingRule();
         // We assume that the first-group symbol will never be _before_ the national prefix.
         int indexOfFirstGroup = candidateNationalPrefixRule.indexOf("$1");
@@ -2731,7 +2743,7 @@ public class PhoneNumberUtil {
   private boolean checkRegionForParsing(String numberToParse, String defaultRegion) {
     if (!isValidRegionCode(defaultRegion)) {
       // If the number is null or empty, we can't infer the region.
-      if (numberToParse == null || numberToParse.length() == 0 ||
+      if ((numberToParse == null) || (numberToParse.length() == 0) ||
           !PLUS_CHARS_PATTERN.matcher(numberToParse).lookingAt()) {
         return false;
       }
@@ -3204,7 +3216,7 @@ public class PhoneNumberUtil {
   /**
    * Returns true if the number can be dialled from outside the region, or unknown. If the number
    * can only be dialled from within the region, returns false. Does not check the number is a valid
-   * number.
+   * number. Note that, at the moment, this method does not handle short numbers.
    * TODO: Make this method public when we have enough metadata to make it worthwhile.
    *
    * @param number  the phone-number for which we want to know whether it is diallable from