ExpressionParser: Show error message with expected arguments.

3 files changed, 102 insertions, 34 deletions

diff --git a/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp b/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp
index f7937a19bd..0d1d0fdbea 100644
--- a/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp
+++ b/Source/Core/InputCommon/ControlReference/ExpressionParser.cpp
@@ -469,7 +469,8 @@ private:
     return tok.type == type;
   }
 
-  ParseResult ParseFunctionArguments(std::unique_ptr<FunctionExpression>&& func,
+  ParseResult ParseFunctionArguments(const std::string_view& func_name,
+                                     std::unique_ptr<FunctionExpression>&& func,
                                      const Token& func_tok)
   {
     std::vector<std::unique_ptr<Expression>> args;
@@ -518,10 +519,15 @@ private:
       }
     }
 
-    if (!func->SetArguments(std::move(args)))
+    const auto argument_validation = func->SetArguments(std::move(args));
+
+    if (std::holds_alternative<FunctionExpression::ExpectedArguments>(argument_validation))
     {
-      // TODO: It would be nice to output how many arguments are expected.
-      return ParseResult::MakeErrorResult(func_tok, _trans("Wrong number of arguments."));
+      const auto text = std::string(func_name) + '(' +
+                        std::get<FunctionExpression::ExpectedArguments>(argument_validation).text +
+                        ')';
+
+      return ParseResult::MakeErrorResult(func_tok, _trans("Expected arguments: " + text));
     }
 
     return ParseResult::MakeSuccessfulResult(std::move(func));
@@ -543,7 +549,7 @@ private:
         return ParseAtom(control_tok);
       }
 
-      return ParseFunctionArguments(std::move(func), tok);
+      return ParseFunctionArguments(tok.data, std::move(func), tok);
     }
     case TOK_CONTROL:
     {
@@ -553,7 +559,7 @@ private:
     }
     case TOK_NOT:
     {
-      return ParseFunctionArguments(MakeFunctionExpression("not"), tok);
+      return ParseFunctionArguments("not", MakeFunctionExpression("not"), tok);
     }
     case TOK_LITERAL:
     {
@@ -571,7 +577,7 @@ private:
     {
       // An atom was expected but we got a subtraction symbol.
       // Interpret it as a unary minus function.
-      return ParseFunctionArguments(MakeFunctionExpression("minus"), tok);
+      return ParseFunctionArguments("minus", MakeFunctionExpression("minus"), tok);
     }
     default:
     {
diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp
index daf36c0fa1..18b98c5e19 100644
--- a/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp
+++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.cpp
@@ -21,10 +21,14 @@ using FSec = std::chrono::duration<ControlState>;
 class ToggleExpression : public FunctionExpression
 {
 private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
     // Optional 2nd argument for clearing state:
-    return 1 == args.size() || 2 == args.size();
+    if (1 == args.size() || 2 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"toggle_state_input, [clear_state_input]"};
   }
 
   ControlState GetValue() const override
@@ -57,9 +61,13 @@ private:
 class NotExpression : public FunctionExpression
 {
 private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 1 == args.size();
+    if (1 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"expression"};
   }
 
   ControlState GetValue() const override { return 1.0 - GetArg(0).GetValue(); }
@@ -70,9 +78,13 @@ private:
 class SinExpression : public FunctionExpression
 {
 private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 1 == args.size();
+    if (1 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"expression"};
   }
 
   ControlState GetValue() const override { return std::sin(GetArg(0).GetValue()); }
@@ -82,9 +94,13 @@ private:
 class TimerExpression : public FunctionExpression
 {
 private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 1 == args.size();
+    if (1 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"seconds"};
   }
 
   ControlState GetValue() const override
@@ -121,9 +137,13 @@ private:
 class IfExpression : public FunctionExpression
 {
 private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 3 == args.size();
+    if (3 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"condition, true_expression, false_expression"};
   }
 
   ControlState GetValue() const override
@@ -137,9 +157,13 @@ private:
 class UnaryMinusExpression : public FunctionExpression
 {
 private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 1 == args.size();
+    if (1 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"expression"};
   }
 
   ControlState GetValue() const override
@@ -152,9 +176,13 @@ private:
 // usage: deadzone(input, amount)
 class DeadzoneExpression : public FunctionExpression
 {
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 2 == args.size();
+    if (2 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"input, amount"};
   }
 
   ControlState GetValue() const override
@@ -169,9 +197,13 @@ class DeadzoneExpression : public FunctionExpression
 // seconds is seconds to change from 0.0 to 1.0
 class SmoothExpression : public FunctionExpression
 {
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 2 == args.size() || 3 == args.size();
+    if (2 == args.size() || 3 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"input, seconds_up, seconds_down = seconds_up"};
   }
 
   ControlState GetValue() const override
@@ -209,9 +241,13 @@ private:
 // usage: hold(input, seconds)
 class HoldExpression : public FunctionExpression
 {
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 2 == args.size();
+    if (2 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"input, seconds"};
   }
 
   ControlState GetValue() const override
@@ -244,9 +280,13 @@ private:
 // usage: tap(input, seconds, taps=2)
 class TapExpression : public FunctionExpression
 {
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 2 == args.size() || 3 == args.size();
+    if (2 == args.size() || 3 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"input, seconds, taps = 2"};
   }
 
   ControlState GetValue() const override
@@ -300,9 +340,13 @@ private:
 // speed is max movement per second
 class RelativeExpression : public FunctionExpression
 {
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return args.size() >= 2 && args.size() <= 4;
+    if (args.size() >= 2 && args.size() <= 4)
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"input, speed, [max_abs_value, [shared_state]]"};
   }
 
   ControlState GetValue() const override
@@ -355,9 +399,13 @@ private:
 // usage: pulse(input, seconds)
 class PulseExpression : public FunctionExpression
 {
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
+  ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
   {
-    return 2 == args.size();
+    if (2 == args.size())
+      return ArgumentsAreValid{};
+    else
+      return ExpectedArguments{"input, seconds"};
   }
 
   ControlState GetValue() const override
@@ -447,7 +495,8 @@ void FunctionExpression::UpdateReferences(ControlEnvironment& env)
     arg->UpdateReferences(env);
 }
 
-bool FunctionExpression::SetArguments(std::vector<std::unique_ptr<Expression>>&& args)
+FunctionExpression::ArgumentValidation
+FunctionExpression::SetArguments(std::vector<std::unique_ptr<Expression>>&& args)
 {
   m_args = std::move(args);
 
diff --git a/Source/Core/InputCommon/ControlReference/FunctionExpression.h b/Source/Core/InputCommon/ControlReference/FunctionExpression.h
index 14b438a299..f5332a0d25 100644
--- a/Source/Core/InputCommon/ControlReference/FunctionExpression.h
+++ b/Source/Core/InputCommon/ControlReference/FunctionExpression.h
@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <string>
+#include <variant>
 #include <vector>
 
 #include "InputCommon/ControlReference/ExpressionParser.h"
@@ -18,15 +19,27 @@ namespace ExpressionParser
 class FunctionExpression : public Expression
 {
 public:
+  struct ArgumentsAreValid
+  {
+  };
+
+  struct ExpectedArguments
+  {
+    std::string text;
+  };
+
+  using ArgumentValidation = std::variant<ArgumentsAreValid, ExpectedArguments>;
+
   int CountNumControls() const override;
   void UpdateReferences(ControlEnvironment& env) override;
 
-  bool SetArguments(std::vector<std::unique_ptr<Expression>>&& args);
+  ArgumentValidation SetArguments(std::vector<std::unique_ptr<Expression>>&& args);
 
   void SetValue(ControlState value) override;
 
 protected:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) = 0;
+  virtual ArgumentValidation
+  ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) = 0;
 
   Expression& GetArg(u32 number);
   const Expression& GetArg(u32 number) const;