From 0bd133294b9e291cc6090515e9a6aa83ea35d3b9 Mon Sep 17 00:00:00 2001 From: Tijana Vislavski Gradina Date: Tue, 24 Dec 2024 10:05:13 +0000 Subject: [PATCH] Documented the issue with checking isPossible only for default country (#3771) * Added Automatic-Module-Name to published jars. Context: https://b.corp.google.com/issues/308334884 * Removed Automatic-Module-Name from carrier and geocoder. Classes in these modules belong to the same package as core libphonenumber library, which leads to the split package problem with Java modules. * Documented the issue with checking isPossible only for default country --- cpp/src/phonenumbers/phonenumberutil.h | 14 ++++ .../com/google/phonenumbers/demo/result.soy | 40 +++++----- .../i18n/phonenumbers/PhoneNumberUtil.java | 13 ++++ javascript/i18n/phonenumbers/demo.js | 76 +++++++++---------- .../i18n/phonenumbers/phonenumberutil.js | 12 +++ 5 files changed, 95 insertions(+), 60 deletions(-) diff --git a/cpp/src/phonenumbers/phonenumberutil.h b/cpp/src/phonenumbers/phonenumberutil.h index 14cfc670c7..d22f997a80 100644 --- a/cpp/src/phonenumbers/phonenumberutil.h +++ b/cpp/src/phonenumbers/phonenumberutil.h @@ -531,6 +531,13 @@ class PhoneNumberUtil : public Singleton { // would most likely be area codes) and length (obviously includes the // length of area codes for fixed line numbers), it will return false for // the subscriber-number-only version. + // + // There is a known issue with this method: if a number is possible only in a + // certain region among several regions that share the same country calling + // code, this method will consider only the "main" region. For example, + // +1310xxxx are valid numbers in Canada. However, they are not possible in + // the US. As a result, this method will return false for +1310xxxx. See + // https://issuetracker.google.com/issues/335892662 for more details. ValidationResult IsPossibleNumberWithReason(const PhoneNumber& number) const; // Convenience wrapper around IsPossibleNumberWithReason(). Instead of @@ -565,6 +572,13 @@ class PhoneNumberUtil : public Singleton { // would most likely be area codes) and length (obviously includes the // length of area codes for fixed line numbers), it will return false for // the subscriber-number-only version. + // + // There is a known issue with this method: if a number is possible only in a + // certain region among several regions that share the same country calling + // code, this method will consider only the "main" region. For example, + // +1310xxxx are valid numbers in Canada. However, they are not possible in + // the US. As a result, this method will return false for +1310xxxx. See + // https://issuetracker.google.com/issues/335892662 for more details. ValidationResult IsPossibleNumberForTypeWithReason( const PhoneNumber& number, PhoneNumberType type) const; diff --git a/java/demo/src/main/resources/com/google/phonenumbers/demo/result.soy b/java/demo/src/main/resources/com/google/phonenumbers/demo/result.soy index e3107109e8..baaa9d4181 100644 --- a/java/demo/src/main/resources/com/google/phonenumbers/demo/result.soy +++ b/java/demo/src/main/resources/com/google/phonenumbers/demo/result.soy @@ -101,44 +101,42 @@ Result from isPossibleNumber() {$isPossibleNumber} -{if $isPossibleNumber} - {if $validationResult == "IS_POSSIBLE_LOCAL_ONLY"} Result from isPossibleNumberWithReason() {$validationResult} - - Number is considered invalid as it is not a possible national number. - - {else} Result from isValidNumber() {$isValidNumber} + {if $isValidNumber} + {if $validationResult != "IS_POSSIBLE"} + + + Warning: This number represents a known + edge case - it is a valid number, but it is not considered (strictly) possible + + + {/if} {if $isValidNumberForRegion != null} - - Result from isValidNumberForRegion() - {$isValidNumberForRegion} - + + Result from isValidNumberForRegion() + {$isValidNumberForRegion} + {/if} Phone Number region - {$phoneNumberRegion ?: ""} + {$phoneNumberRegion} Result from getNumberType() {$numberType} - {/if} -{else} - - Result from isPossibleNumberWithReason() - {$validationResult} - - - Note: Numbers that are not possible have type UNKNOWN, an unknown region, and are considered invalid. - -{/if} + {else} + + Note: Invalid numbers have type UNKNOWN and no region. + + {/if} diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index b37bc48c36..6d8004f182 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -2708,6 +2708,13 @@ private ValidationResult testNumberLength( * length (obviously includes the length of area codes for fixed line numbers), it will * return false for the subscriber-number-only version. * + * + *

There is a known issue with this + * method: if a number is possible only in a certain region among several regions that share the + * same country calling code, this method will consider only the "main" region. For example, + * +1310xxxx are valid numbers in Canada. However, they are not possible in the US. As a result, + * this method will return IS_POSSIBLE_LOCAL_ONLY for +1310xxxx. + * * @param number the number that needs to be checked * @return a ValidationResult object which indicates whether the number is possible */ @@ -2737,6 +2744,12 @@ public ValidationResult isPossibleNumberWithReason(PhoneNumber number) { * return false for the subscriber-number-only version. * * + *

There is a known issue with this + * method: if a number is possible only in a certain region among several regions that share the + * same country calling code, this method will consider only the "main" region. For example, + * +1310xxxx are valid numbers in Canada. However, they are not possible in the US. As a result, + * this method will return IS_POSSIBLE_LOCAL_ONLY for +1310xxxx. + * * @param number the number that needs to be checked * @param type the type we are interested in * @return a ValidationResult object which indicates whether the number is possible diff --git a/javascript/i18n/phonenumbers/demo.js b/javascript/i18n/phonenumbers/demo.js index e18d351ad1..04c1871df0 100644 --- a/javascript/i18n/phonenumbers/demo.js +++ b/javascript/i18n/phonenumbers/demo.js @@ -58,52 +58,50 @@ function phoneNumberParser() { output.append('\nResult from isPossibleNumber(): '); output.append(isPossible); var validationResult = i18n.phonenumbers.PhoneNumberUtil.ValidationResult; - var isPossibleReason = phoneUtil_.isPossibleNumberWithReason(number) - var hasRegionCode = regionCode && regionCode != 'ZZ'; - if (isPossible) { - // Checking as isValid() fails if possible local only. - if (isPossibleReason == validationResult.IS_POSSIBLE_LOCAL_ONLY) { - output.append('\nResult from isPossibleNumberWithReason(): '); + var isPossibleReason = phoneUtil_.isPossibleNumberWithReason(number); + output.append('\nResult from isPossibleNumberWithReason(): '); + switch (isPossibleReason) { + case validationResult.IS_POSSIBLE: + output.append('IS_POSSIBLE'); + break; + case validationResult.IS_POSSIBLE_LOCAL_ONLY: output.append('IS_POSSIBLE_LOCAL_ONLY'); + break; + case validationResult.INVALID_COUNTRY_CODE: + output.append('INVALID_COUNTRY_CODE'); + break; + case validationResult.TOO_SHORT: + output.append('TOO_SHORT'); + break; + case validationResult.TOO_LONG: + output.append('TOO_LONG'); + break; + case validationResult.INVALID_LENGTH: + output.append('INVALID_LENGTH'); + break; + } + var hasRegionCode = regionCode && regionCode != 'ZZ'; + var isNumberValid = phoneUtil_.isValidNumber(number); + output.append('\nResult from isValidNumber(): '); + output.append(isNumberValid); + if (isNumberValid) { + if (isPossibleReason != validationResult.IS_POSSIBLE) { output.append( - '\nNumber is considered invalid as it is ' + - 'not a possible national number.'); - } else { - var isNumberValid = phoneUtil_.isValidNumber(number); - output.append('\nResult from isValidNumber(): '); - output.append(isNumberValid); - if (isNumberValid && hasRegionCode) { + '\nWarning: This number represents a known edge case - it is ' + + 'a valid number, but it is not considered (strictly) possible. ' + + 'See https://issuetracker.google.com/issues/335892662 for more details.'); + } + if (hasRegionCode) { output.append('\nResult from isValidNumberForRegion(): '); output.append(phoneUtil_.isValidNumberForRegion(number, regionCode)); - } - output.append('\nPhone Number region: '); - output.append(phoneUtil_.getRegionCodeForNumber(number)); - output.append('\nResult from getNumberType(): '); - output.append(getNumberTypeString(number)); } + output.append('\nPhone Number region: '); + output.append(phoneUtil_.getRegionCodeForNumber(number)); + output.append('\nResult from getNumberType(): '); + output.append(getNumberTypeString(number)); } else { - output.append('\nResult from isPossibleNumberWithReason(): '); - switch (isPossibleReason) { - case validationResult.INVALID_COUNTRY_CODE: - output.append('INVALID_COUNTRY_CODE'); - break; - case validationResult.TOO_SHORT: - output.append('TOO_SHORT'); - break; - case validationResult.TOO_LONG: - output.append('TOO_LONG'); - break; - case validationResult.INVALID_LENGTH: - output.append('INVALID_LENGTH'); - break; - } - // IS_POSSIBLE shouldn't happen, since we only call this if _not_ - // possible. output.append( - '\nNote: Numbers that are not possible have type UNKNOWN,' + - ' an unknown region, and are considered invalid.'); - } - if (!isNumberValid) { + '\nNote: Invalid numbers have type UNKNOWN and no region.'); var shortInfo = i18n.phonenumbers.ShortNumberInfo.getInstance(); output.append('\n\n****ShortNumberInfo Results:****'); output.append('\nResult from isPossibleShortNumber: '); diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index c64cd1f922..8812d3aa4e 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -3584,6 +3584,12 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.testNumberLengthForType_ = * numbers), it will return false for the subscriber-number-only version. * * + *

There is a known issue with this + * method: if a number is possible only in a certain region among several regions that share the + * same country calling code, this method will consider only the "main" region. For example, + * +1310xxxx are valid numbers in Canada. However, they are not possible in the US. As a result, + * this method will return IS_POSSIBLE_LOCAL_ONLY for +1310xxxx. + * * @param {i18n.phonenumbers.PhoneNumber} number the number that needs to be * checked * @return {i18n.phonenumbers.PhoneNumberUtil.ValidationResult} a @@ -3613,6 +3619,12 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.isPossibleNumberWithReason = * numbers), it will return false for the subscriber-number-only version. * * + *

There is a known issue with this + * method: if a number is possible only in a certain region among several regions that share the + * same country calling code, this method will consider only the "main" region. For example, + * +1310xxxx are valid numbers in Canada. However, they are not possible in the US. As a result, + * this method will return IS_POSSIBLE_LOCAL_ONLY for +1310xxxx. + * * @param {i18n.phonenumbers.PhoneNumber} number the number that needs to be * checked * @param {i18n.phonenumbers.PhoneNumberType} type the type we are interested in