From 3be1de2ec6f07693fcdfd66f07b750c990adc501 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:17:56 -0500 Subject: [PATCH 01/15] ci: allow manual dispatch of the CodeQL full-repo scan Lets a full-repo CodeQL run be triggered on demand from the Actions tab instead of waiting for a push to main or the weekly schedule. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/codeql.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 7734fe6e..beab66ff 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 }} From 7b6c079f1bc85930d958c175ef69fa28ead585a3 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:57:18 -0500 Subject: [PATCH 02/15] style: harden Path.Combine and simplify up-to-date checks in MetadataBuilder - Wrap the filename-derived second argument to Path.Combine in Path.GetFileName() in both output-path builders, so an unexpected path separator or rooted segment can never make Path.Combine silently drop the output directory (cs/path-combine, alerts #249/#250). - IsOutputUpToDate/IsGeocodingOutputUpToDate: both had a foreach loop that was really an Any() check (early-return on first match), and IsGeocodingOutputUpToDate had a separate foreach that was really a Select+Max fold. Replaced both with the LINQ equivalents (cs/linq/missed-where, alerts #13/#14; cs/linq/missed-select, alert #17). Left the top-level catch (Exception) in Main alone (cs/catch-of-all-exceptions, alert #275): it's the standard CLI entry-point idiom, logs to stderr and returns a non-zero exit code rather than swallowing anything. Co-Authored-By: Claude Sonnet 5 --- .../PhoneNumbers.MetadataBuilder/Program.cs | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/csharp/PhoneNumbers.MetadataBuilder/Program.cs b/csharp/PhoneNumbers.MetadataBuilder/Program.cs index a47a3307..15f869c5 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.Combine(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.Combine(outputDir, Path.GetFileName($"{filePrefix}_{key}")); using var gz = new GZipStream(File.Create(path), CompressionLevel.SmallestSize); BuildMetadataFromBin.WriteMetadata(gz, metadata); written++; From 582bbe5d3aa37479835c7f9ebaf268fa99b99a2d Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:59:15 -0500 Subject: [PATCH 03/15] style: LINQ filters and TryGetValue in MetadataFilter/PhoneNumberOfflineGeocoder MetadataFilter.cs: - ComputeComplement's two filter-and-collect loops rewritten with .Where() (cs/linq/missed-where, alerts #10/#11). - ShouldDrop: TryGetValue instead of ContainsKey+indexer, avoiding the double dictionary lookup (cs/inefficient-containskey, alert #25). PhoneNumberOfflineGeocoder.cs: GetCountryNameForNumber's loop wasn't a pure filter (it early-returns once a *second* valid region turns up), so a naive .Where().ToList() would lose that short-circuit and evaluate IsValidNumberForRegion for every remaining region needlessly. .Where().Take(2) preserves the same lazy stopping point while still expressing the filter explicitly (cs/linq/missed-where, alert #12). Left PrefixFileReader.cs's LoadFileNamesFromManifestResources alone (cs/linq/missed-select, alert #15): it's a multi-step parse with three early-continue guard clauses and dictionary mutation, not a pure map; forcing it into .Select() would reduce clarity, not improve it. Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/MetadataFilter.cs | 12 ++++------ .../PhoneNumberOfflineGeocoder.cs | 23 ++++++++----------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/csharp/PhoneNumbers/MetadataFilter.cs b/csharp/PhoneNumbers/MetadataFilter.cs index ac9f62cb..3ae658bf 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/PhoneNumberOfflineGeocoder.cs b/csharp/PhoneNumbers/PhoneNumberOfflineGeocoder.cs index 82b6ef13..b5acfb60 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); } /// From d10226649eb0540b69b42334660b72a74eb72c13 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 10:03:15 -0500 Subject: [PATCH 04/15] test: exception-safe enumerator disposal, LINQ selects, drop dead StringBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 9 spots created a match-iterator with manual .Dispose() after assertions that can throw on failure, leaking the enumerator on the failure path. Switched to using/using-var, in some cases splitting a method into scoped blocks so each of two sequential enumerators is still disposed promptly, matching the original ordering (cs/dispose-not-called-on-throw, alerts #361-#369). - FindMatchesInContexts: two foreach loops that immediately mapped context -> text and never used context again, rewritten as .Select() (cs/linq/missed-select, alerts #19/#20). - EnsureTermination: the per-iteration StringBuilder was write-only — appended to but never read, returned, or asserted against. The method's whole point (per its doc comment) is forcing full enumeration to confirm it terminates; the StringBuilder was dead weight, not an allocation to hoist out of the loop (cs/stringbuilder-creation-in-loop, alert #24). Co-Authored-By: Claude Sonnet 5 --- .../TestPhoneNumberMatcher.cs | 67 +++++++++---------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs b/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs index 989ff8ed..50774aaa 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); + // Iterates over all matches to ensure that doing so terminates. + foreach (var _ in phoneUtil.FindNumbers(sub, defaultCountry, leniency, long.MaxValue)) + { + } } } From f8117252045b796878cfdbc553e32556a8746298 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 10:05:19 -0500 Subject: [PATCH 05/15] test: use Assert.Throws instead of manual try/catch in TestParseFieldMapFromString 5 cases used a leftover try { ...; Assert.True(false); } catch (Exception) { } pattern where every other case in the same method already uses the cleaner Assert.Throws(() => ...) idiom for the exact same kind of assertion. Made these 5 consistent with the rest (cs/catch-of-all-exceptions, alerts #279-#283). Left TestEquals_WhenNull_ReturnsFalse alone (cs/null-argument-to-equals, alert #370): MetadataFilter.Equals uses `obj is not MetadataFilter other` pattern matching, which handles a null argument safely (no NRE) - the test is correctly verifying that exact contract, not an accidental risky Equals(null) call. Co-Authored-By: Claude Sonnet 5 --- .../PhoneNumbers.Test/TestMetadataFilter.cs | 52 +++---------------- 1 file changed, 6 insertions(+), 46 deletions(-) diff --git a/csharp/PhoneNumbers.Test/TestMetadataFilter.cs b/csharp/PhoneNumbers.Test/TestMetadataFilter.cs index 751d46b3..338f9ee9 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")); From d5ad5fdd89949adc5ac9e46b6c863e2fa654a323 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 10:06:26 -0500 Subject: [PATCH 06/15] test: use Assert.Throws instead of manual try/catch in TestBuildMetadataFromXml Same simplification as the TestMetadataFilter commit: 3 "should throw" assertions used a manual try/Assert.True(false)/catch(Exception) block instead of the standard xUnit Assert.Throws(...) idiom (cs/catch-of-all-exceptions, alerts #276-#278). Co-Authored-By: Claude Sonnet 5 --- .../TestBuildMetadataFromXml.cs | 33 ++++--------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/csharp/PhoneNumbers.Test/TestBuildMetadataFromXml.cs b/csharp/PhoneNumbers.Test/TestBuildMetadataFromXml.cs index d3a23e34..d0781e33 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(). From f0f23acc2cc5b7f3e1d4550f741372349b7a6109 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 10:07:51 -0500 Subject: [PATCH 07/15] test: readonly fields, TryGetValue, and LINQ selects in TestPhoneNumberToTimeZonesMapper - numbers and MapTestData are never reassigned; marked readonly (cs/missed-readonly-modifier, alerts #271/#272). - ContainsKey+indexer double lookups replaced with TryGetValue (cs/inefficient-containskey, alerts #29/#30). - Three foreach loops that immediately mapped pn -> a per-number result and never touched pn again, rewritten with .Select() (cs/linq/missed-select, alerts #21-#23). Co-Authored-By: Claude Sonnet 5 --- .../TestPhoneNumberToTimeZonesMapper.cs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/csharp/PhoneNumbers.Test/TestPhoneNumberToTimeZonesMapper.cs b/csharp/PhoneNumbers.Test/TestPhoneNumberToTimeZonesMapper.cs index bb8df038..a20f2fd7 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. From 1619543f6261badd75cc364f28a7d1d7d49cbf35 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 10:09:00 -0500 Subject: [PATCH 08/15] test: TryGetValue and LINQ select in TestBuildPrefixMapFromBin/TestExampleNumbers - TestBuildPrefixMapFromBin.cs: ContainsKey+indexer double lookups replaced with TryGetValue in both round-trip assertions (cs/inefficient-containskey, alerts #27/#28). - TestExampleNumbers.cs: TestGlobalNetworkNumbers' loop immediately mapped callingCode -> exampleNumber and never used callingCode again, rewritten with .Select() (cs/linq/missed-select, alert #18). Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs | 8 ++++---- csharp/PhoneNumbers.Test/TestExampleNumbers.cs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs b/csharp/PhoneNumbers.Test/TestBuildPrefixMapFromBin.cs index 4286c06e..ad6f92bc 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 6523a4f5..ac22cc63 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)) { From 6b9d5403e33b7940fdcae6d235093781e8ebd014 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:45:51 -0500 Subject: [PATCH 09/15] style: map region codes via Select in GetExpectedCost Removes the redundant costForRegion local by folding the mapping into the foreach's source sequence; the fold/early-return logic is unchanged. Addresses CodeQL cs/linq/missed-select (alert #16). Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/ShortNumberInfo.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/csharp/PhoneNumbers/ShortNumberInfo.cs b/csharp/PhoneNumbers/ShortNumberInfo.cs index 8fc02c8e..2789cb2b 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: From d413a88e97d79099a79d418f5858203da32d672c Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:47:35 -0500 Subject: [PATCH 10/15] style: filter local-only lengths explicitly via Where The loop's outer condition was a pure filter (elements failing it are skipped entirely, no side effect); folding it into .Where() on the source sequence makes that explicit without changing which lengths get added or throw. Addresses CodeQL cs/linq/missed-where (alert #9). Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/BuildMetadataFromXml.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/csharp/PhoneNumbers/BuildMetadataFromXml.cs b/csharp/PhoneNumbers/BuildMetadataFromXml.cs index a97bd746..20e53ffa 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)}."); } From f78732b9b02b1f68ba82e048e3c95c7d574a536d Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:48:58 -0500 Subject: [PATCH 11/15] style: drop redundant int.GetHashCode() call in PhoneNumberMatch int.GetHashCode() returns the int itself, so XOR-ing Start directly is equivalent. Addresses CodeQL cs/useless-gethashcode-call (alert #239). Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/PhoneNumberMatch.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/PhoneNumbers/PhoneNumberMatch.cs b/csharp/PhoneNumbers/PhoneNumberMatch.cs index 061b2391..eac1d624 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; From 1e587441a06d8a93c1e59322e6f5d66a982152c2 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:55:14 -0500 Subject: [PATCH 12/15] style: ternary builder merges and drop redundant int.GetHashCode() calls Phonemetadata.cs: all 17 PhoneNumberDesc-typed MergeXxx builder methods share the same if/else-assigns-same-variable shape; collapsed each to a ternary (cs/missed-ternary-operator, alerts #286-#302). Also dropped a redundant int.GetHashCode() call in PhoneMetadata.GetHashCode (cs/useless-gethashcode-call, alert #240). PhoneNumberDesc.cs: dropped two redundant int.GetHashCode() calls in the possibleLength_/possibleLengthLocalOnly_ hash folds (cs/useless-gethashcode-call, alerts #237/#238). int.GetHashCode() returns the int itself, so XOR-ing the value directly is equivalent. Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/PhoneNumberDesc.cs | 4 +- csharp/PhoneNumbers/Phonemetadata.cs | 155 +++++++++++-------------- 2 files changed, 71 insertions(+), 88 deletions(-) diff --git a/csharp/PhoneNumbers/PhoneNumberDesc.cs b/csharp/PhoneNumbers/PhoneNumberDesc.cs index 25675c48..3bc8ca58 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/Phonemetadata.cs b/csharp/PhoneNumbers/Phonemetadata.cs index 4c1b582e..23e9b804 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(); From 263cf3ad8f0122b6d323d6ddd4f34c36930b1afa Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:50:04 -0500 Subject: [PATCH 13/15] style: drop always-true lower bound in IsLatinLetter's BASIC_LATIN check char is unsigned in C#, so `letter >= 0x0000` can never be false. Addresses CodeQL cs/constant-condition (alert #252). Left the six-way Unicode-block range check itself alone (cs/coupled-types alert #371 and cs/complex-condition alert #8 also flagged this area): restructuring it would mean departing from the line-by-line block layout on a hot classification path for a pure readability heuristic, and the PhoneNumberMatcher/PhoneNumberUtil coupling CodeQL flags reflects real, intentional architecture shared with the upstream Java port rather than something a local fix should change. Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/PhoneNumberMatcher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/PhoneNumbers/PhoneNumberMatcher.cs b/csharp/PhoneNumbers/PhoneNumberMatcher.cs index 5694a996..f44cfaf2 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 From 6b902efcb580da8956d5a47d5e20109b4cb8df88 Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 09:52:51 -0500 Subject: [PATCH 14/15] style: address remaining CodeQL findings in PhoneNumberUtil.cs - FormatNumberForMobileDialing: collapse if/else assigning the same variable into a ternary (cs/missed-ternary-operator, alert #284). Left the MX/CL/UZ branch's if/else alone (alert #285): its condition carries ~15 lines of explanatory comments that a ternary would make harder to read, not easier. - MaybeExtractCountryCode: replace the `as object ==` reference-equality hack with an explicit ReferenceEquals call and a comment explaining why value equality would be wrong here (the sentinel check needs to distinguish the specific default-object instance from any region's real prefix that happens to equal the literal text). Same behavior, self-documenting instead of looking like a value-equality bug (cs/reference-equality-with-object, alert #31). - ChooseFormattingPatternForNumber: combine the nested ifs, no behavior change (cs/nested-if-statements, alert #255). - GetExampleNumberForType: the empty catch block was silent by design (falls through to `return null`, matching a metadata-quality issue to "no example number" rather than surfacing it) but had no comment saying so; upstream Java logs the exception here instead, which this port doesn't have a logging story for elsewhere either. Added a comment rather than introducing a logging dependency for one call site (cs/empty-catch-block, alert #248). Co-Authored-By: Claude Sonnet 5 --- csharp/PhoneNumbers/PhoneNumberUtil.cs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/csharp/PhoneNumbers/PhoneNumberUtil.cs b/csharp/PhoneNumbers/PhoneNumberUtil.cs index aeb84ba5..72fe506b 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); From f047ae6a75780fc533f299d891d3b4c51a6d8fed Mon Sep 17 00:00:00 2001 From: Thomas Clegg Date: Wed, 26 Aug 2026 10:43:48 -0500 Subject: [PATCH 15/15] fix: address CodeQL findings on PR's own fix commits - Path.Combine -> Path.Join in BuildGeocoding/BuildMetadata output paths (cs/path-combine: Path.Combine can silently drop earlier args if a later one looks absolute; Path.Join has no such behavior) - move the terminates-only comment inside the empty foreach body in EnsureTermination (cs/empty-block-without-comment wants the comment inside the block, not just above it) --- csharp/PhoneNumbers.MetadataBuilder/Program.cs | 4 ++-- csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/csharp/PhoneNumbers.MetadataBuilder/Program.cs b/csharp/PhoneNumbers.MetadataBuilder/Program.cs index 15f869c5..a51312c0 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, Path.GetFileName($"{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++; @@ -336,7 +336,7 @@ private static int BuildPerRegion( foreach (var metadata in metadataList) { var key = MakeFileNameKey(metadata, isAlternateFormatsMetadata); - var path = Path.Combine(outputDir, Path.GetFileName($"{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/TestPhoneNumberMatcher.cs b/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs index 50774aaa..36c7da49 100644 --- a/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs +++ b/csharp/PhoneNumbers.Test/TestPhoneNumberMatcher.cs @@ -1058,9 +1058,9 @@ private void EnsureTermination(string text, string defaultCountry, PhoneNumberUt for (var index = 0; index <= text.Length; index++) { var sub = text.Substring(index); - // Iterates over all matches to ensure that doing so terminates. foreach (var _ in phoneUtil.FindNumbers(sub, defaultCountry, leniency, long.MaxValue)) { + // Iterates over all matches to ensure that doing so terminates. } } }