From d46ff896cef686fbcfc0fd7cfadfafbd3d8f6a88 Mon Sep 17 00:00:00 2001 From: Nikolaos Trogkanis Date: Fri, 28 Oct 2011 10:09:37 +0000 Subject: [PATCH] - Fix formatNumberForMobileDialing that is modifying its input PhoneNumber. - Fix incorect use of string equality. - Fix testGetEnglishDataPath that is failing under windows. - Remove unused method and variables. --- .../i18n/phonenumbers/PhoneNumberUtil.java | 24 +++++++++---------- .../i18n/phonenumbers/ShortNumberUtil.java | 4 ++-- .../phonenumbers/PhoneNumberMatcherTest.java | 9 ------- .../geocoding/GenerateAreaCodeData.java | 9 +++---- .../geocoding/GenerateAreaCodeDataTest.java | 3 +-- 5 files changed, 18 insertions(+), 31 deletions(-) diff --git a/java/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index a039c472a..01d028188 100644 --- a/java/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -1179,27 +1179,27 @@ public class PhoneNumberUtil { String formattedNumber; // Clear the extension, as that part cannot normally be dialed together with the main number. - number.clearExtension(); - PhoneNumberType numberType = getNumberType(number); - if ((regionCode == "CO") && (regionCallingFrom == "CO") && + PhoneNumber numberNoExt = new PhoneNumber().mergeFrom(number).clearExtension(); + PhoneNumberType numberType = getNumberType(numberNoExt); + if ((regionCode.equals("CO")) && (regionCallingFrom.equals("CO")) && (numberType == PhoneNumberType.FIXED_LINE)) { formattedNumber = - formatNationalNumberWithCarrierCode(number, COLOMBIA_MOBILE_TO_FIXED_LINE_PREFIX); - } else if ((regionCode == "BR") && (regionCallingFrom == "BR") && + formatNationalNumberWithCarrierCode(numberNoExt, COLOMBIA_MOBILE_TO_FIXED_LINE_PREFIX); + } else if ((regionCode.equals("BR")) && (regionCallingFrom.equals("BR")) && ((numberType == PhoneNumberType.FIXED_LINE) || (numberType == PhoneNumberType.MOBILE) || (numberType == PhoneNumberType.FIXED_LINE_OR_MOBILE))) { - formattedNumber = number.hasPreferredDomesticCarrierCode() - ? formatNationalNumberWithPreferredCarrierCode(number, "") + formattedNumber = numberNoExt.hasPreferredDomesticCarrierCode() + ? 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. // Because of that, we return an empty string here. : ""; - } else if (canBeInternationallyDialled(number)) { - return withFormatting ? format(number, PhoneNumberFormat.INTERNATIONAL) - : format(number, PhoneNumberFormat.E164); + } else if (canBeInternationallyDialled(numberNoExt)) { + return withFormatting ? format(numberNoExt, PhoneNumberFormat.INTERNATIONAL) + : format(numberNoExt, PhoneNumberFormat.E164); } else { - formattedNumber = (regionCallingFrom == regionCode) - ? format(number, PhoneNumberFormat.NATIONAL) : ""; + formattedNumber = (regionCallingFrom.equals(regionCode)) + ? format(numberNoExt, PhoneNumberFormat.NATIONAL) : ""; } return withFormatting ? formattedNumber : normalizeDigitsOnly(formattedNumber); } diff --git a/java/src/com/google/i18n/phonenumbers/ShortNumberUtil.java b/java/src/com/google/i18n/phonenumbers/ShortNumberUtil.java index acf878180..458775985 100644 --- a/java/src/com/google/i18n/phonenumbers/ShortNumberUtil.java +++ b/java/src/com/google/i18n/phonenumbers/ShortNumberUtil.java @@ -28,7 +28,7 @@ import java.util.regex.Pattern; */ public class ShortNumberUtil { - private static PhoneNumberUtil phoneUtil; + private final PhoneNumberUtil phoneUtil; public ShortNumberUtil() { phoneUtil = PhoneNumberUtil.getInstance(); @@ -62,7 +62,7 @@ public class ShortNumberUtil { PhoneNumberDesc emergencyNumberDesc = phoneUtil.getMetadataForRegion(regionCode).getEmergency(); Pattern emergencyNumberPattern = Pattern.compile(emergencyNumberDesc.getNationalNumberPattern()); - if (regionCode == "BR") { + if (regionCode.equals("BR")) { // This is to prevent Brazilian local numbers which start with 911 being incorrectly // classified as emergency numbers. In Brazil, it is impossible to append additional digits to // an emergency number to dial the number. diff --git a/java/test/com/google/i18n/phonenumbers/PhoneNumberMatcherTest.java b/java/test/com/google/i18n/phonenumbers/PhoneNumberMatcherTest.java index b7000bda9..7b22e3b01 100644 --- a/java/test/com/google/i18n/phonenumbers/PhoneNumberMatcherTest.java +++ b/java/test/com/google/i18n/phonenumbers/PhoneNumberMatcherTest.java @@ -433,8 +433,6 @@ public class PhoneNumberMatcherTest extends TestCase { }; public void testMatchesWithStrictGroupingLeniency() throws Exception { - int noMatchFoundCount = 0; - int wrongMatchFoundCount = 0; List testCases = new ArrayList(); testCases.addAll(Arrays.asList(STRICT_GROUPING_CASES)); testCases.addAll(Arrays.asList(EXACT_GROUPING_CASES)); @@ -895,13 +893,6 @@ public class PhoneNumberMatcherTest extends TestCase { return phoneUtil.findNumbers(text, defaultCountry, leniency, Long.MAX_VALUE).iterator(); } - /** - * Returns true if there were no matches found. - */ - private boolean hasNoMatches(Iterator iterator) { - return !iterator.hasNext(); - } - private boolean hasNoMatches(Iterable iterable) { return !iterable.iterator().hasNext(); } diff --git a/tools/java/java-build/src/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeData.java b/tools/java/java-build/src/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeData.java index 47e7fa8af..556426541 100644 --- a/tools/java/java-build/src/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeData.java +++ b/tools/java/java-build/src/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeData.java @@ -336,14 +336,12 @@ public class GenerateAreaCodeData { for (Map.Entry mapping : mappings.entrySet()) { String prefix = String.valueOf(mapping.getKey()); File targetFile = null; - int correspondingAreaCode = -1; for (File outputBinaryFile : outputBinaryFiles) { String outputBinaryFilePrefix = getPhonePrefixLanguagePairFromFilename(outputBinaryFile.getName()).prefix; if (prefix.startsWith(outputBinaryFilePrefix)) { targetFile = outputBinaryFile; - correspondingAreaCode = Integer.parseInt(outputBinaryFilePrefix); break; } } @@ -361,9 +359,8 @@ public class GenerateAreaCodeData { * Gets the English data text file path corresponding to the provided one. */ // @VisibleForTesting - static String getEnglishDataPath(File inputTextFile) { - return LANGUAGE_IN_FILE_PATH_PATTERN.matcher(inputTextFile.getAbsolutePath()).replaceFirst( - "$1en$2"); + static String getEnglishDataPath(String inputTextFileName) { + return LANGUAGE_IN_FILE_PATH_PATTERN.matcher(inputTextFileName).replaceFirst("$1en$2"); } /** @@ -416,7 +413,7 @@ public class GenerateAreaCodeData { */ private void makeDataFallbackToEnglish(File inputTextFile, SortedMap mappings) throws IOException { - File englishTextFile = new File(getEnglishDataPath(inputTextFile)); + File englishTextFile = new File(getEnglishDataPath(inputTextFile.getAbsolutePath())); if (inputTextFile.getAbsolutePath().equals(englishTextFile.getAbsolutePath()) || !englishTextFile.exists()) { return; diff --git a/tools/java/java-build/test/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeDataTest.java b/tools/java/java-build/test/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeDataTest.java index 5c7d880ac..451f6559b 100644 --- a/tools/java/java-build/test/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeDataTest.java +++ b/tools/java/java-build/test/com/google/i18n/phonenumbers/geocoding/GenerateAreaCodeDataTest.java @@ -200,8 +200,7 @@ public class GenerateAreaCodeDataTest extends TestCase { } public void testGetEnglishDataPath() { - assertEquals("/path/en/33.txt", - GenerateAreaCodeData.getEnglishDataPath(new File("/path/fr/33.txt"))); + assertEquals("/path/en/33.txt", GenerateAreaCodeData.getEnglishDataPath("/path/fr/33.txt")); } public void testHasOverlap() {