Let runtime library functions specify their parameter names

[oxc]

This commit is part of #2284, but given that it affects not only the Javascript system, I've decided to pull it out into a separate PR.

---

In cases where a runtime library function has multiple overloads with the same arity, the parameter names cannot be nicely resolved from the implementation function, because they have different meaning depending on the selected type signature.
Instead of hardcoding this in TypescriptDefinition, let function definition include parameter names where this makes sense.

Unfortunately, this requires some extra pass-through constructors in LibraryFunction. At least one of them can resolved once we switch to JDK22 and statements before the this() call are allowed.

These parameter names will also show up in ashref/jsref, but unfortunately the reflection-based parameter names don't show up, because KoLmafia is only compiled with the required -parameters switch for tests and tsDef (see build.gradle#L285-L289). Is there a good reason not to enable this generally?

[oxc]

Could someone please re-run the tests on windows? They pass fine here: https://github.com/oxc/kolmafia/actions/runs/8869700421/job/24350661462

[oxc]

For completeness sake, I want to provide an alternative suggestion: instead of mixing reflection and explicitly named parameters, we could just switch all runtime functions to include their parameter names. Let me know if I should add this to this PR.

[gausie]

For completeness sake, I want to provide an alternative suggestion: instead of mixing reflection and explicitly named parameters, we could just switch all runtime functions to include their parameter names. Let me know if I should add this to this PR.

I do think your proposed alternative is probably better, but not necessary for right now if you don't feel up to it.

[midgleyc]

These parameter names will also show up in ashref/jsref, but unfortunately the reflection-based parameter names don't show up, because KoLmafia is only compiled with the required -parameters switch for tests and tsDef (see build.gradle#L285-L289). Is there a good reason not to enable this generally?

My guess would be that they've just not been used until now, and enabling the flag probably increases the filesize by some amount, so it was cleaner to not do it.

I would support its addition for a nicer ashref / jsref.

[oxc]

They would already have shown up in ashref, but your guess might still be true from earlier times.

If nobody objects, I might still add namedParams to all functions, in a couple of days.

[gausie]

Please do!

…

On Tue, 30 Apr 2024 at 17:18, Bernhard Frauendienst < ***@***.***> wrote: They would already have shown up in ashref, but your guess might still be true from earlier times. If nobody objects, I might still add namedParams to all functions, in a couple of days. — Reply to this email directly, view it on GitHub <#2285 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABL4LQ6XGFQUT2X24WH7QTY7674BAVCNFSM6AAAAABG5FWSFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBVHA2DCMZRGQ> . You are receiving this because your review was requested.Message ID: ***@***.***>

[oxc]

I've added param names for all functions. I have not provided any new names (except for the "adventure" function in the last commit for demonstration purposes).
I have removed the reflection based parameter names from the code.

[gausie]

Thank you, this rocks!