More helpful error messages for incorrect # of parameters

[Merudo]

Is your feature request related to a problem? Please describe.
Error messages for incorrect number of parameters can be unnecessarily cryptic. Example:

listGet() and input() both give the same message:

Invalid number of parameters 0, expected at least 1 parameters

which doesn't tell the user the name of the problematic function. Similarly, hasSight(1,2,3,4) returns

Invalid number of parameters 4, expected between 0 and 3 parameters

Describe the solution you'd like
The error message gives the name of the function that caused the error. For example, listGet() would return

Function "listGet" requires at least 1 parameters; 0 were provided.

which is immensely more helpful for debugging.

Additional context
Issue is caused by checkParameters in the Parser not displaying the name of the function.

I think we should either modify checkParameters so that it displays the function name, or remove the call entirely and let each individual function deal with checking the number of parameters.

The later may make more sense, since each aliases in a function class needs a different number of parameters.

Related issue: #613

[Azhrei]

So, I took a step back... Perhaps we need a Param class? It could use chaining to verify parameters and provide useful error messages. Something like this:

Param p = new Param(funcName, args);
if (!p.min(2, "msg.text.tooFew")               // all msgtext's include function name
      .max(4, "msg.text.tooMany")              //   and values shown here
      .isBool(0, "msg.text.mustBeBool")        // this one includes param number
      .isNumber(1, "msg.text.mustBeNumber")
      .lastIsToken()) {
        return null;    // indicates something didn't work
}

I'm not sure what to do about functions whose signature allows different data types in a given position, such as where the first param can be a number or a string? Maybe it requires more than one IF statement, or maybe someone more familiar with all of the functions can see a pattern that can be boiled down to one or two methods for this Param class?

Last, should this be discussed on Discord or here? Here is Google-searchable, but until this is fleshed out, maybe Discord is better?

[Merudo]

I'm not sure what to do about functions whose signature allows different data types in a given position, such as where the first param can be a number or a string?

There is already an example of such function in the code, although I can't find it at the moment.

Basically it takes the parameter array among with an array of class types and compares the two, raising an exception if any discrepancy is found.

EDIT: Found it, it's checkVaryingParameters

[Merudo]

I gave it more thought and think some sort of ParameterList class extending List could be appropriate.

The min & max could be checked at construction, for example

ParameterList paramList = new ParameterList(functName, parameters, min, max)

where parameters is a list of parameters, min is the minimum number of parameters, and max is the max number of parameters. If the min or max doesn't match up, an exception is thrown.

We could also have functions getTokenFromParam(index), parseInteger(index), parseBoolean(index) and asJSON(index) be methods from that class. These methods would throw the appropriate exception if needed.

[JamzTheMan]

Mmm, I think I like this.

Is there room to make it smarter? Maybe have it handle tokens a little better? I wouldn't mind a getToken() that just knew and returned me the token doing all the "if token passed otherwise get from context" stuff vs having to use an index everywhere.

Maybe there is a setup up front that you can specify parameter expected indexes, the class types, and a key name that store in a hashmap or such with defaults.

Then it's getParameter(String key)

I suppose that may now be it's own Parameter type class.

Just tossing ideas out. I like the idea but I can also see myself constantly asking myself "wait, what index do I need again?"

[Azhrei]

Yeah, agreed. The checkVaryingParameters function is pretty nice, but it doesn't do type conversion. It'll check that the Nth parameter is a BigDecimal, but it won't convert a String passed in as parameter N to be a BigDecimal. For that you'd need a list of conversions from String to Class<?>.

We only have a few types, so it'd be a short list of lambdas to convert an input type into String, BigDecimal, Boolean, and Token (and maybe JSONObject/JSONArray?).

I'm curious to see the proposals for a better way to handle this. These kinds of problems are fun to solve, IMO. 😉

[Phergus]

Tested. Incorrect number of parameters messages now include name of macro function used. Closing.