Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🚸Describe why Parse fails due to case-insensitive ambiguity #1484

Merged
merged 2 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ protected string GetQuantityType(IQuantity quantity)
}

/// <summary>
/// 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.
/// <remarks>
/// An exhaustive search using all quantities is very likely to fail with an
/// <exception cref="AmbiguousUnitParseException" />, so make sure you're using the minimum set of supported quantities.
Expand Down
7 changes: 4 additions & 3 deletions UnitsNet.Tests/QuantityParserTests.cs
Original file line number Diff line number Diff line change
@@ -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 ([email protected]). Maintained at https://github.com/angularsen/UnitsNet.

using UnitsNet.Tests.CustomQuantities;
Expand Down Expand Up @@ -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");
Expand All @@ -52,7 +52,8 @@ void Act()
quantityParser.Parse<HowMuch, HowMuchUnit>("1 Foo", null, (value, unit) => new HowMuch((double) value, unit));
}

Assert.Throws<UnitNotFoundException>(Act);
var ex = Assert.Throws<AmbiguousUnitParseException>(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]
Expand Down
8 changes: 8 additions & 0 deletions UnitsNet.Tests/UnitParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,13 @@ public void Parse_MappedCustomUnit()

Assert.Equal(HowMuchUnit.Some, parsedUnit);
}

[Fact]
public void Parse_LengthUnit_MM_ThrowsExceptionDescribingTheAmbiguity()
{
var ex = Assert.Throws<AmbiguousUnitParseException>(() => UnitsNetSetup.Default.UnitParser.Parse<LengthUnit>("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);
}

}
}
18 changes: 15 additions & 3 deletions UnitsNet/CustomCode/UnitParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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());
Expand Down