diff --git a/java/carrier/src/com/google/i18n/phonenumbers/carrier/data/config b/java/carrier/src/com/google/i18n/phonenumbers/carrier/data/config index 4a4e6e441..99bb17154 100644 Binary files a/java/carrier/src/com/google/i18n/phonenumbers/carrier/data/config and b/java/carrier/src/com/google/i18n/phonenumbers/carrier/data/config differ diff --git a/java/geocoder/src/com/google/i18n/phonenumbers/geocoding/data/config b/java/geocoder/src/com/google/i18n/phonenumbers/geocoding/data/config index 7b3822ce1..38783fb39 100644 Binary files a/java/geocoder/src/com/google/i18n/phonenumbers/geocoding/data/config and b/java/geocoder/src/com/google/i18n/phonenumbers/geocoding/data/config differ diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index e1fe8d949..0b25dbf6b 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -1306,8 +1306,8 @@ public class PhoneNumberUtil { // short numbers, which are always dialled in national format. PhoneMetadata regionMetadata = getMetadataForRegion(regionCallingFrom); if (canBeInternationallyDialled(numberNoExt) - && !isShorterThanPossibleNormalNumber(regionMetadata, - getNationalSignificantNumber(numberNoExt))) { + && testNumberLength(getNationalSignificantNumber(numberNoExt), + regionMetadata.getGeneralDesc()) != ValidationResult.TOO_SHORT) { formattedNumber = format(numberNoExt, PhoneNumberFormat.INTERNATIONAL); } else { formattedNumber = format(numberNoExt, PhoneNumberFormat.NATIONAL); @@ -2113,19 +2113,19 @@ public class PhoneNumberUtil { return metadataSource.getMetadataForNonGeographicalRegion(countryCallingCode); } - boolean isNumberPossibleForDesc(String nationalNumber, PhoneNumberDesc numberDesc) { - Matcher possibleNumberPatternMatcher = - regexCache.getPatternForRegex(numberDesc.getPossibleNumberPattern()) - .matcher(nationalNumber); - return possibleNumberPatternMatcher.matches(); - } - boolean isNumberMatchingDesc(String nationalNumber, PhoneNumberDesc numberDesc) { + // Check if any possible number lengths are present; if so, we use them to avoid checking the + // validation pattern if they don't match. If they are absent, this means they match the general + // description, which we have already checked before checking a specific number type. + int actualLength = nationalNumber.length(); + List possibleLengths = numberDesc.getPossibleLengthList(); + if (possibleLengths.size() > 0 && !possibleLengths.contains(actualLength)) { + return false; + } Matcher nationalNumberPatternMatcher = regexCache.getPatternForRegex(numberDesc.getNationalNumberPattern()) .matcher(nationalNumber); - return isNumberPossibleForDesc(nationalNumber, numberDesc) - && nationalNumberPatternMatcher.matches(); + return nationalNumberPatternMatcher.matches(); } /** @@ -2362,32 +2362,35 @@ public class PhoneNumberUtil { } /** - * Helper method to check a number against a particular pattern and determine whether it matches, - * or is too short or too long. Currently, if a number pattern suggests that numbers of length 7 - * and 10 are possible, and a number in between these possible lengths is entered, such as of - * length 8, this will return TOO_LONG. + * Helper method to check a number against possible lengths for this number, and determine whether + * it matches, or is too short or too long. Currently, if a number pattern suggests that numbers + * of length 7 and 10 are possible, and a number in between these possible lengths is entered, + * such as of length 8, this will return TOO_LONG. */ - private ValidationResult testNumberLengthAgainstPattern(Pattern numberPattern, String number) { - Matcher numberMatcher = numberPattern.matcher(number); - if (numberMatcher.matches()) { + private ValidationResult testNumberLength(String number, PhoneNumberDesc phoneNumberDesc) { + List possibleLengths = phoneNumberDesc.getPossibleLengthList(); + List localLengths = phoneNumberDesc.getPossibleLengthLocalOnlyList(); + int actualLength = number.length(); + if (localLengths.contains(actualLength)) { return ValidationResult.IS_POSSIBLE; } - if (numberMatcher.lookingAt()) { - return ValidationResult.TOO_LONG; - } else { + // There should always be "possibleLengths" set for every element. This will be a build-time + // check once ShortNumberMetadata.xml is migrated to contain this information as well. + int minimumLength = possibleLengths.get(0); + if (minimumLength == actualLength) { + return ValidationResult.IS_POSSIBLE; + } else if (minimumLength > actualLength) { return ValidationResult.TOO_SHORT; + } else if (possibleLengths.get(possibleLengths.size() - 1) < actualLength) { + return ValidationResult.TOO_LONG; } - } - - /** - * Helper method to check whether a number is too short to be a regular length phone number in a - * region. - */ - private boolean isShorterThanPossibleNormalNumber(PhoneMetadata regionMetadata, String number) { - Pattern possibleNumberPattern = regexCache.getPatternForRegex( - regionMetadata.getGeneralDesc().getPossibleNumberPattern()); - return testNumberLengthAgainstPattern(possibleNumberPattern, number) == - ValidationResult.TOO_SHORT; + // Note that actually the number is not too long if possibleLengths does not contain the length: + // we know it is less than the highest possible number length, and higher than the lowest + // possible number length. However, we don't currently have an enum to express this, so we + // return TOO_LONG in the short-term. + // We skip the first element; we've already checked it. + return possibleLengths.subList(1, possibleLengths.size()).contains(actualLength) + ? ValidationResult.IS_POSSIBLE : ValidationResult.TOO_LONG; } /** @@ -2424,9 +2427,7 @@ public class PhoneNumberUtil { String regionCode = getRegionCodeForCountryCode(countryCode); // Metadata cannot be null because the country calling code is valid. PhoneMetadata metadata = getMetadataForRegionOrCallingCode(countryCode, regionCode); - Pattern possibleNumberPattern = - regexCache.getPatternForRegex(metadata.getGeneralDesc().getPossibleNumberPattern()); - return testNumberLengthAgainstPattern(possibleNumberPattern, nationalNumber); + return testNumberLength(nationalNumber, metadata.getGeneralDesc()); } /** @@ -2596,15 +2597,12 @@ public class PhoneNumberUtil { regexCache.getPatternForRegex(generalDesc.getNationalNumberPattern()); maybeStripNationalPrefixAndCarrierCode( potentialNationalNumber, defaultRegionMetadata, null /* Don't need the carrier code */); - Pattern possibleNumberPattern = - regexCache.getPatternForRegex(generalDesc.getPossibleNumberPattern()); // If the number was not valid before but is valid now, or if it was too long before, we // consider the number with the country calling code stripped to be a better result and // keep that instead. if ((!validNumberPattern.matcher(fullNumber).matches() && validNumberPattern.matcher(potentialNationalNumber).matches()) - || testNumberLengthAgainstPattern(possibleNumberPattern, fullNumber.toString()) - == ValidationResult.TOO_LONG) { + || testNumberLength(fullNumber.toString(), generalDesc) == ValidationResult.TOO_LONG) { nationalNumber.append(potentialNationalNumber); if (keepRawInput) { phoneNumber.setCountryCodeSource(CountryCodeSource.FROM_NUMBER_WITHOUT_PLUS_SIGN); @@ -3013,7 +3011,8 @@ public class PhoneNumberUtil { // We require that the NSN remaining after stripping the national prefix and carrier code be // long enough to be a possible length for the region. Otherwise, we don't do the stripping, // since the original number could be a valid short number. - if (!isShorterThanPossibleNormalNumber(regionMetadata, potentialNationalNumber.toString())) { + if (testNumberLength(potentialNationalNumber.toString(), regionMetadata.getGeneralDesc()) + != ValidationResult.TOO_SHORT) { normalizedNationalNumber = potentialNationalNumber; if (keepRawInput) { 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 e5313c7b3..274f874c4 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java @@ -145,8 +145,12 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { assertEquals("[13-689]\\d{9}|2[0-35-9]\\d{8}", metadata.getGeneralDesc().getNationalNumberPattern()); assertEquals("\\d{7}(?:\\d{3})?", metadata.getGeneralDesc().getPossibleNumberPattern()); - assertTrue(metadata.getGeneralDesc().exactlySameAs(metadata.getFixedLine())); + // Fixed-line data should be inherited from the general desc for the national number pattern, + // since it wasn't overridden. + assertEquals(metadata.getGeneralDesc().getNationalNumberPattern(), + metadata.getFixedLine().getNationalNumberPattern()); assertEquals("\\d{10}", metadata.getTollFree().getPossibleNumberPattern()); + assertEquals(1, metadata.getGeneralDesc().getPossibleLengthCount()); assertEquals(10, metadata.getGeneralDesc().getPossibleLength(0)); // Possible lengths are the same as the general description, so aren't stored separately in the // toll free element as well. @@ -1315,7 +1319,7 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { assertTrue(phoneUtil.isPossibleNumber("253-0000", RegionCode.US)); assertTrue(phoneUtil.isPossibleNumber("+1 650 253 0000", RegionCode.GB)); assertTrue(phoneUtil.isPossibleNumber("+44 20 7031 3000", RegionCode.GB)); - assertTrue(phoneUtil.isPossibleNumber("(020) 7031 3000", RegionCode.GB)); + assertTrue(phoneUtil.isPossibleNumber("(020) 7031 300", RegionCode.GB)); assertTrue(phoneUtil.isPossibleNumber("7031 3000", RegionCode.GB)); assertTrue(phoneUtil.isPossibleNumber("3331 6005", RegionCode.NZ)); assertTrue(phoneUtil.isPossibleNumber("+800 1234 5678", RegionCode.UN001)); diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_AD b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_AD index 42e2c8572..517691e37 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_AD and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_AD differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BB b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BB index 76b000cd1..8a7bac095 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BB and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BB differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BR b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BR index 228417670..f02fb5915 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BR and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_BR differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CA b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CA index 3be2c4494..0eeacb8d8 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CA and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CA differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CC b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CC index 7547c46ff..2e1158f01 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CC and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CC differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CX b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CX index 7c76e1729..de3bfa562 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CX and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_CX differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_FR b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_FR index c15c8cda8..700304437 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_FR and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_FR differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GB b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GB index 59c486189..544cb203a 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GB and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GB differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GG b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GG index b20aac7ba..1a781edaa 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GG and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_GG differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_SE b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_SE index 7e9e68b71..f3468ec40 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_SE and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_SE differ diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_US b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_US index 6ee417836..5e95bd020 100644 Binary files a/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_US and b/java/libphonenumber/test/com/google/i18n/phonenumbers/data/PhoneNumberMetadataProtoForTesting_US differ diff --git a/java/pending_code_changes.txt b/java/pending_code_changes.txt index 74cf0cd95..5bbc20bcc 100644 --- a/java/pending_code_changes.txt +++ b/java/pending_code_changes.txt @@ -4,6 +4,12 @@ Build changes: - Fix geocoding file generation to handle language codes that are not just two letters long properly and to process filenames in the same order. Code changes: + - Switching the internal implementation of isPossible and related functions + to use the new possibleLengths metadata. This affects a lot of countries, + making isPossible more restrictive as more precise data is available. It + also affects parsing ambiguous and invalid numbers, as we decide whether + to strip a possible national prefix (1) or country code based on the length + of the number and whether it is possible before or after this. - Formatting, naming (LOGGER -> logger) and comment tweaks to follow style guide - Removal of unneeded canBeGeocoded method in the diff --git a/resources/PhoneNumberMetadataForTesting.xml b/resources/PhoneNumberMetadataForTesting.xml index 7e1913803..e1f1c33e7 100644 --- a/resources/PhoneNumberMetadataForTesting.xml +++ b/resources/PhoneNumberMetadataForTesting.xml @@ -23,6 +23,13 @@ @@ -169,12 +176,27 @@ + + 246\d{7} + \d{7,10} + + + + + recognised by the library. It is also used for formatInOriginalFormat tests, so some length + metadata is needed. --> + + \d{8,10} + \d{8,10} + + + + @@ -262,13 +284,27 @@ + + 226\d{7} + \d{7,10} + + + + + recognised by the library, and some length information is needed for parsing. --> + + \d{6,10} + \d{6,10} + + + + @@ -326,8 +362,15 @@ + recognised by the library, and some length information is needed for parsing. --> + + \d{8,10} + \d{8,10} + + + + @@ -415,6 +458,9 @@ 3\d{6} \d{7} + + + @@ -445,7 +491,7 @@ [1-6]\d{9} - + 7[1-57-9]\d{8} @@ -488,6 +534,13 @@ + + \d{6,10} + \d{6,10} + + + + @@ -903,6 +956,13 @@ + + \d{9} + \d{9} + + + + @@ -976,6 +1036,9 @@ \d{7}(?:\d{3})? 1234567890 + + + 800\d{7}