mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-05-18 05:05:25 +02:00
[CmdPal Calculator] Add rand() and randi(). Expand result responses to differentiate between NaN and ParseError (#47725)
## Summary of the Pull Request This adds `rand()` and `randi()` functions to Command Palette's Calculator, making it consistent with Run. It also expands upon the return values from `ToWStringFullPrecision()`, so NaN, ParseError and +/-infinity results are passed back to the caller, improving the specificity of the error message display. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #47707 <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments The two new functions have been added to **ExprtkEvaluator.cpp**, alongside `sign()` and `factorial()`. As they need to handle the state of the RNG, they're slightly more complex in implementation. I used the Mersenne Twister RNG with a uniform distribution, and the instances are marked `static thread_local` in case the engine moves to multithreaded evaluation in the future. It's possible for the RNG to return a value out of the range of `double`, and this is caught and `quiet_NaN()` is returned. To prevent this being caught as a generic parse error, I updated `ToWStringFullPrecision()` to distinguish between `NaN`, expression parsing errors and infinity values. This should improve the accuracy of error messages for other expressions, too. Finally, I corrected a comment in **CalculateEngine.cs,** which still referred to the Mages calculation engine. The log/ln mapping is the same for both engines, so the comment was still accurate except for this reference. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Unit tests were added to exercise the new functions. All Calculator tests pass: <img width="375" height="59" alt="image" src="https://github.com/user-attachments/assets/5a33e1ed-a4fd-4d53-b9ba-6b44000f1bf4" /> Confirmed that error messages are displaying correctly for the newly-exposed result types: **Not a number** <img width="787" height="128" alt="image" src="https://github.com/user-attachments/assets/8c73dcf6-122b-4af8-bf1a-62284842433a" /> <img width="786" height="145" alt="image" src="https://github.com/user-attachments/assets/fe14338c-1160-4aae-83dd-5ca3491ae59e" /> **+/- Infinity** <img width="898" height="137" alt="image" src="https://github.com/user-attachments/assets/20cfacda-72a7-44bb-a875-af7be39ee7e2" /> **Parser failure** <img width="607" height="139" alt="image" src="https://github.com/user-attachments/assets/7d7120b2-a2cf-45b6-ab89-79af4051fa50" /> <img width="587" height="140" alt="image" src="https://github.com/user-attachments/assets/2dc7a365-7ee6-4379-8b3f-47b3912e6891" />
This commit is contained in:
Dave Rayment
6
.github/actions/spell-check/allow/code.txt
vendored
6
.github/actions/spell-check/allow/code.txt
vendored
@@ -186,6 +186,12 @@ xmlutil
|
||||
# Prefix
|
||||
pcs
|
||||
|
||||
# EXPRTK / C++ MATH
|
||||
|
||||
ifunction
|
||||
isinf
|
||||
isnan
|
||||
|
||||
# User32.SYSTEM_METRICS_INDEX.cs
|
||||
|
||||
CLEANBOOT
|
||||
|
||||
@@ -5,11 +5,17 @@
|
||||
#include <sstream>
|
||||
#include <cmath>
|
||||
#include <limits>
|
||||
#include <random>
|
||||
|
||||
namespace ExprtkCalculator::internal
|
||||
{
|
||||
static double factorial(const double n)
|
||||
{
|
||||
if (std::isnan(n) || std::isinf(n))
|
||||
{
|
||||
return std::numeric_limits<double>::quiet_NaN();
|
||||
}
|
||||
|
||||
// Only allow non-negative integers
|
||||
if (n < 0.0 || std::floor(n) != n)
|
||||
{
|
||||
@@ -20,13 +26,80 @@ namespace ExprtkCalculator::internal
|
||||
|
||||
static double sign(const double n)
|
||||
{
|
||||
// The sign of NaN is undefined.
|
||||
if (std::isnan(n))
|
||||
{
|
||||
return std::numeric_limits<double>::quiet_NaN();
|
||||
}
|
||||
|
||||
if (n > 0.0) return 1.0;
|
||||
if (n < 0.0) return -1.0;
|
||||
return 0.0;
|
||||
}
|
||||
|
||||
// rand(): returns a uniformly distributed random double in [0, 1)
|
||||
struct rand_func : public exprtk::ifunction<double>
|
||||
{
|
||||
std::mt19937_64 rng;
|
||||
std::uniform_real_distribution<double> dist;
|
||||
|
||||
rand_func() :
|
||||
exprtk::ifunction<double>(0),
|
||||
rng(std::random_device{}()),
|
||||
dist(0.0, 1.0)
|
||||
{}
|
||||
|
||||
inline double operator()() override
|
||||
{
|
||||
return dist(rng);
|
||||
}
|
||||
};
|
||||
|
||||
// randi(n): returns a uniformly distributed random integer in [0, n-1]
|
||||
struct randi_func : public exprtk::ifunction<double>
|
||||
{
|
||||
std::mt19937_64 rng;
|
||||
|
||||
randi_func() :
|
||||
exprtk::ifunction<double>(1),
|
||||
rng(std::random_device{}())
|
||||
{}
|
||||
|
||||
inline double operator()(const double& n) override
|
||||
{
|
||||
if (std::isnan(n) || std::isinf(n))
|
||||
{
|
||||
return std::numeric_limits<double>::quiet_NaN();
|
||||
}
|
||||
|
||||
constexpr double maxLongLongAsDouble = static_cast<double>(std::numeric_limits<long long>::max());
|
||||
if (n < 1.0 || n >= maxLongLongAsDouble)
|
||||
{
|
||||
return std::numeric_limits<double>::quiet_NaN();
|
||||
}
|
||||
|
||||
if (std::floor(n) != n)
|
||||
{
|
||||
return std::numeric_limits<double>::quiet_NaN();
|
||||
}
|
||||
|
||||
std::uniform_int_distribution<long long> dist(0, static_cast<long long>(n) - 1);
|
||||
return static_cast<double>(dist(rng));
|
||||
}
|
||||
};
|
||||
|
||||
std::wstring ToWStringFullPrecision(double value)
|
||||
{
|
||||
if (std::isnan(value))
|
||||
{
|
||||
return L"NaN";
|
||||
}
|
||||
|
||||
if (std::isinf(value))
|
||||
{
|
||||
return value > 0 ? L"inf" : L"-inf";
|
||||
}
|
||||
|
||||
std::wostringstream oss;
|
||||
oss.imbue(std::locale::classic());
|
||||
oss << std::fixed << std::setprecision(15) << value;
|
||||
@@ -47,6 +120,13 @@ namespace ExprtkCalculator::internal
|
||||
symbol_table.add_function("factorial", factorial);
|
||||
symbol_table.add_function("sign", sign);
|
||||
|
||||
// thread_local ensures each thread has its own RNG instance (seeded once,
|
||||
// state preserved across calls) without requiring locks.
|
||||
static thread_local rand_func rand_fn;
|
||||
static thread_local randi_func randi_fn;
|
||||
symbol_table.add_function("rand", rand_fn);
|
||||
symbol_table.add_function("randi", randi_fn);
|
||||
|
||||
exprtk::expression<double> expression;
|
||||
expression.register_symbol_table(symbol_table);
|
||||
|
||||
@@ -65,7 +145,7 @@ namespace ExprtkCalculator::internal
|
||||
parser.settings().disable_all_inequality_ops(); // Disable inequality operators like <, >, <=, >=, !=, etc.
|
||||
|
||||
if (!parser.compile(expressionText, expression))
|
||||
return L"NaN";
|
||||
return L"ParseError";
|
||||
|
||||
return ToWStringFullPrecision(expression.value());
|
||||
}
|
||||
|
||||
@@ -16,6 +16,8 @@ namespace Microsoft.CmdPal.Ext.Calc.UnitTests;
|
||||
[TestClass]
|
||||
public class ExtendedCalculatorParserTests : CommandPaletteUnitTestBase
|
||||
{
|
||||
private const int IntMax = int.MaxValue;
|
||||
|
||||
[DataTestMethod]
|
||||
[DataRow(null)]
|
||||
[DataRow("")]
|
||||
@@ -409,13 +411,136 @@ public class ExtendedCalculatorParserTests : CommandPaletteUnitTestBase
|
||||
[DataTestMethod]
|
||||
[DataRow("171!")]
|
||||
[DataRow("1000!")]
|
||||
public void Interpret_ReturnsError_WhenValueOverflowsDecimal(string input)
|
||||
public void Interpret_ReturnsOutOfBoundsError_WhenValueOverflowsDecimal(string input)
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
CalculateEngine.Interpret(settings, input, CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.IsFalse(string.IsNullOrEmpty(error));
|
||||
Assert.AreNotEqual(null, error);
|
||||
Assert.AreEqual(Properties.Resources.calculator_not_covert_to_decimal, error);
|
||||
}
|
||||
|
||||
[DataTestMethod]
|
||||
[DataRow("exp(99999)")]
|
||||
[DataRow("-exp(99999)")]
|
||||
public void Interpret_ReturnsOutOfBoundsError_WhenResultIsInfinity(string input)
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, input, CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.AreEqual(default, result);
|
||||
Assert.AreEqual(Properties.Resources.calculator_not_covert_to_decimal, error);
|
||||
}
|
||||
|
||||
[DataTestMethod]
|
||||
[DataRow("1 2")]
|
||||
public void Interpret_ReturnsExpressionNotCompleteError_WhenExpressionIsInvalid(string input)
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, input, CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.AreEqual(default, result);
|
||||
Assert.AreEqual(Properties.Resources.calculator_expression_not_complete, error);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Interpret_ReturnsNotANumberError_WhenResultIsNaN()
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, "sqrt(-1)", CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.AreEqual(default, result);
|
||||
Assert.AreEqual(Properties.Resources.calculator_not_a_number, error);
|
||||
}
|
||||
|
||||
[DataTestMethod]
|
||||
[DataRow("factorial(-1)")]
|
||||
[DataRow("factorial(0.5)")]
|
||||
[DataRow("factorial(sqrt(-1))")]
|
||||
[DataRow("sign(sqrt(-1))")]
|
||||
public void Interpret_ReturnsNotANumberError_WhenCustomFunctionArgumentInvalid(string input)
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, input, CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.AreEqual(default, result);
|
||||
Assert.AreEqual(Properties.Resources.calculator_not_a_number, error);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Interpret_Rand_ReturnsValueInRange()
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, "rand()", CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.IsNull(error);
|
||||
Assert.IsTrue(result.Result.HasValue, "rand() returned no result");
|
||||
Assert.IsTrue(result.Result >= 0M && result.Result < 1M, $"rand() result {result.Result} was not in [0, 1)");
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Interpret_Randi_ReturnsZero_WhenArgIsOne()
|
||||
{
|
||||
// randi(1) has only one valid outcome: 0. This test is fully deterministic.
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, "randi(1)", CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.IsNull(error);
|
||||
Assert.IsTrue(result.Result.HasValue, "randi(1) returned no result");
|
||||
Assert.AreEqual(0M, result.Result!.Value, "randi(1) must always return 0");
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Interpret_Randi_StaysInRange_WhenArgIsTwo()
|
||||
{
|
||||
// randi(2) has the smallest non-trivial range [0, 1]. Running many iterations
|
||||
// gives high confidence both boundary values are reachable.
|
||||
var settings = new Settings();
|
||||
|
||||
for (var i = 0; i < 100; i++)
|
||||
{
|
||||
var result = CalculateEngine.Interpret(settings, "randi(2)", CultureInfo.InvariantCulture, out var error);
|
||||
Assert.IsNull(error);
|
||||
Assert.IsTrue(result.Result.HasValue, "randi(2) returned no result");
|
||||
var value = result.Result!.Value;
|
||||
Assert.AreEqual(value, Math.Floor(value), $"randi(2) result {value} was not an integer");
|
||||
Assert.IsTrue(value >= 0M && value <= 1M, $"randi(2) result {value} was not in [0, 1]");
|
||||
}
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Interpret_Randi_HandlesIntMaxArgument()
|
||||
{
|
||||
// Ensures no integer overflow in the C++ cast when the argument is int.MaxValue.
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, $"randi({IntMax})", CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.IsNull(error);
|
||||
Assert.IsTrue(result.Result.HasValue, $"randi({IntMax}) returned no result");
|
||||
var value = result.Result!.Value;
|
||||
Assert.AreEqual(value, Math.Floor(value), $"randi({IntMax}) result {value} was not an integer");
|
||||
Assert.IsTrue(value >= 0M && value < IntMax, $"randi({IntMax}) result {value} was out of range");
|
||||
}
|
||||
|
||||
[DataTestMethod]
|
||||
[DataRow("randi(0)")]
|
||||
[DataRow("randi(0.5)")]
|
||||
[DataRow("randi(-1)")]
|
||||
[DataRow("randi(exp(10000))")]
|
||||
public void Interpret_Randi_ReturnsNotANumberError_WhenArgumentInvalid(string input)
|
||||
{
|
||||
var settings = new Settings();
|
||||
|
||||
var result = CalculateEngine.Interpret(settings, input, CultureInfo.InvariantCulture, out var error);
|
||||
|
||||
Assert.AreEqual(default, result);
|
||||
Assert.AreEqual(Properties.Resources.calculator_not_a_number, error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,8 +51,8 @@ public static partial class CalculateEngine
|
||||
return default;
|
||||
}
|
||||
|
||||
// mages has quirky log representation
|
||||
// mage has log == ln vs log10
|
||||
// ExprTK uses log == ln and log10 for base-10, so we remap here
|
||||
// to match the user-facing names (log → log10, ln → log).
|
||||
// Use regex replacements so optional whitespace between the function name and
|
||||
// '(' is handled correctly - "log (100)" must map to log10 just like "log(100)"
|
||||
// does. The negative lookahead prevents "log10" / "log2" from being touched.
|
||||
@@ -75,12 +75,18 @@ public static partial class CalculateEngine
|
||||
var result = _calculator.EvaluateExpression(input);
|
||||
|
||||
// This could happen for some incorrect queries, like pi(2)
|
||||
if (result == "NaN")
|
||||
if (result == "ParseError")
|
||||
{
|
||||
error = Properties.Resources.calculator_expression_not_complete;
|
||||
return default;
|
||||
}
|
||||
|
||||
if (result == "NaN")
|
||||
{
|
||||
error = Properties.Resources.calculator_not_a_number;
|
||||
return default;
|
||||
}
|
||||
|
||||
// If we're out of bounds
|
||||
if (result is "inf" or "-inf")
|
||||
{
|
||||
@@ -90,6 +96,7 @@ public static partial class CalculateEngine
|
||||
|
||||
if (string.IsNullOrEmpty(result))
|
||||
{
|
||||
error = Properties.Resources.calculator_not_a_number;
|
||||
return default;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
@@ -14,7 +14,7 @@ public static partial class CalculateHelper
|
||||
@"^(" +
|
||||
@"%|" +
|
||||
@"ceil\s*\(|floor\s*\(|exp\s*\(|max\s*\(|min\s*\(|abs\s*\(|log(?:2|10)?\s*\(|ln\s*\(|sqrt\s*\(|pow\s*\(|" +
|
||||
@"factorial\s*\(|sign\s*\(|round\s*\(|rand\s*\(\)|randi\s*\([^\)]|" +
|
||||
@"factorial\s*\(|sign\s*\(|round\s*\(|rand\s*\(\)|randi\s*\((?=[^\)])|" +
|
||||
@"sin\s*\(|cos\s*\(|tan\s*\(|arcsin\s*\(|arccos\s*\(|arctan\s*\(|" +
|
||||
@"sinh\s*\(|cosh\s*\(|tanh\s*\(|arsinh\s*\(|arcosh\s*\(|artanh\s*\(|" +
|
||||
@"rad\s*\(|deg\s*\(|grad\s*\(|" + /* trigonometry unit conversion macros */
|
||||
|
||||
Reference in New Issue
Block a user