Skip to content

Commit 572f151

Browse files
lipchevangularsen
andauthored
🚸UnitParser.Parse should throw AmbiguousUnitParseException before any UnitNotFoundException (#1484) (#1494)
Fixes #1423 for `release-v6` Carry over from #1484 `UnitParser.Parse<LengthUnit>("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 - Avoid some of the unnecessary array allocations Co-authored-by: Andreas Gullberg Larsen <[email protected]>
1 parent ddf6134 commit 572f151

File tree

4 files changed

+28
-15
lines changed

4 files changed

+28
-15
lines changed

UnitsNet.Serialization.JsonNet/AbbreviatedUnitsConverter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ protected string GetQuantityType(IQuantity quantity)
178178
}
179179

180180
/// <summary>
181-
/// Attempt to find an a unique (non-ambiguous) unit matching the provided abbreviation.
181+
/// Attempt to find a unique (non-ambiguous) unit matching the provided abbreviation.
182182
/// <remarks>
183183
/// An exhaustive search using all quantities is very likely to fail with an
184184
/// <exception cref="AmbiguousUnitParseException" />, so make sure you're using the minimum set of supported quantities.

UnitsNet.Tests/QuantityParserTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public void Parse_WithOneCaseInsensitiveMatchAndOneExactMatch_ParsesWithTheExact
4242
}
4343

4444
[Fact]
45-
public void Parse_WithMultipleCaseInsensitiveMatchesButNoExactMatches_ThrowsUnitNotFoundException()
45+
public void Parse_WithMultipleCaseInsensitiveMatchesButNoExactMatches_ThrowsAmbiguousUnitParseException()
4646
{
4747
var unitAbbreviationsCache = new UnitAbbreviationsCache();
4848
unitAbbreviationsCache.MapUnitToAbbreviation(HowMuchUnit.Some, "foo");
@@ -54,7 +54,8 @@ void Act()
5454
quantityParser.Parse<HowMuch, HowMuchUnit>("1 Foo", null, (value, unit) => new HowMuch((double) value, unit));
5555
}
5656

57-
Assert.Throws<UnitNotFoundException>(Act);
57+
var ex = Assert.Throws<AmbiguousUnitParseException>(Act);
58+
Assert.Equal("""Cannot parse "Foo" since it matches multiple units: ATon ("FOO"), Some ("foo").""", ex.Message);
5859
}
5960

6061
[Fact]

UnitsNet.Tests/UnitParserTests.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ public void Parse_AmbiguousUnitsThrowsException()
115115
var exception2 = Assert.Throws<AmbiguousUnitParseException>(() => Length.Parse("1 pt", CultureInfo.InvariantCulture));
116116

117117
// Assert
118-
Assert.Equal("Cannot parse \"pt\" since it could be either of these: DtpPoint, PrinterPoint", exception1.Message);
119-
Assert.Equal("Cannot parse \"pt\" since it could be either of these: DtpPoint, PrinterPoint", exception2.Message);
118+
Assert.Equal("""Cannot parse "pt" since it matches multiple units: DtpPoint ("pt"), PrinterPoint ("pt").""", exception1.Message);
119+
Assert.Equal("""Cannot parse "pt" since it matches multiple units: DtpPoint ("pt"), PrinterPoint ("pt").""", exception2.Message);
120120
}
121121

122122
[Theory]
@@ -144,6 +144,13 @@ public void Parse_MappedCustomUnit()
144144
Assert.Equal(HowMuchUnit.Some, parsedUnit);
145145
}
146146

147+
[Fact]
148+
public void Parse_LengthUnit_MM_ThrowsExceptionDescribingTheAmbiguity()
149+
{
150+
var ex = Assert.Throws<AmbiguousUnitParseException>(() => UnitsNetSetup.Default.UnitParser.Parse<LengthUnit>("MM"));
151+
Assert.Equal("""Cannot parse "MM" since it matches multiple units: Megameter ("Mm"), Millimeter ("mm").""", ex.Message);
152+
}
153+
147154
[Fact]
148155
public void TryParse_WithNullAbbreviation_ReturnsFalse()
149156
{

UnitsNet/CustomCode/UnitParser.cs

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ public Enum Parse(string unitAbbreviation, Type unitType, IFormatProvider? forma
8787
formatProvider = UnitAbbreviationsCache.FallbackCulture;
8888
continue;
8989
default:
90-
var unitsCsv = string.Join(", ", matches.Select(x => Enum.GetName(unitType, x.Unit)).ToArray());
91-
throw new AmbiguousUnitParseException($"Cannot parse \"{unitAbbreviation}\" since it could be either of these: {unitsCsv}");
90+
var unitsCsv = string.Join(", ", matches.Select(x => $"{Enum.GetName(unitType, x.Unit)} (\"{x.Abbreviation}\")").OrderBy(x => x));
91+
throw new AmbiguousUnitParseException($"Cannot parse \"{unitAbbreviation}\" since it matches multiple units: {unitsCsv}.");
9292
}
9393
}
9494
}
@@ -216,21 +216,26 @@ public bool TryParse([NotNullWhen(true)] string? unitAbbreviation, Type unitType
216216
// TODO see about optimizing this method: both Parse and TryParse only care about having one match (in case of a failure we could return the number of matches)
217217
List<(Enum Unit, string Abbreviation)> stringUnitPairs = _unitAbbreviationsCache.GetStringUnitPairs(enumValues, formatProvider);
218218
(Enum Unit, string Abbreviation)[] matches =
219-
stringUnitPairs.Where(pair => pair.Item2.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();
219+
stringUnitPairs.Where(pair => pair.Abbreviation.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();
220220

221221
if (matches.Length == 0)
222222
{
223-
unitAbbreviation = NormalizeUnitString(unitAbbreviation);
224-
matches = stringUnitPairs.Where(pair => pair.Item2.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();
223+
var normalizeUnitString = NormalizeUnitString(unitAbbreviation);
224+
if (unitAbbreviation != normalizeUnitString)
225+
{
226+
unitAbbreviation = normalizeUnitString;
227+
matches = stringUnitPairs.Where(pair => pair.Abbreviation.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();
228+
}
225229
}
226230

227-
// Narrow the search if too many hits, for example Megabar "Mbar" and Millibar "mbar" need to be distinguished
228-
if (matches.Length > 1)
231+
if (matches.Length == 1)
229232
{
230-
matches = stringUnitPairs.Where(pair => pair.Item2.Equals(unitAbbreviation)).ToArray();
233+
return matches;
231234
}
232-
233-
return matches;
235+
236+
// Narrow the search if too many hits, for example Megabar "Mbar" and Millibar "mbar" need to be distinguished
237+
(Enum Unit, string Abbreviation)[] caseSensitiveMatches = stringUnitPairs.Where(pair => pair.Abbreviation.Equals(unitAbbreviation)).ToArray();
238+
return caseSensitiveMatches.Length == 0 ? matches : caseSensitiveMatches;
234239
}
235240
}
236241
}

0 commit comments

Comments
 (0)