Skip to content
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 @@ -178,7 +178,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
5 changes: 3 additions & 2 deletions UnitsNet.Tests/QuantityParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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 @@ -54,7 +54,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 matches multiple units: ATon ("FOO"), Some ("foo").""", ex.Message);
}

[Fact]
Expand Down
11 changes: 9 additions & 2 deletions UnitsNet.Tests/UnitParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ public void Parse_AmbiguousUnitsThrowsException()
var exception2 = Assert.Throws<AmbiguousUnitParseException>(() => Length.Parse("1 pt", CultureInfo.InvariantCulture));

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

[Theory]
Expand Down Expand Up @@ -144,6 +144,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.Equal("""Cannot parse "MM" since it matches multiple units: Megameter ("Mm"), Millimeter ("mm").""", ex.Message);
}
Comment on lines +147 to +152
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice the new message format, the other one seemed excessively long..

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it


[Fact]
public void TryParse_WithNullAbbreviation_ReturnsFalse()
{
Expand Down
25 changes: 15 additions & 10 deletions UnitsNet/CustomCode/UnitParser.cs
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I said in #1484 about the recursive call requiring additional handling was wrong: I was under the persuasion that if there are multiple matches with the first culture, then we still want to try to disambiguate with the fallback culture, but that makes no sense..

Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ public Enum Parse(string unitAbbreviation, Type unitType, IFormatProvider? forma
formatProvider = UnitAbbreviationsCache.FallbackCulture;
continue;
default:
var unitsCsv = string.Join(", ", matches.Select(x => Enum.GetName(unitType, x.Unit)).ToArray());
throw new AmbiguousUnitParseException($"Cannot parse \"{unitAbbreviation}\" since it could be either of these: {unitsCsv}");
var unitsCsv = string.Join(", ", matches.Select(x => $"{Enum.GetName(unitType, x.Unit)} (\"{x.Abbreviation}\")").OrderBy(x => x));
throw new AmbiguousUnitParseException($"Cannot parse \"{unitAbbreviation}\" since it matches multiple units: {unitsCsv}.");
}
}
}
Expand Down Expand Up @@ -216,21 +216,26 @@ public bool TryParse([NotNullWhen(true)] string? unitAbbreviation, Type unitType
// 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)
List<(Enum Unit, string Abbreviation)> stringUnitPairs = _unitAbbreviationsCache.GetStringUnitPairs(enumValues, formatProvider);
(Enum Unit, string Abbreviation)[] matches =
stringUnitPairs.Where(pair => pair.Item2.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();
stringUnitPairs.Where(pair => pair.Abbreviation.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();

if (matches.Length == 0)
{
unitAbbreviation = NormalizeUnitString(unitAbbreviation);
matches = stringUnitPairs.Where(pair => pair.Item2.Equals(unitAbbreviation, StringComparison.OrdinalIgnoreCase)).ToArray();
var normalizeUnitString = NormalizeUnitString(unitAbbreviation);
if (unitAbbreviation != normalizeUnitString)
{
unitAbbreviation = normalizeUnitString;
matches = stringUnitPairs.Where(pair => pair.Abbreviation.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)
if (matches.Length == 1)
{
matches = stringUnitPairs.Where(pair => pair.Item2.Equals(unitAbbreviation)).ToArray();
return matches;
}

return matches;

// Narrow the search if too many hits, for example Megabar "Mbar" and Millibar "mbar" need to be distinguished
(Enum Unit, string Abbreviation)[] caseSensitiveMatches = stringUnitPairs.Where(pair => pair.Abbreviation.Equals(unitAbbreviation)).ToArray();
return caseSensitiveMatches.Length == 0 ? matches : caseSensitiveMatches;
}
}
}