Repository: marmelroy/PhoneNumberKit (swift)

Issue:
Invalid US phone number incorrectly parsed as valid international number
phoneNumberKit.parse sometimes treats an invalid US number as a valid international number when the first part of the number matches a country code.

Here's a test that reproduces the issue. Both should pass, but only first assert passes while the second fails. This happens because 212 is not a country code while 352 is the country code of Luxembourg.

```
func testRegionCountryCodeConflict() {
    XCTAssertThrowsError(try phoneNumberKit.parse("212-2344", withRegion: "US")) { error in
        XCTAssertEqual(error as? PhoneNumberError, PhoneNumberError.notANumber)
    }
    XCTAssertThrowsError(try phoneNumberKit.parse("352-2344", withRegion: "US")) { error in
        XCTAssertEqual(error as? PhoneNumberError, PhoneNumberError.notANumber)
    }
}
```

Pull request:
Fix #640

Patch:
diff --git a/PhoneNumberKit/Constants.swift b/PhoneNumberKit/Constants.swift
index d8a160a19..cde03addd 100644
--- a/PhoneNumberKit/Constants.swift
+++ b/PhoneNumberKit/Constants.swift
@@ -24,7 +24,7 @@ enum PhoneNumberCountryCodeSource {
 
  - GeneralError: A general error occured.
  - InvalidCountryCode: A country code could not be found or the one found was invalid
- - NotANumber: The string provided is not a number
+ - InvalidNumber: The string provided is not a number
  - TooLong: The string provided is too long to be a valid number
  - TooShort: The string provided is too short to be a valid number
  - Deprecated: The method used was deprecated
@@ -34,8 +34,7 @@ enum PhoneNumberCountryCodeSource {
 public enum PhoneNumberError: Error, Equatable {
     case generalError
     case invalidCountryCode
-    case notANumber
-    case unknownType
+    case invalidNumber
     case tooLong
     case tooShort
     case deprecated
@@ -48,8 +47,7 @@ extension PhoneNumberError: LocalizedError {
         switch self {
         case .generalError: return NSLocalizedString("An error occured whilst validating the phone number.", comment: "")
         case .invalidCountryCode: return NSLocalizedString("The country code is invalid.", comment: "")
-        case .notANumber: return NSLocalizedString("The number provided is invalid.", comment: "")
-        case .unknownType: return NSLocalizedString("Phone number type is unknown.", comment: "")
+        case .invalidNumber: return NSLocalizedString("The number provided is invalid.", comment: "")
         case .tooLong: return NSLocalizedString("The number provided is too long.", comment: "")
         case .tooShort: return NSLocalizedString("The number provided is too short.", comment: "")
         case .deprecated: return NSLocalizedString("This function is deprecated.", comment: "")
diff --git a/PhoneNumberKit/ParseManager.swift b/PhoneNumberKit/ParseManager.swift
index 5859891b5..ca1dd219b 100644
--- a/PhoneNumberKit/ParseManager.swift
+++ b/PhoneNumberKit/ParseManager.swift
@@ -62,7 +62,7 @@ final class ParseManager {
             if let result = try validPhoneNumber(from: nationalNumber, using: regionMetadata, countryCode: regionMetadata.countryCode, ignoreType: ignoreType, numberString: numberString, numberExtension: numberExtension) {
                 return result
             }
-            throw PhoneNumberError.notANumber
+            throw PhoneNumberError.invalidNumber
         }
         
         // If country code is not default, grab correct metadata (6)
@@ -85,7 +85,7 @@ final class ParseManager {
         }
         
         switch possibleResults.count {
-        case 0: throw PhoneNumberError.notANumber
+        case 0: throw PhoneNumberError.invalidNumber
         case 1: return possibleResults.first!
         default: throw PhoneNumberError.ambiguousNumber(phoneNumbers: possibleResults)
         }
@@ -169,13 +169,14 @@ final class ParseManager {
         self.parser.stripNationalPrefix(&nationalNumber, metadata: regionMetadata)
 
         // Test number against general number description for correct metadata (2)
-        if let generalNumberDesc = regionMetadata.generalDesc, regexManager.hasValue(generalNumberDesc.nationalNumberPattern) == false || parser.isNumberMatchingDesc(nationalNumber, numberDesc: generalNumberDesc) == false {
+        if let generalNumberDesc = regionMetadata.generalDesc,
+            regexManager.hasValue(generalNumberDesc.nationalNumberPattern) == false || parser.isNumberMatchingDesc(nationalNumber, numberDesc: generalNumberDesc) == false {
             return nil
         }
         // Finalize remaining parameters and create phone number object (3)
         let leadingZero = nationalNumber.hasPrefix("0")
         guard let finalNationalNumber = UInt64(nationalNumber) else {
-            throw PhoneNumberError.notANumber
+            throw PhoneNumberError.invalidNumber
         }
 
         // Check if the number if of a known type (4)
@@ -186,7 +187,7 @@ final class ParseManager {
             }
             type = self.parser.checkNumberType(String(nationalNumber), metadata: regionMetadata, leadingZero: leadingZero)
             if type == .unknown {
-                throw PhoneNumberError.unknownType
+                throw PhoneNumberError.invalidNumber
             }
         }
 
diff --git a/PhoneNumberKit/PhoneNumberKit.swift b/PhoneNumberKit/PhoneNumberKit.swift
index 04c55e8b3..4958fedda 100644
--- a/PhoneNumberKit/PhoneNumberKit.swift
+++ b/PhoneNumberKit/PhoneNumberKit.swift
@@ -36,17 +36,7 @@ public final class PhoneNumberKit: NSObject {
     ///   - ignoreType: Avoids number type checking for faster performance.
     /// - Returns: PhoneNumber object.
     public func parse(_ numberString: String, withRegion region: String = PhoneNumberKit.defaultRegionCode(), ignoreType: Bool = false) throws -> PhoneNumber {
-        var numberStringWithPlus = numberString
-
-        do {
-            return try self.parseManager.parse(numberString, withRegion: region, ignoreType: ignoreType)
-        } catch {
-            if numberStringWithPlus.first != "+" {
-                numberStringWithPlus = "+" + numberStringWithPlus
-            }
-        }
-
-        return try self.parseManager.parse(numberStringWithPlus, withRegion: region, ignoreType: ignoreType)
+        return try self.parseManager.parse(numberString, withRegion: region, ignoreType: ignoreType)
     }
 
     /// Parses an array of number strings. Optimised for performance. Invalid numbers are ignored in the resulting array
diff --git a/PhoneNumberKit/RegexManager.swift b/PhoneNumberKit/RegexManager.swift
index 7730220a7..a00736863 100644
--- a/PhoneNumberKit/RegexManager.swift
+++ b/PhoneNumberKit/RegexManager.swift
@@ -64,7 +64,7 @@ final class RegexManager {
         if let firstMatch = fallBackMatches.first {
             return firstMatch
         } else {
-            throw PhoneNumberError.notANumber
+            throw PhoneNumberError.invalidNumber
         }
     }
 


Test patch:
diff --git a/PhoneNumberKitTests/PhoneNumberKitParsingTests.swift b/PhoneNumberKitTests/PhoneNumberKitParsingTests.swift
index c244e2457..b0ae4906f 100644
--- a/PhoneNumberKitTests/PhoneNumberKitParsingTests.swift
+++ b/PhoneNumberKitTests/PhoneNumberKitParsingTests.swift
@@ -31,79 +31,6 @@ class PhoneNumberKitParsingTests: XCTestCase {
         }
     }
 
-//    func testUSMetadata() {
-//        let sut = self.phoneNumberKit.metadataManager.filterTerritories(byCountry: "US")!
-//        XCTAssertEqual(sut.codeID, "US")
-//        XCTAssertEqual(sut.countryCode, 1)
-//        XCTAssertEqual(sut.internationalPrefix, "011")
-//        XCTAssertEqual(sut.mainCountryForCode, true)
-//        XCTAssertEqual(sut.nationalPrefix, "1")
-//        XCTAssertNil(sut.nationalPrefixFormattingRule)
-//        XCTAssertEqual(sut.nationalPrefixForParsing, "1")
-//        XCTAssertNil(sut.nationalPrefixTransformRule)
-//        XCTAssertNil(sut.preferredExtnPrefix)
-//        let fixedLine = sut.fixedLine!
-//        XCTAssertEqual(fixedLine.exampleNumber, "2015550123")
-//        XCTAssertEqual(fixedLine.nationalNumberPattern, "5(?:05(?:[2-57-9]\\d\\d|6(?:[0-35-9]\\d|44))|82(?:2(?:0[0-3]|[268]2)|3(?:0[02]|22|33)|4(?:00|4[24]|65|82)|5(?:00|29|58|83)|6(?:00|66|82)|7(?:58|77)|8(?:00|42|88)|9(?:00|9[89])))\\d{4}|(?:2(?:0[1-35-9]|1[02-9]|2[03-589]|3[149]|4[08]|5[1-46]|6[0279]|7[0269]|8[13])|3(?:0[1-57-9]|1[02-9]|2[01356]|3[0-24679]|4[167]|5[12]|6[014]|8[056])|4(?:0[124-9]|1[02-579]|2[3-5]|3[0245]|4[023578]|58|6[349]|7[0589]|8[04])|5(?:0[1-47-9]|1[0235-8]|20|3[0149]|4[01]|5[19]|6[1-47]|7[0-5]|8[056])|6(?:0[1-35-9]|1[024-9]|2[03689]|[34][016]|5[01679]|6[0-279]|78|8[0-29])|7(?:0[1-46-8]|1[2-9]|2[04-7]|3[1247]|4[037]|5[47]|6[02359]|7[0-59]|8[156])|8(?:0[1-68]|1[02-8]|2[068]|3[0-289]|4[03578]|5[046-9]|6[02-5]|7[028])|9(?:0[1346-9]|1[02-9]|2[0589]|3[0146-8]|4[01357-9]|5[12469]|7[0-389]|8[04-69]))[2-9]\\d{6}")
-//        XCTAssertNil(fixedLine.possibleNumberPattern)
-//        let generalDesc = sut.generalDesc!
-//        XCTAssertNil(generalDesc.exampleNumber)
-//        XCTAssertEqual(generalDesc.nationalNumberPattern, "[2-9]\\d{9}|3\\d{6}")
-//        XCTAssertNil(generalDesc.possibleNumberPattern)
-//        let mobile = sut.mobile!
-//        XCTAssertEqual(mobile.exampleNumber, "2015550123")
-//        XCTAssertEqual(mobile.nationalNumberPattern, "5(?:05(?:[2-57-9]\\d\\d|6(?:[0-35-9]\\d|44))|82(?:2(?:0[0-3]|[268]2)|3(?:0[02]|22|33)|4(?:00|4[24]|65|82)|5(?:00|29|58|83)|6(?:00|66|82)|7(?:58|77)|8(?:00|42|88)|9(?:00|9[89])))\\d{4}|(?:2(?:0[1-35-9]|1[02-9]|2[03-589]|3[149]|4[08]|5[1-46]|6[0279]|7[0269]|8[13])|3(?:0[1-57-9]|1[02-9]|2[01356]|3[0-24679]|4[167]|5[12]|6[014]|8[056])|4(?:0[124-9]|1[02-579]|2[3-5]|3[0245]|4[023578]|58|6[349]|7[0589]|8[04])|5(?:0[1-47-9]|1[0235-8]|20|3[0149]|4[01]|5[19]|6[1-47]|7[0-5]|8[056])|6(?:0[1-35-9]|1[024-9]|2[03689]|[34][016]|5[01679]|6[0-279]|78|8[0-29])|7(?:0[1-46-8]|1[2-9]|2[04-7]|3[1247]|4[037]|5[47]|6[02359]|7[0-59]|8[156])|8(?:0[1-68]|1[02-8]|2[068]|3[0-289]|4[03578]|5[046-9]|6[02-5]|7[028])|9(?:0[1346-9]|1[02-9]|2[0589]|3[0146-8]|4[01357-9]|5[12469]|7[0-389]|8[04-69]))[2-9]\\d{6}")
-//        XCTAssertNil(mobile.possibleNumberPattern)
-//        let personalNumber = sut.personalNumber!
-//        XCTAssertEqual(personalNumber.exampleNumber, "5002345678")
-//        XCTAssertEqual(personalNumber.nationalNumberPattern, "52(?:3(?:[2-46-9][02-9]\\d|5(?:[02-46-9]\\d|5[0-46-9]))|4(?:[2-478][02-9]\\d|5(?:[034]\\d|2[024-9]|5[0-46-9])|6(?:0[1-9]|[2-9]\\d)|9(?:[05-9]\\d|2[0-5]|49)))\\d{4}|52[34][2-9]1[02-9]\\d{4}|5(?:00|2[125-7]|33|44|66|77|88)[2-9]\\d{6}")
-//        XCTAssertNil(personalNumber.possibleNumberPattern)
-//        let premiumRate = sut.premiumRate!
-//        XCTAssertEqual(premiumRate.exampleNumber, "9002345678")
-//        XCTAssertEqual(premiumRate.nationalNumberPattern, "900[2-9]\\d{6}")
-//        XCTAssertNil(premiumRate.possibleNumberPattern)
-//        let tollFree = sut.tollFree!
-//        XCTAssertEqual(tollFree.exampleNumber, "8002345678")
-//        XCTAssertEqual(tollFree.nationalNumberPattern, "8(?:00|33|44|55|66|77|88)[2-9]\\d{6}")
-//        XCTAssertNil(tollFree.possibleNumberPattern)
-////        let uan = sut.uan!
-////        XCTAssertEqual(uan.exampleNumber, "7102123456")
-////        XCTAssertEqual(uan.nationalNumberPattern, "710[2-9]\\d{6}")
-////        XCTAssertNil(uan.possibleNumberPattern)
-//        let numberFormats = sut.numberFormats
-//        
-//        let firstNumberFormat: MetadataPhoneNumberFormat = numberFormats[0]
-//        XCTAssertEqual(firstNumberFormat.pattern, "(\\d{3})(\\d{4})")
-//        XCTAssertEqual(firstNumberFormat.format, "$1-$2")
-//        XCTAssertEqual(firstNumberFormat.intlFormat, nil)
-//        let firstLeadingDigits = firstNumberFormat.leadingDigitsPatterns!.first
-//        XCTAssertEqual(firstLeadingDigits, "310")
-//        XCTAssertNil(firstNumberFormat.nationalPrefixFormattingRule)
-//        XCTAssertTrue(firstNumberFormat.nationalPrefixOptionalWhenFormatting!)
-//        XCTAssertNil(firstNumberFormat.domesticCarrierCodeFormattingRule)
-//        
-//        let secondNumberFormat: MetadataPhoneNumberFormat = numberFormats[1]
-//        XCTAssertEqual(secondNumberFormat.pattern, "(\\d{3})(\\d{4})")
-//        XCTAssertEqual(secondNumberFormat.format, "$1-$2")
-//        XCTAssertEqual(secondNumberFormat.intlFormat, "NA")
-//        let secondLeadingDigits = secondNumberFormat.leadingDigitsPatterns!.first
-//        XCTAssertEqual(secondLeadingDigits, "[24-9]|3(?:[02-9]|1[1-9])")
-//        XCTAssertNil(secondNumberFormat.nationalPrefixFormattingRule)
-//        XCTAssertFalse(secondNumberFormat.nationalPrefixOptionalWhenFormatting!)
-//        XCTAssertNil(secondNumberFormat.domesticCarrierCodeFormattingRule)
-//        
-//        let thirdNumberFormat: MetadataPhoneNumberFormat = numberFormats[2]
-//        XCTAssertEqual(thirdNumberFormat.pattern, "(\\d{3})(\\d{3})(\\d{4})")
-//        XCTAssertEqual(thirdNumberFormat.format, "($1) $2-$3")
-//        XCTAssertEqual(thirdNumberFormat.intlFormat, "$1-$2-$3")
-//        let thirdLeadingDigits = thirdNumberFormat.leadingDigitsPatterns!.first
-//        XCTAssertEqual(thirdLeadingDigits, "[2-9]")
-//        XCTAssertNil(thirdNumberFormat.nationalPrefixFormattingRule)
-//        XCTAssertTrue(thirdNumberFormat.nationalPrefixOptionalWhenFormatting!)
-//        XCTAssertNil(thirdNumberFormat.domesticCarrierCodeFormattingRule)
-//        XCTAssertNil(sut.leadingDigits)
-//    }
-
     func testUSNumberNoPrefix() {
         do {
             let phoneNumber1 = try phoneNumberKit.parse("650 253 0000", withRegion: "US")
@@ -501,35 +428,25 @@ class PhoneNumberKitParsingTests: XCTestCase {
     }
 
     func testUANumber() {
-        do {
-            let phoneNumber1 = try phoneNumberKit.parse("380501887766", withRegion: "UA")
-            XCTAssertNotNil(phoneNumber1)
-            let phoneNumberInternationalFormat1 = self.phoneNumberKit.format(phoneNumber1, toType: .international)
-            XCTAssertTrue(phoneNumberInternationalFormat1 == "+380 50 188 7766")
-            let phoneNumberNationalFormat1 = self.phoneNumberKit.format(phoneNumber1, toType: .national)
-            XCTAssertTrue(phoneNumberNationalFormat1 == "050 188 7766")
-            let phoneNumberE164Format1 = self.phoneNumberKit.format(phoneNumber1, toType: .e164)
-            XCTAssertTrue(phoneNumberE164Format1 == "+380501887766")
-            let phoneNumber2 = try phoneNumberKit.parse("050 188 7766", withRegion: "UA")
-            XCTAssertNotNil(phoneNumber2)
-            let phoneNumberInternationalFormat2 = self.phoneNumberKit.format(phoneNumber2, toType: .international)
-            XCTAssertTrue(phoneNumberInternationalFormat2 == "+380 50 188 7766")
-            let phoneNumberNationalFormat2 = self.phoneNumberKit.format(phoneNumber2, toType: .national)
-            XCTAssertTrue(phoneNumberNationalFormat2 == "050 188 7766")
-            let phoneNumberE164Format2 = self.phoneNumberKit.format(phoneNumber2, toType: .e164)
-            XCTAssertTrue(phoneNumberE164Format2 == "+380501887766")
-            let phoneNumber3 = try phoneNumberKit.parse("050 188 7766", withRegion: "UA")
-            XCTAssertNotNil(phoneNumber3)
-            let phoneNumberInternationalFormat3 = self.phoneNumberKit.format(phoneNumber3, toType: .international)
-            XCTAssertTrue(phoneNumberInternationalFormat3 == "+380 50 188 7766")
-            let phoneNumberNationalFormat3 = self.phoneNumberKit.format(phoneNumber3, toType: .national)
-            XCTAssertTrue(phoneNumberNationalFormat3 == "050 188 7766")
-            let phoneNumberE164Format3 = self.phoneNumberKit.format(phoneNumber3, toType: .e164)
-            XCTAssertTrue(phoneNumberE164Format3 == "+380501887766")
-        } catch {
-            XCTFail()
-        }
+        let phoneNumber1 = try? phoneNumberKit.parse("501887766", withRegion: "UA")
+        XCTAssertNotNil(phoneNumber1)
+        let phoneNumberInternationalFormat1 = self.phoneNumberKit.format(phoneNumber1!, toType: .international)
+        XCTAssertTrue(phoneNumberInternationalFormat1 == "+380 50 188 7766")
+        let phoneNumberNationalFormat1 = self.phoneNumberKit.format(phoneNumber1!, toType: .national)
+        XCTAssertTrue(phoneNumberNationalFormat1 == "050 188 7766")
+        let phoneNumberE164Format1 = self.phoneNumberKit.format(phoneNumber1!, toType: .e164)
+        XCTAssertTrue(phoneNumberE164Format1 == "+380501887766")
+        
+        let phoneNumber2 = try? phoneNumberKit.parse("050 188 7766", withRegion: "UA")
+        XCTAssertNotNil(phoneNumber2)
+        let phoneNumberInternationalFormat2 = self.phoneNumberKit.format(phoneNumber2!, toType: .international)
+        XCTAssertTrue(phoneNumberInternationalFormat2 == "+380 50 188 7766")
+        let phoneNumberNationalFormat2 = self.phoneNumberKit.format(phoneNumber2!, toType: .national)
+        XCTAssertTrue(phoneNumberNationalFormat2 == "050 188 7766")
+        let phoneNumberE164Format2 = self.phoneNumberKit.format(phoneNumber2!, toType: .e164)
+        XCTAssertTrue(phoneNumberE164Format2 == "+380501887766")
     }
+    
     func testExtensionWithCommaParsing() {
         guard let number = try? phoneNumberKit.parse("+33 612-345-678,22") else {
             XCTFail()
@@ -553,4 +470,13 @@ class PhoneNumberKitParsingTests: XCTestCase {
         let address = "+1 345 916 1234"
         try XCTAssertNotNil(phoneNumberKit.parse(address, withRegion: "JM"))
     }
+    
+    func testRegionCountryCodeConflict() {
+        XCTAssertThrowsError(try phoneNumberKit.parse("212-2344", withRegion: "US")) { error in
+            XCTAssertEqual(error as? PhoneNumberError, PhoneNumberError.invalidNumber)
+        }
+        XCTAssertThrowsError(try phoneNumberKit.parse("352-2344", withRegion: "US")) { error in
+            XCTAssertEqual(error as? PhoneNumberError, PhoneNumberError.invalidNumber)
+        }
+    }
 }
diff --git a/PhoneNumberKitTests/PhoneNumberKitTests.swift b/PhoneNumberKitTests/PhoneNumberKitTests.swift
index f740e552c..23cc39668 100644
--- a/PhoneNumberKitTests/PhoneNumberKitTests.swift
+++ b/PhoneNumberKitTests/PhoneNumberKitTests.swift
@@ -370,15 +370,8 @@ class PhoneNumberKitTests: XCTestCase {
 
     //  Invalid number invalid format
     func testInvalidNumberNotANumberInvalidFormat() {
-        let testNumber = "+33(02)689555555"
-        do {
-            let phoneNumber = try phoneNumberKit.parse(testNumber)
-            _ = self.phoneNumberKit.format(phoneNumber, toType: .e164)
-            XCTFail()
-        } catch PhoneNumberError.notANumber {
-            XCTAssert(true)
-        } catch {
-            XCTAssert(false)
+        XCTAssertThrowsError(try phoneNumberKit.parse("+33(02)689555555")) { error in
+            XCTAssertEqual(error as? PhoneNumberError, PhoneNumberError.invalidNumber)
         }
     }