search

microsoft / Power-Fx

Power Fx low-code programming language
MIT License
3.14k stars 309 forks source link

Updated option set testing #2446

Closed gregli-msft closed 1 month ago

gregli-msft commented 1 month ago

Fixes a bug in Canvas apps coercing an option set value to number. There were missing checks for pre-V1 semantics in DType.cs.

But this bug, along with the other recent fixes, is because we were not testing with actual option sets, instead relying on enums since those can be built for all the backing kinds. In general, enums and option sets aren't that different in V1, but they are very different in pre-V1 which is what Canvas uses today. This has been corrected in this PR. We now run tests with the test enums registered as option sets instead, mimicing the semantics in Canvas for Dataverse number and Boolean option sets. These new typed option sets are not exposed generally, as we should look at that implementation more closely, today it translates between string values more than it needs to.

By doing this testing, another bug was found in the interpeter, where we weren't back converting boolean values to the option set, for example in Text( If( false, OptionSet.Value, true ) ). C# now has the same semantics as Canvas for Booleans. Number and other backing kinds may also want this behavior long term, but since those aren't yet supported we have time to decide how to handle that scenario. Without any changes, a CalculatedOptionSetValue will be returned for the label if coerced to a string, but the value itself will be correct.

LucGenetier commented 1 month ago

✅ No public API change.

LucGenetier commented 1 month ago

✅ No public API change.

LucGenetier commented 1 month ago

✅ No public API change.

LucGenetier commented 1 month ago

✅ No public API change.