diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index 791216118..31163acf5 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -991,7 +991,11 @@ void PhoneNumberUtil::FormatNationalNumberWithPreferredCarrierCode( string* formatted_number) const { FormatNationalNumberWithCarrierCode( number, - number.has_preferred_domestic_carrier_code() + // Historically, we set this to an empty string when parsing with raw + // input if none was found in the input string. However, this doesn't + // result in a number we can dial. For this reason, we treat the empty + // string the same as if it isn't set at all. + !number.preferred_domestic_carrier_code().empty() ? number.preferred_domestic_carrier_code() : fallback_carrier_code, formatted_number); @@ -1027,9 +1031,13 @@ void PhoneNumberUtil::FormatNumberForMobileDialing( number_no_extension, kColombiaMobileToFixedLinePrefix, formatted_number); } else if ((region_code == "BR") && (is_fixed_line_or_mobile)) { - if (number_no_extension.has_preferred_domestic_carrier_code()) { - FormatNationalNumberWithPreferredCarrierCode(number_no_extension, "", - formatted_number); + // Historically, we set this to an empty string when parsing with raw + // input if none was found in the input string. However, this doesn't + // result in a number we can dial. For this reason, we treat the empty + // string the same as if it isn't set at all. + if (!number_no_extension.preferred_domestic_carrier_code().empty()) { + FormatNationalNumberWithPreferredCarrierCode(number_no_extension, "", + formatted_number); } else { // Brazilian fixed line and mobile numbers need to be dialed with a // carrier code when called within Brazil. Without that, most of the @@ -2017,7 +2025,7 @@ PhoneNumberUtil::ErrorType PhoneNumberUtil::ParseHelper( if (TestNumberLength(potential_national_number, country_metadata->general_desc()) != TOO_SHORT) { normalized_national_number.assign(potential_national_number); - if (keep_raw_input) { + if (keep_raw_input && !carrier_code.empty()) { temp_number.set_preferred_domestic_carrier_code(carrier_code); } } diff --git a/cpp/test/phonenumbers/phonenumberutil_test.cc b/cpp/test/phonenumbers/phonenumberutil_test.cc index d82f54fe8..e8b15b390 100644 --- a/cpp/test/phonenumbers/phonenumberutil_test.cc +++ b/cpp/test/phonenumbers/phonenumberutil_test.cc @@ -999,12 +999,18 @@ TEST_F(PhoneNumberUtilTest, FormatWithPreferredCarrierCode) { phone_util_.FormatNationalNumberWithPreferredCarrierCode(ar_number, "", &formatted_number); EXPECT_EQ("01234 19 12-5678", formatted_number); - // When the preferred_domestic_carrier_code is present (even when it contains - // an empty string), use it instead of the default carrier code passed in. + // When the preferred_domestic_carrier_code is present (even when it is just a + // space), use it instead of the default carrier code passed in. + ar_number.set_preferred_domestic_carrier_code(" "); + phone_util_.FormatNationalNumberWithPreferredCarrierCode(ar_number, "15", + &formatted_number); + EXPECT_EQ("01234 12-5678", formatted_number); + // When the preferred_domestic_carrier_code is present but empty, treat is as + // unset and use instead the default carrier code passed in. ar_number.set_preferred_domestic_carrier_code(""); phone_util_.FormatNationalNumberWithPreferredCarrierCode(ar_number, "15", &formatted_number); - EXPECT_EQ("01234 12-5678", formatted_number); + EXPECT_EQ("01234 15 12-5678", formatted_number); // We don't support this for the US so there should be no change. PhoneNumber us_number; us_number.set_country_code(1); @@ -3620,9 +3626,6 @@ TEST_F(PhoneNumberUtilTest, ParseNumbersWithPlusWithNoRegion) { nz_number.set_raw_input("+64 3 331 6005"); nz_number.set_country_code_source(PhoneNumber::FROM_NUMBER_WITH_PLUS_SIGN); - // It is important that we set this to an empty string, since we used - // ParseAndKeepRawInput and no carrrier code was found. - nz_number.set_preferred_domestic_carrier_code(""); result_proto.Clear(); EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR, phone_util_.ParseAndKeepRawInput("+64 3 331 6005", @@ -3863,7 +3866,6 @@ TEST_F(PhoneNumberUtilTest, ParseAndKeepRaw) { alpha_numeric_number.set_raw_input("800 six-flags"); alpha_numeric_number.set_country_code_source( PhoneNumber::FROM_DEFAULT_COUNTRY); - alpha_numeric_number.set_preferred_domestic_carrier_code(""); PhoneNumber test_number; EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR, diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index 0b25dbf6b..d4e83018a 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -1247,9 +1247,13 @@ public class PhoneNumberUtil { */ public String formatNationalNumberWithPreferredCarrierCode(PhoneNumber number, String fallbackCarrierCode) { - return formatNationalNumberWithCarrierCode(number, number.hasPreferredDomesticCarrierCode() - ? number.getPreferredDomesticCarrierCode() - : fallbackCarrierCode); + return formatNationalNumberWithCarrierCode(number, + // Historically, we set this to an empty string when parsing with raw input if none was + // found in the input string. However, this doesn't result in a number we can dial. For this + // reason, we treat the empty string the same as if it isn't set at all. + number.getPreferredDomesticCarrierCode().length() > 0 + ? number.getPreferredDomesticCarrierCode() + : fallbackCarrierCode); } /** @@ -1286,7 +1290,10 @@ public class PhoneNumberUtil { formattedNumber = formatNationalNumberWithCarrierCode(numberNoExt, COLOMBIA_MOBILE_TO_FIXED_LINE_PREFIX); } else if (regionCode.equals("BR") && isFixedLineOrMobile) { - formattedNumber = numberNoExt.hasPreferredDomesticCarrierCode() + // Historically, we set this to an empty string when parsing with raw input if none was + // found in the input string. However, this doesn't result in a number we can dial. For this + // reason, we treat the empty string the same as if it isn't set at all. + formattedNumber = numberNoExt.getPreferredDomesticCarrierCode().length() > 0 ? formattedNumber = formatNationalNumberWithPreferredCarrierCode(numberNoExt, "") // Brazilian fixed line and mobile numbers need to be dialed with a carrier code when // called within Brazil. Without that, most of the carriers won't connect the call. @@ -3014,7 +3021,7 @@ public class PhoneNumberUtil { if (testNumberLength(potentialNationalNumber.toString(), regionMetadata.getGeneralDesc()) != ValidationResult.TOO_SHORT) { normalizedNationalNumber = potentialNationalNumber; - if (keepRawInput) { + if (keepRawInput && carrierCode.length() > 0) { phoneNumber.setPreferredDomesticCarrierCode(carrierCode.toString()); } } diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java index 274f874c4..ce3c28c1a 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java @@ -731,10 +731,15 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, "15")); assertEquals("01234 19 12-5678", phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, "")); - // When the preferred_domestic_carrier_code is present (even when it contains an empty string), - // use it instead of the default carrier code passed in. + // When the preferred_domestic_carrier_code is present (even when it is just a space), use it + // instead of the default carrier code passed in. + arNumber.setPreferredDomesticCarrierCode(" "); + assertEquals("01234 12-5678", + phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, "15")); + // When the preferred_domestic_carrier_code is present but empty, treat it as unset and use + // instead the default carrier code passed in. arNumber.setPreferredDomesticCarrierCode(""); - assertEquals("01234 12-5678", + assertEquals("01234 15 12-5678", phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, "15")); // We don't support this for the US so there should be no change. PhoneNumber usNumber = new PhoneNumber(); @@ -2192,12 +2197,9 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { assertEquals(NZ_NUMBER, phoneUtil.parse("tel:03-331-6005;isub=12345;phone-context=+64", RegionCode.ZZ)); - // It is important that we set the carrier code to an empty string, since we used - // ParseAndKeepRawInput and no carrier code was found. PhoneNumber nzNumberWithRawInput = new PhoneNumber().mergeFrom(NZ_NUMBER). setRawInput("+64 3 331 6005"). - setCountryCodeSource(CountryCodeSource.FROM_NUMBER_WITH_PLUS_SIGN). - setPreferredDomesticCarrierCode(""); + setCountryCodeSource(CountryCodeSource.FROM_NUMBER_WITH_PLUS_SIGN); assertEquals(nzNumberWithRawInput, phoneUtil.parseAndKeepRawInput("+64 3 331 6005", RegionCode.ZZ)); // Null is also allowed for the region code in these cases. @@ -2299,16 +2301,14 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { public void testParseAndKeepRaw() throws Exception { PhoneNumber alphaNumericNumber = new PhoneNumber().mergeFrom(ALPHA_NUMERIC_NUMBER). setRawInput("800 six-flags"). - setCountryCodeSource(CountryCodeSource.FROM_DEFAULT_COUNTRY). - setPreferredDomesticCarrierCode(""); + setCountryCodeSource(CountryCodeSource.FROM_DEFAULT_COUNTRY); assertEquals(alphaNumericNumber, phoneUtil.parseAndKeepRawInput("800 six-flags", RegionCode.US)); PhoneNumber shorterAlphaNumber = new PhoneNumber(). setCountryCode(1).setNationalNumber(8007493524L). setRawInput("1800 six-flag"). - setCountryCodeSource(CountryCodeSource.FROM_NUMBER_WITHOUT_PLUS_SIGN). - setPreferredDomesticCarrierCode(""); + setCountryCodeSource(CountryCodeSource.FROM_NUMBER_WITHOUT_PLUS_SIGN); assertEquals(shorterAlphaNumber, phoneUtil.parseAndKeepRawInput("1800 six-flag", RegionCode.US)); diff --git a/java/pending_code_changes.txt b/java/pending_code_changes.txt index 8b1378917..cf67526a8 100644 --- a/java/pending_code_changes.txt +++ b/java/pending_code_changes.txt @@ -1 +1,3 @@ - +Code changes: + - Stop setting empty preferred_domestic_carrier_code, and if we are passed such + a number then treat the empty field as if unset. diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index 0d471dc0a..7f2ac35f1 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -1682,7 +1682,11 @@ i18n.phonenumbers.PhoneNumberUtil.prototype. number, fallbackCarrierCode) { return this.formatNationalNumberWithCarrierCode( number, - number.hasPreferredDomesticCarrierCode() ? + // Historically, we set this to an empty string when parsing with raw + // input if none was found in the input string. However, this doesn't + // result in a number we can dial. For this reason, we treat the empty + // string the same as if it isn't set at all. + number.getPreferredDomesticCarrierCodeOrDefault().length > 0 ? number.getPreferredDomesticCarrierCodeOrDefault() : fallbackCarrierCode); }; @@ -1737,7 +1741,12 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.formatNumberForMobileDialing = i18n.phonenumbers.PhoneNumberUtil .COLOMBIA_MOBILE_TO_FIXED_LINE_PREFIX_); } else if (regionCode == 'BR' && isFixedLineOrMobile) { - formattedNumber = numberNoExt.hasPreferredDomesticCarrierCode() ? + formattedNumber = + // Historically, we set this to an empty string when parsing with raw + // input if none was found in the input string. However, this doesn't + // result in a number we can dial. For this reason, we treat the empty + // string the same as if it isn't set at all. + numberNoExt.getPreferredDomesticCarrierCodeOrDefault().length > 0 ? this.formatNationalNumberWithPreferredCarrierCode(numberNoExt, '') : // Brazilian fixed line and mobile numbers need to be dialed with a // carrier code when called within Brazil. Without that, most of the @@ -3905,7 +3914,7 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.parseHelper_ = regionMetadata.getGeneralDesc()) != i18n.phonenumbers.PhoneNumberUtil.ValidationResult.TOO_SHORT) { normalizedNationalNumber = potentialNationalNumber; - if (keepRawInput) { + if (keepRawInput && carrierCode.toString().length > 0) { phoneNumber.setPreferredDomesticCarrierCode(carrierCode.toString()); } } diff --git a/javascript/i18n/phonenumbers/phonenumberutil_test.js b/javascript/i18n/phonenumbers/phonenumberutil_test.js index 119330645..059a73fda 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil_test.js +++ b/javascript/i18n/phonenumbers/phonenumberutil_test.js @@ -974,10 +974,15 @@ function testFormatWithPreferredCarrierCode() { phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, '15')); assertEquals('01234 19 12-5678', phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, '')); - // When the preferred_domestic_carrier_code is present (even when it contains - // an empty string), use it instead of the default carrier code passed in. + // When the preferred_domestic_carrier_code is present (even when it is just a + // space), use it instead of the default carrier code passed in. + arNumber.setPreferredDomesticCarrierCode(' '); + assertEquals("01234 12-5678", + phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, '15')); + // When the preferred_domestic_carrier_code is present but empty, treat it as + // unset and use instead the default carrier code passed in. arNumber.setPreferredDomesticCarrierCode(''); - assertEquals('01234 12-5678', + assertEquals('01234 15 12-5678', phoneUtil.formatNationalNumberWithPreferredCarrierCode(arNumber, '15')); // We don't support this for the US so there should be no change. /** @type {i18n.phonenumbers.PhoneNumber} */ @@ -2819,14 +2824,11 @@ function testParseNumbersWithPlusWithNoRegion() { phoneUtil.parse('tel:03-331-6005;isub=12345;phone-context=+64', RegionCode.ZZ))); - // It is important that we set the carrier code to an empty string, since we - // used ParseAndKeepRawInput and no carrier code was found. /** @type {i18n.phonenumbers.PhoneNumber} */ var nzNumberWithRawInput = NZ_NUMBER.clone(); nzNumberWithRawInput.setRawInput('+64 3 331 6005'); nzNumberWithRawInput.setCountryCodeSource(i18n.phonenumbers.PhoneNumber .CountryCodeSource.FROM_NUMBER_WITH_PLUS_SIGN); - nzNumberWithRawInput.setPreferredDomesticCarrierCode(''); assertTrue(nzNumberWithRawInput.equals( phoneUtil.parseAndKeepRawInput('+64 3 331 6005', RegionCode.ZZ))); // Null is also allowed for the region code in these cases. @@ -2984,7 +2986,6 @@ function testParseAndKeepRaw() { var alphaNumericNumber = ALPHA_NUMERIC_NUMBER.clone(); alphaNumericNumber.setRawInput('800 six-flags'); alphaNumericNumber.setCountryCodeSource(CCS.FROM_DEFAULT_COUNTRY); - alphaNumericNumber.setPreferredDomesticCarrierCode(''); assertTrue(alphaNumericNumber.equals( phoneUtil.parseAndKeepRawInput('800 six-flags', RegionCode.US))); @@ -2994,7 +2995,6 @@ function testParseAndKeepRaw() { shorterAlphaNumber.setNationalNumber(8007493524); shorterAlphaNumber.setRawInput('1800 six-flag'); shorterAlphaNumber.setCountryCodeSource(CCS.FROM_NUMBER_WITHOUT_PLUS_SIGN); - shorterAlphaNumber.setPreferredDomesticCarrierCode(''); assertTrue(shorterAlphaNumber.equals( phoneUtil.parseAndKeepRawInput('1800 six-flag', RegionCode.US))); diff --git a/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar b/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar index 2bd87584f..7f463a62f 100644 Binary files a/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar and b/tools/java/cpp-build/target/cpp-build-1.0-SNAPSHOT-jar-with-dependencies.jar differ diff --git a/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar b/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar index e07029ea5..510b47e1b 100644 Binary files a/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar and b/tools/java/java-build/target/java-build-1.0-SNAPSHOT-jar-with-dependencies.jar differ