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

Use invariant culture for converting propertie's values to string #9874

Merged

Conversation

f-alizada
Copy link
Contributor

Fixes #9757

Context

The evaluation of the project is affected in case:
The resulting values of the properties is a negative number and the Culture the parsing used contains different unicode for a minus sign and later used in arithmetic condition results failed project exception.
Due to the converting numeric type ToString using current Culture (or not specifying to convert using InvariantCulture) and later using the InvariantCulture for trying to parse the saved .

private static bool ValidDecimalNumber(string number, out double value)

Changes Made

Check if the object is a numeric type then convert it to string using InvariantCulture. ChangeWave 17.10

Testing

Added tests to replicate the behavior with/without changewave.

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Looks good!

I'd vote on trying to generalize this for all conversions.

src/Build/Evaluation/Expander.cs Outdated Show resolved Hide resolved
src/Build/Evaluation/Expander.cs Outdated Show resolved Hide resolved
@f-alizada f-alizada requested a review from rainersigwald March 18, 2024 13:27
@f-alizada f-alizada changed the title Use invariant culture for the number parsing Use invariant culture for converting propertie's values Mar 18, 2024
@f-alizada f-alizada changed the title Use invariant culture for converting propertie's values Use invariant culture for converting propertie's values to string Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Numeric comparison uses current culture
3 participants