From f8148843c5a4a952c68eab4c5426f2e6086f3088 Mon Sep 17 00:00:00 2001 From: Andreas Gullberg Larsen Date: Thu, 2 Jan 2025 11:50:16 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8Describe=20why=20Parse=20fails=20du?= =?UTF-8?q?e=20to=20case-insensitive=20ambiguity=20(#1484)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1423 `UnitParser.Parse("MM")` fails due to matching both `Megameter` and `Millimeter` in case-insensitive matching, but matches neither of them in the case-sensitive fallback. It was confusing to get `UnitsNotFoundException` in this case, since case-insensitive usually works for most units. ### Changes - Handle this case and throw `AmbiguousUnitParseException` instead of `UnitNotFoundException` - Describe the case-insensitive units that matched - Fix existing test `Parse_WithMultipleCaseInsensitiveMatchesButNoExactMatches_ThrowsUnitNotFoundException` - Skip retrying with fallback culture if no specific `formatProvider` was given --- .../AbbreviatedUnitsConverter.cs | 2 +- UnitsNet.Tests/QuantityParserTests.cs | 7 ++++--- UnitsNet.Tests/UnitParserTests.cs | 8 ++++++++ UnitsNet/CustomCode/UnitParser.cs | 18 +++++++++++++++--- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/UnitsNet.Serialization.JsonNet/AbbreviatedUnitsConverter.cs b/UnitsNet.Serialization.JsonNet/AbbreviatedUnitsConverter.cs index ba06ab91d8..3209906f39 100644 --- a/UnitsNet.Serialization.JsonNet/AbbreviatedUnitsConverter.cs +++ b/UnitsNet.Serialization.JsonNet/AbbreviatedUnitsConverter.cs @@ -190,7 +190,7 @@ protected string GetQuantityType(IQuantity quantity) } /// - /// Attempt to find an a unique (non-ambiguous) unit matching the provided abbreviation. + /// Attempt to find a unique (non-ambiguous) unit matching the provided abbreviation. /// /// An exhaustive search using all quantities is very likely to fail with an /// , so make sure you're using the minimum set of supported quantities. diff --git a/UnitsNet.Tests/QuantityParserTests.cs b/UnitsNet.Tests/QuantityParserTests.cs index 63828fb7a5..3ed90d5300 100644 --- a/UnitsNet.Tests/QuantityParserTests.cs +++ b/UnitsNet.Tests/QuantityParserTests.cs @@ -1,4 +1,4 @@ -// Licensed under MIT No Attribution, see LICENSE file at the root. +// Licensed under MIT No Attribution, see LICENSE file at the root. // Copyright 2013 Andreas Gullberg Larsen (andreas.larsen84@gmail.com). Maintained at https://github.com/angularsen/UnitsNet. using UnitsNet.Tests.CustomQuantities; @@ -40,7 +40,7 @@ public void Parse_WithOneCaseInsensitiveMatchAndOneExactMatch_ParsesWithTheExact } [Fact] - public void Parse_WithMultipleCaseInsensitiveMatchesButNoExactMatches_ThrowsUnitNotFoundException() + public void Parse_WithMultipleCaseInsensitiveMatchesButNoExactMatches_ThrowsAmbiguousUnitParseException() { var unitAbbreviationsCache = new UnitAbbreviationsCache(); unitAbbreviationsCache.MapUnitToAbbreviation(HowMuchUnit.Some, "foo"); @@ -52,7 +52,8 @@ void Act() quantityParser.Parse("1 Foo", null, (value, unit) => new HowMuch((double) value, unit)); } - Assert.Throws(Act); + var ex = Assert.Throws(Act); + Assert.Equal("Cannot parse \"Foo\" since it matched multiple units [Some, ATon] with case-insensitive comparison, but zero units with case-sensitive comparison. To resolve the ambiguity, pass a unit abbreviation with the correct casing.", ex.Message); } [Fact] diff --git a/UnitsNet.Tests/UnitParserTests.cs b/UnitsNet.Tests/UnitParserTests.cs index be318e1582..72a8104d79 100644 --- a/UnitsNet.Tests/UnitParserTests.cs +++ b/UnitsNet.Tests/UnitParserTests.cs @@ -136,5 +136,13 @@ public void Parse_MappedCustomUnit() Assert.Equal(HowMuchUnit.Some, parsedUnit); } + + [Fact] + public void Parse_LengthUnit_MM_ThrowsExceptionDescribingTheAmbiguity() + { + var ex = Assert.Throws(() => UnitsNetSetup.Default.UnitParser.Parse("MM")); + Assert.Contains("Cannot parse \"MM\" since it matched multiple units [Millimeter, Megameter] with case-insensitive comparison, but zero units with case-sensitive comparison. To resolve the ambiguity, pass a unit abbreviation with the correct casing.", ex.Message); + } + } } diff --git a/UnitsNet/CustomCode/UnitParser.cs b/UnitsNet/CustomCode/UnitParser.cs index 74fa0cb652..91da1db829 100644 --- a/UnitsNet/CustomCode/UnitParser.cs +++ b/UnitsNet/CustomCode/UnitParser.cs @@ -72,14 +72,19 @@ public Enum Parse(string? unitAbbreviation, Type unitType, IFormatProvider? form var stringUnitPairs = _unitAbbreviationsCache.GetStringUnitPairs(enumValues, formatProvider); var matches = stringUnitPairs.Where(pair => pair.Item1.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray(); + // No match? Retry after normalizing the unit abbreviation. if(matches.Length == 0) { unitAbbreviation = NormalizeUnitString(unitAbbreviation); matches = stringUnitPairs.Where(pair => pair.Item1.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray(); } - // Narrow the search if too many hits, for example Megabar "Mbar" and Millibar "mbar" need to be distinguished - if(matches.Length > 1) + var caseInsensitiveMatches = matches; + + // More than one case-insensitive match? Retry with case-sensitive match. + // For example, Megabar "Mbar" and Millibar "mbar" need to be distinguished. + bool hasMultipleCaseInsensitiveMatches = matches.Length > 1; + if (hasMultipleCaseInsensitiveMatches) matches = stringUnitPairs.Where(pair => pair.Item1.Equals(unitAbbreviation)).ToArray(); switch(matches.Length) @@ -88,11 +93,18 @@ public Enum Parse(string? unitAbbreviation, Type unitType, IFormatProvider? form return (Enum)Enum.ToObject(unitType, matches[0].Unit); case 0: // Retry with fallback culture, if different. - if(!Equals(formatProvider, UnitAbbreviationsCache.FallbackCulture)) + if (formatProvider != null && !Equals(formatProvider, UnitAbbreviationsCache.FallbackCulture)) { return Parse(unitAbbreviation, unitType, UnitAbbreviationsCache.FallbackCulture); } + if (hasMultipleCaseInsensitiveMatches) + { + string ciUnitsCsv = string.Join(", ", caseInsensitiveMatches.Select(x => Enum.GetName(unitType, x.Unit))); + throw new AmbiguousUnitParseException( + $"Cannot parse \"{unitAbbreviation}\" since it matched multiple units [{ciUnitsCsv}] with case-insensitive comparison, but zero units with case-sensitive comparison. To resolve the ambiguity, pass a unit abbreviation with the correct casing."); + } + throw new UnitNotFoundException($"Unit not found with abbreviation [{unitAbbreviation}] for unit type [{unitType}]."); default: string unitsCsv = string.Join(", ", matches.Select(x => Enum.GetName(unitType, x.Unit)).ToArray());