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

.Net 5.0 Int.Parse fails parsing with negative values. #40258

Closed
eriksimonic opened this issue Aug 3, 2020 · 15 comments
Closed

.Net 5.0 Int.Parse fails parsing with negative values. #40258

eriksimonic opened this issue Aug 3, 2020 · 15 comments
Labels
area-System.Globalization question Answer questions and provide assistance, not an issue with source code or documentation.

Comments

@eriksimonic
Copy link

.Net 5.0 Preview 7, fails on Int.Parse("-1") with
Message:
System.FormatException : Input string was not in a correct format.
Stack Trace:
Number.ThrowOverflowOrFormatException(ParsingStatus status, TypeCode type)
Number.ParseInt32(ReadOnlySpan`1 value, NumberStyles styles, NumberFormatInfo info)
Int32.Parse(String s)
NetCore50prev7_Tests.NegativeIntegerFailsParse() line 15

public class NetCore50prev7_Tests
{
[Fact]
public void NegativeIntegerFailsParse()
{
int.Parse("-1");
}
}

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 3, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@EgorBo
Copy link
Member

EgorBo commented Aug 3, 2020

@eriksimonic what is the current culture?

Console.WriteLine(CultureInfo.CurrentCulture);

and OS?

@eriksimonic
Copy link
Author

eriksimonic commented Aug 3, 2020

@EgorBo
Windows 10 2004, build 19041.388
Culture is sl-SI

If I change the culture to CultureInfo.InvariantCulture, the parse works.

@EgorBo
Copy link
Member

EgorBo commented Aug 3, 2020

Console.WriteLine(string.Join(", ", Encoding.Unicode.GetBytes("-"))); // en-US negative sign
Console.WriteLine(string.Join(", ", Encoding.Unicode.GetBytes(""))); // sl-SI negative sign
{45, 0}
{18, 34}

sl-SI locale uses a slightly different negative sign 🙂

You can obtain it via new CultureInfo("sl-SI").NumberFormat.NegativeSign

@EgorBo EgorBo added area-System.Globalization question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Aug 3, 2020
@ghost
Copy link

ghost commented Aug 3, 2020

Tagging subscribers to this area: @tarekgh, @safern, @krwq
See info in area-owners.md if you want to be subscribed.

@eriksimonic
Copy link
Author

@EgorBo , this works correctly in .net core 5.0 or any other, It is this a "feature"

@EgorBo
Copy link
Member

EgorBo commented Aug 3, 2020

hm... indeed, 🤔

Console.WriteLine(int.Parse("-1", new CultureInfo("sl-SI")));

works for net4x and netcoreapp3.1 (windows)
but doesn't work for net5 (windows and *nix) and netcoreapp3.1 (*nix, ICU)

@benaadams
Copy link
Member

Hasn't net5 moved to ICU on windows also?

@am11
Copy link
Member

am11 commented Aug 3, 2020

Yes. net5 switched to ICU library on Windows. Some cultures (e.g. en-US) work with multiple minus signs U+002D and U+221 with ICU. Apparently not the case with sl-SI. Interestingly, there was a related bug for Slovenian minus sign (U+2013), which was fixed last year in release 31: https://unicode-org.atlassian.net/browse/CLDR-10050.

@EgorBo
Copy link
Member

EgorBo commented Aug 3, 2020

@eriksimonic as a workaround you can define an env. variable DOTNET_SYSTEM_GLOBALIZATION_USENLS=true to rely on NLS instead of ICU on Windows.

@tarekgh
Copy link
Member

tarekgh commented Aug 3, 2020

Yes this is by design as we switched to ICU. sl-SI is not using - as the negative sign. If you are parsing with such culture you have to provide the correct negative sign. As mentioned earlier in the discussion, to workaround it, either use different culture e.g. Invariant, or enable NLS by the config switch System.Globalization.UseNls https://docs.microsoft.com/en-us/dotnet/standard/globalization-localization/globalization-icu.

@tarekgh tarekgh closed this as completed Aug 3, 2020
@benaadams
Copy link
Member

You could also write a method that tries both

int ParseInt(string value)
{
    if (int.TryParse(value, out int result))
    {
        return result;
    }

    return int.Parse(value, CultureInfo.InvariantCulture);
}

@eriksimonic
Copy link
Author

eriksimonic commented Aug 4, 2020

@benaadams , hard to do when third party library uses the method.
@tarekgh why such a braking change?
At least is <InvariantGlobalization>true</InvariantGlobalization> fisible option.
Thanks.

@Clockwork-Muse
Copy link
Contributor

@eriksimonic - Outside the move to ICU, this sort of thing is always possible; you can construct/use custom cultures on windows that would have the same effect.

It might be handy to know what sort of third-party library you're using here; for instance, if you're doing data serialization/processing, you should likely be in the invariant culture anyways. Although I'd probably raise a bug with them if you can't override the culture/formatting options.

@tarekgh
Copy link
Member

tarekgh commented Aug 4, 2020

@eriksimonic, the answer of @Clockwork-Muse is correct. .NET 5.0 is by default using ICU and NLS. you have the option to switch back to NLS. but I wouldn't recommend that because NLS data can change too especially Windows working hard to converge with CLDR data which is shipped with ICU. As @Clockwork-Muse mentioned, if you want to guarantee the parsing, you should format and parse using Invariant culture.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Globalization question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

7 participants