Skip to content

Commit

Permalink
Documented the issue with checking isPossible only for default country (
Browse files Browse the repository at this point in the history 
#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
  • Loading branch information
@tvislavski
tvislavski authored Dec 24, 2024
1 parent bf08a3c commit 0bd1332
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 60 deletions.
14 changes: 14 additions & 0 deletions cpp/src/phonenumbers/phonenumberutil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,13 @@ class PhoneNumberUtil : public Singleton<PhoneNumberUtil> {
// 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
Expand Down Expand Up @@ -565,6 +572,13 @@ class PhoneNumberUtil : public Singleton<PhoneNumberUtil> {
// 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;

Expand Down
40 changes: 19 additions & 21 deletions java/demo/src/main/resources/com/google/phonenumbers/demo/result.soy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,44 +101,42 @@
<TH>Result from isPossibleNumber()</TH>
<TD>{$isPossibleNumber}</TD>
</TR>
{if $isPossibleNumber}
{if $validationResult == "IS_POSSIBLE_LOCAL_ONLY"}
<TR>
<TH>Result from isPossibleNumberWithReason()</TH>
<TD>{$validationResult}</TD>
</TR>
<TR>
<TD colspan=2>Number is considered invalid as it is not a possible national number.</TD>
</TR>
{else}
<TR>
<TH>Result from isValidNumber()</TH>
<TD>{$isValidNumber}</TD>
</TR>
{if $isValidNumber}
{if $validationResult != "IS_POSSIBLE"}
<TR>
<TD colspan=2 style="color:red">
Warning: This number represents a known <a href="https://issuetracker.google.com/issues/335892662">
edge case</a> - it is a valid number, but it is not considered (strictly) possible
</TD>
</TR>
{/if}
{if $isValidNumberForRegion != null}
<TR>
<TH>Result from isValidNumberForRegion()</TH>
<TD>{$isValidNumberForRegion}</TD>
</TR>
<TR>
<TH>Result from isValidNumberForRegion()</TH>
<TD>{$isValidNumberForRegion}</TD>
</TR>
{/if}
<TR>
<TH>Phone Number region</TH>
<TD>{$phoneNumberRegion ?: ""}</TD>
<TD>{$phoneNumberRegion}</TD>
</TR>
<TR>
<TH>Result from getNumberType()</TH>
<TD>{$numberType}</TD>
</TR>
{/if}
{else}
<TR>
<TH>Result from isPossibleNumberWithReason()</TH>
<TD>{$validationResult}</TD>
</TR>
<TR>
<TD colspan=2>Note: Numbers that are not possible have type UNKNOWN, an unknown region, and are considered invalid.</TD>
</TR>
{/if}
{else}
<TR>
<TD colspan=2>Note: Invalid numbers have type UNKNOWN and no region.</TD>
</TR>
{/if}
</TABLE>
</DIV>

Expand Down
13 changes: 13 additions & 0 deletions java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* </ol>
*
* <p>There is a known <a href="https://issuetracker.google.com/issues/335892662">issue</a> 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
*/
Expand Down Expand Up @@ -2737,6 +2744,12 @@ public ValidationResult isPossibleNumberWithReason(PhoneNumber number) {
* return false for the subscriber-number-only version.
* </ol>
*
* <p>There is a known <a href="https://issuetracker.google.com/issues/335892662">issue</a> 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
Expand Down
76 changes: 37 additions & 39 deletions javascript/i18n/phonenumbers/demo.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: ');
Expand Down
12 changes: 12 additions & 0 deletions javascript/i18n/phonenumbers/phonenumberutil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3584,6 +3584,12 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.testNumberLengthForType_ =
* numbers), it will return false for the subscriber-number-only version.
* </ol>
*
* <p>There is a known <a href="https://issuetracker.google.com/issues/335892662">issue</a> 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
Expand Down Expand Up @@ -3613,6 +3619,12 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.isPossibleNumberWithReason =
* numbers), it will return false for the subscriber-number-only version.
* </ol>
*
* <p>There is a known <a href="https://issuetracker.google.com/issues/335892662">issue</a> 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
Expand Down

0 comments on commit 0bd1332

Please sign in to comment.