Add standardised patching framework

[ruipin]

Inspired by some modules like Mess which patch methods using a regex, I have been wondering about standardising an API to do this officially supported by libWrapper, and that attempts to handle possible issues such as conflicts.

This API could be e.g.

libWrapper.patch("module-id", "path.to.method", /match/, "replace");

and for ease-of-use, also support

libWrapper.patch("module-id", "path.to.method",
  [/match1/, "replace1"],
  [/match2/, "replace2"],
  ...,
  [/matchN/, "replaceN"]
);

Each pair would behave exactly like a call to String.prototype.replace, so would also support replacement methods, etc.

libWrapper would then be responsible for working out which method to patch (including, if necessary, modifying a prior patch), and automatically setting up a corresponding OVERRIDE.

• If there's already an override, this method would fail, unless the given module has a higher priority, in which case the existing override gets discarded (and a conflict is signalled).
• Someone registering an OVERRIDE that is higher priority than all of the modules that set up a patch will cause the patch to be discarded, similar to the current behaviour when there are two OVERRIDE conflicts.
• If libWrapper cannot resolve the function to source code, it throws an exception.
• If any individual patch "pair" leaves the source code unchanged, libWrapper throws an exception. A third parameter can be used to signal that patches are not required.
  • The exception is if the patch is being applied on top of an already patched method, and the current module is higher priority than the existing module(s). In such a case, the existing patches are dropped, and the new module takes over.
• If the final source code is unchanged from the original (i.e. none of the patch pairs applied successfuly), libWrapper throws an exception.
• If the final source code fails to parse, libWrapper throws an exception.
• Patches (or to be more exact, the corresponding OVERRIDE) cannot be unregistered.
• The 🎁 emoji will be prefixed to the patched function names, to make it obvious in the call stack the method has been modified.

This would mean that two modules patching the same method, as long as their regexes don't overlap, would still be compatible.

Modules would be responsible for catching the exceptions if they require fall-back behaviour. As usual, uncaught exceptions will be raised to the user as errors.

The shim could be given a naive fallback implementation, that just does toString(), replace, and then overwrites the original.

[manuelVo]

Having done some patching in Drag Ruler I'd like to share some insights I gained through that process.

Here are my thoughts about the API:

• I think it should be possible for multiple modules to patch the same function. I'd expect patches (especially if they are small) to be compatible and easily to apply on top of each other (similarly to how git auto-merges changes in different locations).
• A way to insert after or before the regex (in addition to replacing) might be useful
• A way to add additional parameters to an existing function would be neat
• From the module authors side, just listing a bunch of patches that'll be applied to the function are hell to work with, because you won't know what the final code really looks like without dumping it into the console. As an alternative it would probably neat if a patch-file could be supplied instead of a list of replacements. That way the author could work on the actual function (which is obviously way easier) and just ship the patch file. That's just an idea that crossed my mind during patching though. I have no concept of how such an API could look like and if it would actually lead to a better workflow.

Here are some pitfalls to be aware of when patching other functions:

• The output of toString for a function is in no way normalized. This includes
  • Most Foundry installations seem to be using LF as line ending in foundry.js (Linux, MacOS and some Windows installations do), while some Windows installations use CRLF. This will apply to the string returned by toString as well. It's probably a good idea to replace all occurrences of CRLF with LF before patching so modules know what they'll have to put up with.
  • Depending on how a function was created, the function signature in the string looks different. For example the signature of Ruler._highlightMeasurement will look like this natively _highlightMeasurement(ray) {, but will like this after Drag Ruler patched it function anonymous(ray,startDistance=undefined\n) { (the \n is actually a line break)
• Not every string returned by Function.toString() will be accepted by new Function() to create a new function. Especially function signatures may need to be transformed (or removed, together with the trailing } at the end of the function) so new Function() will consider the coe valid. For example _highlightMeasurement(ray) { isn't a valid function signature to use in code passed to new Function().
• If a function uses symbols that are not available in the global scope the patched function will fail to execute every time it's called, because the used symbols aren't available. This is especially likely to happen if the function that should be patched has already been overwritten by another module.

[ruipin]

@manuelVo Thanks for your comments. They are extremely useful.

I think it should be possible for multiple modules to patch the same function [...]

Yeah, the proposal above would allow multiple modules to patch the same function as long as there's no overlap.

A way to insert after or before the regex (in addition to replacing) might be useful A way to add additional parameters to an existing function would be neat [...] As an alternative it would probably neat if a patch-file could be supplied instead of a list of replacements. [...]

Good points. I'm thinking to change the pairs to tuples, where the first entry is the type of operation.

We could have:

1. replace/replaceAll: Calls the corresponding JS String.prototype functions
2. prepend/prependAll: Inserts text before the first match / all matches
3. append/appendAll: Inserts text after the first match / all matches
4. diff: Applies a patch, using diff syntax. Probably would use a node module like diff

Due to the extra complexities, it's possible 4 would be added later, and not with the first implementation of this API.

The output of toString for a function is in no way normalized [...] Especially function signatures may need to be transformed [...]

I hadn't thought of the line-breaks, you're right this should be normalised. The function signature, I'm aware.

My plan was to extract the parameters and body and then recreate the signature myself. I would also clean up the string (e.g. remove comments, unify line breaks, deindent). The problem here is that this can't be done with regex, as there could be comments before the start of the body that contain { or (, or e.g. options = {} as a parameter. Using something like decomment might be feasible. Alternatively, a full-blown javascript parser might be the only choice here, e.g. esprima. More investigation is necessary.

An API would be provided for people to use to obtain the libWrapper string representation of a method, onto which they'd be applying their patches.

If a function uses symbols that are not available in the global scope the patched function will fail to execute [...]

This is a big problem, I think. You're completely right. I don't think there's any way around this, unfortunately - can't get a function's scope. Fortunately I don't think FVTT relies on scoped variables very often, but it's certainly something to document.

This is especially likely to happen if the function that should be patched has already been overwritten by another module.

Not a big issue with libWrapper, since either we're patching the wrong code (and the patch would fail anyway), or the function is only wrapped by libWrapper in which case we know every single wrapper, and what the original method is.

[akrigline]

I know this isn't a super high priority feature given its relatively limited uses, but I want to thank both of you for the documentation of the challenges here as they helped me in my quest to implement such a patching utility myself. I won't say I'm proud of my solution as it's very cobbled together, but it's something that would have been harder still to cobble together without your notes on the subject.