diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 7734fe6eb..beab66ffe 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -7,6 +7,7 @@ on: branches: [ "main" ] schedule: - cron: '35 21 * * 0' + workflow_dispatch: concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/csharp/PhoneNumbers.MetadataBuilder/Program.cs b/csharp/PhoneNumbers.MetadataBuilder/Program.cs index a47a33077..a51312c00 100644 --- a/csharp/PhoneNumbers.MetadataBuilder/Program.cs +++ b/csharp/PhoneNumbers.MetadataBuilder/Program.cs @@ -141,7 +141,7 @@ private static int BuildGeocoding(string inputDir, string outputDir) { var countryCode = Path.GetFileNameWithoutExtension(txtPath); var map = ParseAreaCodeText(txtPath); - var outPath = Path.Combine(outputDir, $"{lang}.{countryCode}"); + var outPath = Path.Join(outputDir, Path.GetFileName($"{lang}.{countryCode}")); using var gz = new GZipStream(File.Create(outPath), CompressionLevel.SmallestSize); BuildPrefixMapFromBin.WriteAreaCodeMap(gz, map); written++; @@ -251,11 +251,7 @@ private static bool IsOutputUpToDate(string inputXml, string outputDir, string f var existing = Directory.GetFiles(outputDir, filePrefix + "_*"); if (existing.Length == 0) return false; var inputMTime = File.GetLastWriteTimeUtc(inputXml); - foreach (var file in existing) - { - if (File.GetLastWriteTimeUtc(file) < inputMTime) return false; - } - return true; + return !existing.Any(file => File.GetLastWriteTimeUtc(file) < inputMTime); } /// @@ -267,17 +263,9 @@ private static bool IsGeocodingOutputUpToDate(string inputDir, string outputDir) if (!Directory.Exists(outputDir)) return false; var existing = Directory.GetFiles(outputDir); if (existing.Length == 0) return false; - var newestInput = DateTime.MinValue; - foreach (var f in Directory.EnumerateFiles(inputDir, "*.txt", SearchOption.AllDirectories)) - { - var t = File.GetLastWriteTimeUtc(f); - if (t > newestInput) newestInput = t; - } - foreach (var file in existing) - { - if (File.GetLastWriteTimeUtc(file) < newestInput) return false; - } - return true; + var newestInput = Directory.EnumerateFiles(inputDir, "*.txt", SearchOption.AllDirectories) + .Select(File.GetLastWriteTimeUtc).DefaultIfEmpty(DateTime.MinValue).Max(); + return !existing.Any(file => File.GetLastWriteTimeUtc(file) < newestInput); } private static SortedDictionary ParseAreaCodeText(string path) @@ -348,7 +336,7 @@ private static int BuildPerRegion( foreach (var metadata in metadataList) { var key = MakeFileNameKey(metadata, isAlternateFormatsMetadata); - var path = Path.Combine(outputDir, $"{filePrefix}_{key}"); + var path = Path.Join(outputDir, Path.GetFileName($"{filePrefix}_{key}")); using var gz = new GZipStream(File.Create(path), CompressionLevel.SmallestSize); BuildMetadataFromBin.WriteMetadata(gz, metadata); written++; diff --git a/csharp/PhoneNumbers.Test/TestBuildMetadataFromXml.cs b/csharp/PhoneNumbers.Test/TestBuildMetadataFromXml.cs index d3a23e343..d0781e33a 100644 --- a/csharp/PhoneNumbers.Test/TestBuildMetadataFromXml.cs +++ b/csharp/PhoneNumbers.Test/TestBuildMetadataFromXml.cs @@ -181,15 +181,8 @@ public void TestLoadInternationalFormatExpectsOnlyOnePattern() var metadata = new PhoneMetadata.Builder(); // Should throw an exception as multiple intlFormats are provided. - try - { - BuildMetadataFromXml.LoadInternationalFormat(metadata, numberFormatElement, ""); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => + BuildMetadataFromXml.LoadInternationalFormat(metadata, numberFormatElement, "")); } [Fact] @@ -247,15 +240,8 @@ public void TestLoadNationalFormatRequiresFormat() var metadata = new PhoneMetadata.Builder(); var numberFormat = new NumberFormat.Builder(); - try - { - BuildMetadataFromXml.LoadNationalFormat(metadata, numberFormatElement, numberFormat); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => + BuildMetadataFromXml.LoadNationalFormat(metadata, numberFormatElement, numberFormat)); } [Fact] @@ -266,15 +252,8 @@ public void TestLoadNationalFormatExpectsExactlyOneFormat() var metadata = new PhoneMetadata.Builder(); var numberFormat = new NumberFormat.Builder(); - try - { - BuildMetadataFromXml.LoadNationalFormat(metadata, numberFormatElement, numberFormat); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => + BuildMetadataFromXml.LoadNationalFormat(metadata, numberFormatElement, numberFormat)); } // Tests loadAvailableFormats(). diff --git a/csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs b/csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs index 4286c06e0..ad6f92bc1 100644 --- a/csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs +++ b/csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs @@ -27,8 +27,8 @@ public void TestAreaCodeMap_WriteAndRead_VerifiesIntegrity() Assert.Equal(testMap.Count, deserializedMap.Count); foreach (var kvp in testMap) { - Assert.True(deserializedMap.ContainsKey(kvp.Key)); - Assert.Equal(kvp.Value, deserializedMap[kvp.Key]); + Assert.True(deserializedMap.TryGetValue(kvp.Key, out var value)); + Assert.Equal(kvp.Value, value); } } @@ -93,8 +93,8 @@ public void TestTimezoneMap_WriteAndRead_VerifiesIntegrity() Assert.Equal(testMap.Count, deserializedMap.Count); foreach (var kvp in testMap) { - Assert.True(deserializedMap.ContainsKey(kvp.Key)); - Assert.Equal(kvp.Value, deserializedMap[kvp.Key]); + Assert.True(deserializedMap.TryGetValue(kvp.Key, out var value)); + Assert.Equal(kvp.Value, value); } } diff --git a/csharp/PhoneNumbers.Test/TestExampleNumbers.cs b/csharp/PhoneNumbers.Test/TestExampleNumbers.cs index 6523a4f52..ac22cc63d 100644 --- a/csharp/PhoneNumbers.Test/TestExampleNumbers.cs +++ b/csharp/PhoneNumbers.Test/TestExampleNumbers.cs @@ -15,6 +15,7 @@ */ using System.Collections.Generic; +using System.Linq; using Xunit; namespace PhoneNumbers.Test @@ -162,10 +163,9 @@ public void TestSharedCost() [Fact] public void TestGlobalNetworkNumbers() { - foreach(var callingCode in phoneNumberUtil.GetSupportedGlobalNetworkCallingCodes()) + foreach (var exampleNumber in phoneNumberUtil.GetSupportedGlobalNetworkCallingCodes() + .Select(callingCode => phoneNumberUtil.GetExampleNumberForNonGeoEntity(callingCode))) { - var exampleNumber = - phoneNumberUtil.GetExampleNumberForNonGeoEntity(callingCode); Assert.NotNull(exampleNumber); if (!phoneNumberUtil.IsValidNumber(exampleNumber)) { diff --git a/csharp/PhoneNumbers.Test/TestMetadataFilter.cs b/csharp/PhoneNumbers.Test/TestMetadataFilter.cs index 751d46b36..338f9ee9d 100644 --- a/csharp/PhoneNumbers.Test/TestMetadataFilter.cs +++ b/csharp/PhoneNumbers.Test/TestMetadataFilter.cs @@ -396,15 +396,7 @@ public void testParseFieldMapFromString_RuntimeExceptionCases() Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("")); // Whitespace input. - try - { - MetadataFilter.ParseFieldMapFromString(" "); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => MetadataFilter.ParseFieldMapFromString(" ")); // Bad token given as only group. Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("something_else")); @@ -413,16 +405,8 @@ public void testParseFieldMapFromString_RuntimeExceptionCases() Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("fixedLine:something_else")); // Bad token given as middle group. - try - { - MetadataFilter.ParseFieldMapFromString( - "pager:nationalPrefix:something_else:nationalNumberPattern"); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => MetadataFilter.ParseFieldMapFromString( + "pager:nationalPrefix:something_else:nationalNumberPattern")); // Childless field given as parent. Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("nationalPrefix(exampleNumber)")); @@ -458,43 +442,19 @@ public void testParseFieldMapFromString_RuntimeExceptionCases() Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("(exampleNumber)")); // Whitespace parent. - try - { - MetadataFilter.ParseFieldMapFromString(" (exampleNumber)"); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => MetadataFilter.ParseFieldMapFromString(" (exampleNumber)")); // Empty child. Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("fixedLine()")); // Whitespace child. - try - { - MetadataFilter.ParseFieldMapFromString("fixedLine( )"); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("fixedLine( )")); // Empty parent and child. Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("()")); // Whitespace parent and empty child. - try - { - MetadataFilter.ParseFieldMapFromString(" ()"); - Assert.True(false); - } - catch (Exception) - { - // Test passed. - } + Assert.Throws(() => MetadataFilter.ParseFieldMapFromString(" ()")); // Parent field given as a group twice. Assert.Throws(() => MetadataFilter.ParseFieldMapFromString("fixedLine:uan:fixedLine")); diff --git a/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs b/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs index 989ff8ed2..36c7da490 100644 --- a/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs +++ b/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs @@ -220,24 +220,24 @@ public void TestMatchWithSurroundingZipcodes() var zipPreceding = "My address is CA 34215 - " + number + " is my number."; var expectedResult = phoneUtil.Parse(number, "US"); - var iterator = phoneUtil.FindNumbers(zipPreceding, "US").GetEnumerator(); - var match = iterator.MoveNext() ? iterator.Current : null; - Assert.NotNull(match); - Assert.Equal(expectedResult, match.Number); - Assert.Equal(number, match.RawString); - iterator.Dispose(); + using (var iterator = phoneUtil.FindNumbers(zipPreceding, "US").GetEnumerator()) + { + var match = iterator.MoveNext() ? iterator.Current : null; + Assert.NotNull(match); + Assert.Equal(expectedResult, match.Number); + Assert.Equal(number, match.RawString); + } // Now repeat, but this time the phone number has spaces in it. It should still be found. number = "(415) 666 7777"; var zipFollowing = "My number is " + number + ". 34215 is my zip-code."; - iterator = phoneUtil.FindNumbers(zipFollowing, "US").GetEnumerator(); + using var iterator2 = phoneUtil.FindNumbers(zipFollowing, "US").GetEnumerator(); - var matchWithSpaces = iterator.MoveNext() ? iterator.Current : null; + var matchWithSpaces = iterator2.MoveNext() ? iterator2.Current : null; Assert.NotNull(matchWithSpaces); Assert.Equal(expectedResult, matchWithSpaces.Number); Assert.Equal(number, matchWithSpaces.RawString); - iterator.Dispose(); } [Fact] @@ -451,12 +451,11 @@ public void TestMatchesMultiplePhoneNumbersSeparatedByPhoneNumberPunctuation() .Build(); var match2 = new PhoneNumberMatch(21, "455-234-3451", number2); - var matches = phoneUtil.FindNumbers(text, region).GetEnumerator(); + using var matches = phoneUtil.FindNumbers(text, region).GetEnumerator(); matches.MoveNext(); Assert.Equal(match1, matches.Current); matches.MoveNext(); Assert.Equal(match2, matches.Current); - matches.Dispose(); } [Fact] @@ -716,9 +715,8 @@ private void FindMatchesInContexts(List contexts, bool isValid, } else { - foreach (var context in contexts) + foreach (var text in contexts.Select(context => context.LeadingText + number + context.TrailingText)) { - var text = context.LeadingText + number + context.TrailingText; Assert.True(HasNoMatches(phoneUtil.FindNumbers(text, region)), "Should not have found a number in " + text); } @@ -729,9 +727,8 @@ private void FindMatchesInContexts(List contexts, bool isValid, } else { - foreach (var context in contexts) + foreach (var text in contexts.Select(context => context.LeadingText + number + context.TrailingText)) { - var text = context.LeadingText + number + context.TrailingText; Assert.True(HasNoMatches(phoneUtil.FindNumbers(text, region, PhoneNumberUtil.Leniency.POSSIBLE, long.MaxValue)), "Should not have found a number in " + text); @@ -811,12 +808,11 @@ public void TestSequences() .SetNationalNumber(32316005).Build(); var match2 = new PhoneNumberMatch(19, "032316005", number2); - var matches = phoneUtil.FindNumbers(text, region, PhoneNumberUtil.Leniency.POSSIBLE, long.MaxValue).GetEnumerator(); + using var matches = phoneUtil.FindNumbers(text, region, PhoneNumberUtil.Leniency.POSSIBLE, long.MaxValue).GetEnumerator(); matches.MoveNext(); Assert.Equal(match1, matches.Current); matches.MoveNext(); Assert.Equal(match2, matches.Current); - matches.Dispose(); } [Fact] @@ -887,11 +883,10 @@ public void TestNonPlusPrefixedNumbersNotFoundForInvalidRegion() { // Does not start with a "+", we won't match it. var iterable = phoneUtil.FindNumbers("1 456 764 156", RegionCode.ZZ); - var iterator = iterable.GetEnumerator(); + using var iterator = iterable.GetEnumerator(); Assert.False(iterator.MoveNext()); Assert.False(iterator.MoveNext()); - iterator.Dispose(); } @@ -899,11 +894,10 @@ public void TestNonPlusPrefixedNumbersNotFoundForInvalidRegion() public void TestEmptyIteration() { var iterable = phoneUtil.FindNumbers("", "ZZ"); - var iterator = iterable.GetEnumerator(); + using var iterator = iterable.GetEnumerator(); Assert.False(iterator.MoveNext()); Assert.False(iterator.MoveNext()); - iterator.Dispose(); } [Fact] @@ -913,18 +907,18 @@ public void TestSingleIteration() var iterable = phoneUtil.FindNumbers("+14156667777", "ZZ"); // With hasNext() -> next(). - var iterator = iterable.GetEnumerator(); - // Double hasNext() to ensure it does not advance. - Assert.True(iterator.MoveNext()); - Assert.NotNull(iterator.Current); - Assert.False(iterator.MoveNext()); - iterator.Dispose(); + using (var iterator = iterable.GetEnumerator()) + { + // Double hasNext() to ensure it does not advance. + Assert.True(iterator.MoveNext()); + Assert.NotNull(iterator.Current); + Assert.False(iterator.MoveNext()); + } // With next() only. - iterator = iterable.GetEnumerator(); - Assert.True(iterator.MoveNext()); - Assert.False(iterator.MoveNext()); - iterator.Dispose(); + using var iterator2 = iterable.GetEnumerator(); + Assert.True(iterator2.MoveNext()); + Assert.False(iterator2.MoveNext()); } /// @@ -935,14 +929,13 @@ public void TestSingleIteration() private void AssertEqualRange(string text, int index, int start, int end) { var sub = text.Substring(index); - var matches = + using var matches = phoneUtil.FindNumbers(sub, "NZ", PhoneNumberUtil.Leniency.POSSIBLE, long.MaxValue).GetEnumerator(); Assert.True(matches.MoveNext()); var match = matches.Current; Assert.Equal(start - index, match.Start); Assert.Equal(end - start, match.Length); Assert.Equal(sub.Substring(match.Start, match.Length), match.RawString); - matches.Dispose(); } /// @@ -1065,10 +1058,10 @@ private void EnsureTermination(string text, string defaultCountry, PhoneNumberUt for (var index = 0; index <= text.Length; index++) { var sub = text.Substring(index); - var matches = new StringBuilder(); - // Iterates over all matches. - foreach (var match in phoneUtil.FindNumbers(sub, defaultCountry, leniency, long.MaxValue)) - matches.Append(", ").Append(match); + foreach (var _ in phoneUtil.FindNumbers(sub, defaultCountry, leniency, long.MaxValue)) + { + // Iterates over all matches to ensure that doing so terminates. + } } } diff --git a/csharp/PhoneNumbers.Test/TestPhoneNumberToTimeZonesMapper.cs b/csharp/PhoneNumbers.Test/TestPhoneNumberToTimeZonesMapper.cs index bb8df038d..a20f2fd7b 100644 --- a/csharp/PhoneNumbers.Test/TestPhoneNumberToTimeZonesMapper.cs +++ b/csharp/PhoneNumbers.Test/TestPhoneNumberToTimeZonesMapper.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Xunit; namespace PhoneNumbers.Test @@ -23,7 +24,7 @@ namespace PhoneNumbers.Test [Collection("TestMetadataTestCase")] public class TestPhoneNumberToTimeZonesMapper { - private static long[][] numbers = + private static readonly long[][] numbers = { new long[] { 45, 35353535L }, // denmark new long[] { 45, 53831292L }, @@ -101,8 +102,8 @@ public void TestMapDataReader() var map = TimezoneMapDataReader.GetPrefixMap(ms, ianaTZListDelimiter); Assert.NotNull(map); Assert.Equal(11, map.Count); - Assert.True(map.ContainsKey(1)); - Assert.True(1 < map[1].Length); + Assert.True(map.TryGetValue(1, out var timezones)); + Assert.True(1 < timezones.Length); } } @@ -112,9 +113,8 @@ public void TestMapper() var mapper = PhoneNumberToTimeZonesMapper.GetInstance(); Assert.Same(mapper, PhoneNumberToTimeZonesMapper.GetInstance()); Assert.Equal("Etc/Unknown", mapper.GetUnknownTimeZone()); - foreach (var pn in testNumbers) + foreach (var res0 in testNumbers.Select(pn => mapper.GetTimeZonesForNumber(pn))) { - var res0 = mapper.GetTimeZonesForNumber(pn); Assert.NotEmpty(res0); } } @@ -161,9 +161,8 @@ public void TestMapperOutcomes(int countryCode, long nationalNumber, bool hasTim public void TestMapperWithNoData() { var emptyMapper = new PhoneNumberToTimeZonesMapper(new Dictionary()); - foreach (var pn in testNumbers) + foreach (var list in testNumbers.Select(pn => emptyMapper.GetTimeZonesForNumber(pn))) { - var list = emptyMapper.GetTimeZonesForNumber(pn); Assert.NotNull(list); Assert.Single(list); Assert.Equal("Etc/Unknown", list[0]); @@ -179,13 +178,12 @@ public void TestMapperWithBadData() var map = TimezoneMapDataReader.GetPrefixMap(ms, new char[] { '&' }); Assert.NotNull(map); Assert.Equal(11, map.Count); - Assert.True(map.ContainsKey(1)); - Assert.True(1 < map[1].Length); + Assert.True(map.TryGetValue(1, out var timezones)); + Assert.True(1 < timezones.Length); var wrongMapper = new PhoneNumberToTimeZonesMapper(map); - foreach (var pn in testNumbers) + foreach (var list in testNumbers.Select(pn => wrongMapper.GetTimeZonesForNumber(pn))) { - var list = wrongMapper.GetTimeZonesForNumber(pn); Assert.NotNull(list); Assert.NotEmpty(list); } @@ -258,7 +256,7 @@ private static PhoneNumberToTimeZonesMapper CreateTestMapper() } } - private static string MapTestData = @"# Copyright (C) 2012 The Libphonenumber Authors + private static readonly string MapTestData = @"# Copyright (C) 2012 The Libphonenumber Authors # Licensed under the Apache License, Version 2.0 (the ""License""); # you may not use this file except in compliance with the License. diff --git a/csharp/PhoneNumbers/BuildMetadataFromXml.cs b/csharp/PhoneNumbers/BuildMetadataFromXml.cs index a97bd7468..20e53ffa0 100644 --- a/csharp/PhoneNumbers/BuildMetadataFromXml.cs +++ b/csharp/PhoneNumbers/BuildMetadataFromXml.cs @@ -623,14 +623,13 @@ private static void SetPossibleLengths(SortedSet lengths, // We check that the local-only length isn't also a normal possible length (only relevant for // the general-desc, since within elements such as fixed-line we would throw an exception if we // saw this) before adding it to the collection of possible local-only lengths. - foreach (var length in localOnlyLengths) - if (!lengths.Contains(length)) - if (parentDesc is null || parentDesc.possibleLengthLocalOnly_.Contains(length) - || parentDesc.possibleLength_.Contains(length)) - desc.possibleLengthLocalOnly_.Add(length); - else - throw new Exception( - $"Out-of-range local-only possible length found ({length}), parent length {string.Join(", ", parentDesc.PossibleLengthLocalOnlyList)}."); + foreach (var length in localOnlyLengths.Where(length => !lengths.Contains(length))) + if (parentDesc is null || parentDesc.possibleLengthLocalOnly_.Contains(length) + || parentDesc.possibleLength_.Contains(length)) + desc.possibleLengthLocalOnly_.Add(length); + else + throw new Exception( + $"Out-of-range local-only possible length found ({length}), parent length {string.Join(", ", parentDesc.PossibleLengthLocalOnlyList)}."); } diff --git a/csharp/PhoneNumbers/MetadataFilter.cs b/csharp/PhoneNumbers/MetadataFilter.cs index 5c6219ff6..95617c8a7 100644 --- a/csharp/PhoneNumbers/MetadataFilter.cs +++ b/csharp/PhoneNumbers/MetadataFilter.cs @@ -299,16 +299,12 @@ internal static Dictionary> ComputeComplement( // parent as a key. if (otherChildren.Count != ExcludableChildFields.Count) { - var children = new SortedSet(); - foreach (var child in ExcludableChildFields) - if (!otherChildren.Contains(child)) - children.Add(child); + var children = new SortedSet(ExcludableChildFields.Where(child => !otherChildren.Contains(child))); complement.Add(parent, children); } } - foreach (var childlessField in ExcludableChildlessFields) - if (!fieldMap.ContainsKey(childlessField)) - complement.Add(childlessField, new SortedSet()); + foreach (var childlessField in ExcludableChildlessFields.Where(f => !fieldMap.ContainsKey(f))) + complement.Add(childlessField, new SortedSet()); return complement; } @@ -318,7 +314,7 @@ internal bool ShouldDrop(string parent, string child) throw new Exception(parent + " is not an excludable parent field"); if (!ExcludableChildFields.Contains(child)) throw new Exception(child + " is not an excludable child field"); - return blacklist.ContainsKey(parent) && blacklist[parent].Contains(child); + return blacklist.TryGetValue(parent, out var children) && children.Contains(child); } internal bool ShouldDrop(string childlessField) diff --git a/csharp/PhoneNumbers/PhoneNumberDesc.cs b/csharp/PhoneNumbers/PhoneNumberDesc.cs index 25675c48e..3bc8ca586 100644 --- a/csharp/PhoneNumbers/PhoneNumberDesc.cs +++ b/csharp/PhoneNumbers/PhoneNumberDesc.cs @@ -246,8 +246,8 @@ public override int GetHashCode() var hash = GetType().GetHashCode(); if (HasNationalNumberPattern) hash ^= NationalNumberPattern.GetHashCode(); - hash = possibleLength_.Aggregate(hash, (current, i) => current ^ i.GetHashCode()); - hash = possibleLengthLocalOnly_.Aggregate(hash, (current, i) => current ^ i.GetHashCode()); + hash = possibleLength_.Aggregate(hash, (current, i) => current ^ i); + hash = possibleLengthLocalOnly_.Aggregate(hash, (current, i) => current ^ i); if (HasExampleNumber) hash ^= ExampleNumber.GetHashCode(); return hash; diff --git a/csharp/PhoneNumbers/PhoneNumberMatch.cs b/csharp/PhoneNumbers/PhoneNumberMatch.cs index 061b23911..eac1d6248 100644 --- a/csharp/PhoneNumbers/PhoneNumberMatch.cs +++ b/csharp/PhoneNumbers/PhoneNumberMatch.cs @@ -59,7 +59,7 @@ public override bool Equals(object obj) public override int GetHashCode() { var hash = GetType().GetHashCode(); - hash ^= Start.GetHashCode(); + hash ^= Start; hash ^= RawString.GetHashCode(); hash ^= Number.GetHashCode(); return hash; diff --git a/csharp/PhoneNumbers/PhoneNumberMatcher.cs b/csharp/PhoneNumbers/PhoneNumberMatcher.cs index 5694a996a..f44cfaf23 100644 --- a/csharp/PhoneNumbers/PhoneNumberMatcher.cs +++ b/csharp/PhoneNumbers/PhoneNumberMatcher.cs @@ -218,7 +218,7 @@ public static bool IsLatinLetter(char letter) if (!char.IsLetter(letter) && CharUnicodeInfo.GetUnicodeCategory(letter) != UnicodeCategory.NonSpacingMark) return false; return - letter >= 0x0000 && letter <= 0x007F // BASIC_LATIN + letter <= 0x007F // BASIC_LATIN || letter >= 0x0080 && letter <= 0x00FF // LATIN_1_SUPPLEMENT || letter >= 0x0100 && letter <= 0x017F // LATIN_EXTENDED_A || letter >= 0x1E00 && letter <= 0x1EFF // LATIN_EXTENDED_ADDITIONAL diff --git a/csharp/PhoneNumbers/PhoneNumberOfflineGeocoder.cs b/csharp/PhoneNumbers/PhoneNumberOfflineGeocoder.cs index 9ddeaebd8..1db26da64 100644 --- a/csharp/PhoneNumbers/PhoneNumberOfflineGeocoder.cs +++ b/csharp/PhoneNumbers/PhoneNumberOfflineGeocoder.cs @@ -17,6 +17,7 @@ using System; using System.Globalization; +using System.Linq; using System.Reflection; namespace PhoneNumbers @@ -146,19 +147,15 @@ private string GetCountryNameForNumber(PhoneNumber number, Locale language) { return GetRegionDisplayName(regionCodes[0], language); } - var regionWhereNumberIsValid = "ZZ"; - foreach (var regionCode in regionCodes) - { - if (phoneUtil.IsValidNumberForRegion(number, regionCode)) - { - // If the number has already been found valid for one region, then we don't know - // which region it belongs to so we return nothing. - if (regionWhereNumberIsValid != "ZZ") - return ""; - regionWhereNumberIsValid = regionCode; - } - } - return GetRegionDisplayName(regionWhereNumberIsValid, language); + // Take(2): once a second valid region turns up we already know the answer (below), so + // there's no need to keep testing the rest. + var validRegions = regionCodes.Where(regionCode => phoneUtil.IsValidNumberForRegion(number, regionCode)) + .Take(2).ToList(); + // If the number is valid for more than one region, we don't know which region it belongs + // to so we return nothing. + if (validRegions.Count > 1) + return ""; + return GetRegionDisplayName(validRegions.Count == 1 ? validRegions[0] : "ZZ", language); } /// diff --git a/csharp/PhoneNumbers/PhoneNumberUtil.cs b/csharp/PhoneNumbers/PhoneNumberUtil.cs index aeb84ba55..72fe506b3 100644 --- a/csharp/PhoneNumbers/PhoneNumberUtil.cs +++ b/csharp/PhoneNumbers/PhoneNumberUtil.cs @@ -1312,16 +1312,11 @@ public string FormatNumberForMobileDialing(PhoneNumber number, string regionCall // internationally, since that always works, except for numbers which might potentially be // short numbers, which are always dialled in national format. var regionMetadata = GetMetadataForRegion(regionCallingFrom); - if (CanBeInternationallyDialled(numberNoExt) + formattedNumber = CanBeInternationallyDialled(numberNoExt) && TestNumberLength(GetNationalSignificantNumberLength(numberNoExt), regionMetadata) - != ValidationResult.TOO_SHORT) - { - formattedNumber = Format(numberNoExt, PhoneNumberFormat.INTERNATIONAL); - } - else - { - formattedNumber = Format(numberNoExt, PhoneNumberFormat.NATIONAL); - } + != ValidationResult.TOO_SHORT + ? Format(numberNoExt, PhoneNumberFormat.INTERNATIONAL) + : Format(numberNoExt, PhoneNumberFormat.NATIONAL); } else { @@ -1579,13 +1574,13 @@ internal NumberFormat ChooseFormattingPatternForNumber(List availa foreach (var numFormat in availableFormats) { var size = numFormat.LeadingDigitsPatternCount; - if (size == 0 || PhoneRegex.Get( + if ((size == 0 || PhoneRegex.Get( // We always use the last leading_digits_pattern, as it is the most detailed. numFormat.GetLeadingDigitsPattern(size - 1)) .IsMatchBeginning(nationalNumber)) + && PhoneRegex.Get(numFormat.Pattern).IsMatchAll(nationalNumber)) { - if (PhoneRegex.Get(numFormat.Pattern).IsMatchAll(nationalNumber)) - return numFormat; + return numFormat; } } return null; @@ -1670,6 +1665,8 @@ public PhoneNumber GetExampleNumberForType(string regionCode, PhoneNumberType ty } catch (NumberParseException) { + // The example number in the metadata failed to parse; treat it the same as no example + // number being present rather than surfacing a metadata-quality issue to the caller. } return null; } @@ -2568,7 +2565,9 @@ public PhoneNumber.Types.CountryCodeSource MaybeStripInternationalPrefixAndNorma } // Attempt to parse the first digits as an international prefix. Normalize(number); - if (possibleIddPrefix as object == "NonMatch" as object) + // Reference equality, not value equality: this only matches the specific sentinel object + // assigned above, not any region's real prefix that happens to equal the literal text. + if (ReferenceEquals(possibleIddPrefix, "NonMatch")) return PhoneNumber.Types.CountryCodeSource.FROM_DEFAULT_COUNTRY; var iddPattern = PhoneRegex.Get(possibleIddPrefix); diff --git a/csharp/PhoneNumbers/Phonemetadata.cs b/csharp/PhoneNumbers/Phonemetadata.cs index 4c1b582e1..23e9b8044 100644 --- a/csharp/PhoneNumbers/Phonemetadata.cs +++ b/csharp/PhoneNumbers/Phonemetadata.cs @@ -556,11 +556,10 @@ public Builder SetGeneralDesc(PhoneNumberDesc.Builder builderForValue) public Builder MergeGeneralDesc(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasGeneralDesc && - MessageBeingBuilt.GeneralDesc != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.GeneralDesc = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.GeneralDesc) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.GeneralDesc = value; + MessageBeingBuilt.GeneralDesc = MessageBeingBuilt.HasGeneralDesc && + MessageBeingBuilt.GeneralDesc != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.GeneralDesc).MergeFrom(value).BuildPartial() + : value; return this; } @@ -587,11 +586,10 @@ public Builder SetFixedLine(PhoneNumberDesc.Builder builderForValue) public Builder MergeFixedLine(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasFixedLine && - MessageBeingBuilt.FixedLine != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.FixedLine = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.FixedLine) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.FixedLine = value; + MessageBeingBuilt.FixedLine = MessageBeingBuilt.HasFixedLine && + MessageBeingBuilt.FixedLine != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.FixedLine).MergeFrom(value).BuildPartial() + : value; return this; } @@ -618,11 +616,10 @@ public Builder SetMobile(PhoneNumberDesc.Builder builderForValue) public Builder MergeMobile(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasMobile && - MessageBeingBuilt.Mobile != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.Mobile = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Mobile).MergeFrom(value) - .BuildPartial(); - else MessageBeingBuilt.Mobile = value; + MessageBeingBuilt.Mobile = MessageBeingBuilt.HasMobile && + MessageBeingBuilt.Mobile != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Mobile).MergeFrom(value).BuildPartial() + : value; return this; } @@ -649,11 +646,10 @@ public Builder SetTollFree(PhoneNumberDesc.Builder builderForValue) public Builder MergeTollFree(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasTollFree && - MessageBeingBuilt.TollFree != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.TollFree = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.TollFree) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.TollFree = value; + MessageBeingBuilt.TollFree = MessageBeingBuilt.HasTollFree && + MessageBeingBuilt.TollFree != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.TollFree).MergeFrom(value).BuildPartial() + : value; return this; } @@ -680,11 +676,10 @@ public Builder SetPremiumRate(PhoneNumberDesc.Builder builderForValue) public Builder MergePremiumRate(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasPremiumRate && - MessageBeingBuilt.PremiumRate != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.PremiumRate = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.PremiumRate) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.PremiumRate = value; + MessageBeingBuilt.PremiumRate = MessageBeingBuilt.HasPremiumRate && + MessageBeingBuilt.PremiumRate != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.PremiumRate).MergeFrom(value).BuildPartial() + : value; return this; } @@ -711,11 +706,10 @@ public Builder SetSharedCost(PhoneNumberDesc.Builder builderForValue) public Builder MergeSharedCost(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasSharedCost && - MessageBeingBuilt.SharedCost != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.SharedCost = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.SharedCost) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.SharedCost = value; + MessageBeingBuilt.SharedCost = MessageBeingBuilt.HasSharedCost && + MessageBeingBuilt.SharedCost != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.SharedCost).MergeFrom(value).BuildPartial() + : value; return this; } @@ -742,11 +736,10 @@ public Builder SetPersonalNumber(PhoneNumberDesc.Builder builderForValue) public Builder MergePersonalNumber(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasPersonalNumber && - MessageBeingBuilt.PersonalNumber != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.PersonalNumber = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.PersonalNumber) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.PersonalNumber = value; + MessageBeingBuilt.PersonalNumber = MessageBeingBuilt.HasPersonalNumber && + MessageBeingBuilt.PersonalNumber != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.PersonalNumber).MergeFrom(value).BuildPartial() + : value; return this; } @@ -773,11 +766,10 @@ public Builder SetVoip(PhoneNumberDesc.Builder builderForValue) public Builder MergeVoip(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasVoip && - MessageBeingBuilt.Voip != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.Voip = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Voip).MergeFrom(value) - .BuildPartial(); - else MessageBeingBuilt.Voip = value; + MessageBeingBuilt.Voip = MessageBeingBuilt.HasVoip && + MessageBeingBuilt.Voip != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Voip).MergeFrom(value).BuildPartial() + : value; return this; } @@ -804,11 +796,10 @@ public Builder SetPager(PhoneNumberDesc.Builder builderForValue) public Builder MergePager(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasPager && - MessageBeingBuilt.Pager != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.Pager = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Pager).MergeFrom(value) - .BuildPartial(); - else MessageBeingBuilt.Pager = value; + MessageBeingBuilt.Pager = MessageBeingBuilt.HasPager && + MessageBeingBuilt.Pager != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Pager).MergeFrom(value).BuildPartial() + : value; return this; } @@ -835,11 +826,10 @@ public Builder SetUan(PhoneNumberDesc.Builder builderForValue) public Builder MergeUan(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasUan && - MessageBeingBuilt.Uan != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.Uan = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Uan).MergeFrom(value) - .BuildPartial(); - else MessageBeingBuilt.Uan = value; + MessageBeingBuilt.Uan = MessageBeingBuilt.HasUan && + MessageBeingBuilt.Uan != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Uan).MergeFrom(value).BuildPartial() + : value; return this; } @@ -866,11 +856,10 @@ public Builder SetEmergency(PhoneNumberDesc.Builder builderForValue) public Builder MergeEmergency(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasEmergency && - MessageBeingBuilt.Emergency != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.Emergency = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Emergency) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.Emergency = value; + MessageBeingBuilt.Emergency = MessageBeingBuilt.HasEmergency && + MessageBeingBuilt.Emergency != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Emergency).MergeFrom(value).BuildPartial() + : value; return this; } @@ -897,11 +886,10 @@ public Builder SetVoicemail(PhoneNumberDesc.Builder builderForValue) public Builder MergeVoicemail(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasVoicemail && - MessageBeingBuilt.Voicemail != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.Voicemail = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Voicemail) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.Voicemail = value; + MessageBeingBuilt.Voicemail = MessageBeingBuilt.HasVoicemail && + MessageBeingBuilt.Voicemail != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.Voicemail).MergeFrom(value).BuildPartial() + : value; return this; } @@ -928,11 +916,10 @@ public Builder SetShortCode(PhoneNumberDesc.Builder builderForValue) public Builder MergeShortCode(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasShortCode && - MessageBeingBuilt.ShortCode != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.ShortCode = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.ShortCode) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.ShortCode = value; + MessageBeingBuilt.ShortCode = MessageBeingBuilt.HasShortCode && + MessageBeingBuilt.ShortCode != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.ShortCode).MergeFrom(value).BuildPartial() + : value; return this; } @@ -959,11 +946,10 @@ public Builder SetStandardRate(PhoneNumberDesc.Builder builderForValue) public Builder MergeStandardRate(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasStandardRate && - MessageBeingBuilt.StandardRate != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.StandardRate = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.StandardRate) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.StandardRate = value; + MessageBeingBuilt.StandardRate = MessageBeingBuilt.HasStandardRate && + MessageBeingBuilt.StandardRate != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.StandardRate).MergeFrom(value).BuildPartial() + : value; return this; } @@ -990,11 +976,10 @@ public Builder SetCarrierSpecific(PhoneNumberDesc.Builder builderForValue) public Builder MergeCarrierSpecific(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasCarrierSpecific && - MessageBeingBuilt.CarrierSpecific != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.CarrierSpecific = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.CarrierSpecific) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.CarrierSpecific = value; + MessageBeingBuilt.CarrierSpecific = MessageBeingBuilt.HasCarrierSpecific && + MessageBeingBuilt.CarrierSpecific != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.CarrierSpecific).MergeFrom(value).BuildPartial() + : value; return this; } @@ -1021,11 +1006,10 @@ public Builder SetSmsServices(PhoneNumberDesc.Builder builderForValue) public Builder MergeSmsServices(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasSmsServices && - MessageBeingBuilt.SmsServices != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.SmsServices = PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.SmsServices) - .MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.SmsServices = value; + MessageBeingBuilt.SmsServices = MessageBeingBuilt.HasSmsServices && + MessageBeingBuilt.SmsServices != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.SmsServices).MergeFrom(value).BuildPartial() + : value; return this; } @@ -1052,11 +1036,10 @@ public Builder SetNoInternationalDialling(PhoneNumberDesc.Builder builderForValu public Builder MergeNoInternationalDialling(PhoneNumberDesc value) { if (value is null) throw new ArgumentNullException(nameof(value)); - if (MessageBeingBuilt.HasNoInternationalDialling && - MessageBeingBuilt.NoInternationalDialling != PhoneNumberDesc.DefaultInstance) - MessageBeingBuilt.NoInternationalDialling = PhoneNumberDesc - .CreateBuilder(MessageBeingBuilt.NoInternationalDialling).MergeFrom(value).BuildPartial(); - else MessageBeingBuilt.NoInternationalDialling = value; + MessageBeingBuilt.NoInternationalDialling = MessageBeingBuilt.HasNoInternationalDialling && + MessageBeingBuilt.NoInternationalDialling != PhoneNumberDesc.DefaultInstance + ? PhoneNumberDesc.CreateBuilder(MessageBeingBuilt.NoInternationalDialling).MergeFrom(value).BuildPartial() + : value; return this; } @@ -1333,7 +1316,7 @@ public override int GetHashCode() if (HasSmsServices) hash ^= SmsServices.GetHashCode(); if (HasNoInternationalDialling) hash ^= NoInternationalDialling.GetHashCode(); if (HasId) hash ^= Id.GetHashCode(); - if (HasCountryCode) hash ^= CountryCode.GetHashCode(); + if (HasCountryCode) hash ^= CountryCode; if (HasInternationalPrefix) hash ^= InternationalPrefix.GetHashCode(); if (HasPreferredInternationalPrefix) hash ^= PreferredInternationalPrefix.GetHashCode(); if (HasNationalPrefix) hash ^= NationalPrefix.GetHashCode(); diff --git a/csharp/PhoneNumbers/ShortNumberInfo.cs b/csharp/PhoneNumbers/ShortNumberInfo.cs index 8fc02c8e7..2789cb2b2 100644 --- a/csharp/PhoneNumbers/ShortNumberInfo.cs +++ b/csharp/PhoneNumbers/ShortNumberInfo.cs @@ -327,9 +327,8 @@ public ShortNumberCost GetExpectedCost(PhoneNumber number) } var cost = ShortNumberCost.TOLL_FREE; - foreach (var regionCode in regionCodes) + foreach (var costForRegion in regionCodes.Select(regionCode => GetExpectedCostForRegion(number, regionCode))) { - var costForRegion = GetExpectedCostForRegion(number, regionCode); switch (costForRegion) { case ShortNumberCost.PREMIUM_RATE: