leafo / tableshape

Test the shape or structure of a Lua table, inspired by React.PropTypes & LPeg
111 stars 9 forks source link

Do you use repair mode? #7

Closed leafo closed 6 years ago

leafo commented 6 years ago

The current repair mode is very limited. I started to rewrite it but I realized that repairing a object is more like a series of steps instead of an individual transformation.

I want to deprecate the current repair mode and build something that works more like lpeg where you chain operations to repair using overloaded operators.

I'm creating this topic in the hopes that anyone who sees it and uses this library can tell me if and how they are using repair mode.

leafo commented 6 years ago

Just collecting examples of repair mode...

Lets say you're receiving a coordinate input like this:

{ coord = "1,5"}

and the final shape you want is:

{ cord = {x = 1, y = 5}}

and the coordinate is also optional

Right now you could do

types.shape({
  cord = types.shape({x = types.number, y = types.number}):on_repair(function(v)
    local x,y = v:match("^(%d+),(%d+)$")
    return { x = x, y = y}
  end):is_optional()
})

The problem with this is that you need to pretty much write a failing shape so you can provide an on_repair method. The return value of on_repair isn't validated to match the original shape either, so it doesn't really have anything to do with the shape that held the method.

It makes more sense to have repair be it's own kind of node, instead of a property of all shapes.

Another problem with the above is that the repair function now has to check types, the example doesn't verify that the argument is a string type. And it does't handle the case where the pattern is wrong. Does a return value from repair mean it should convert the value to nil or should it throw an error?

Conclusion: repair system is fundamentally flawed, and that's not even considering how the repair callbacks can work subtly different for composite types.

leafo commented 6 years ago

Here's updated theoretical syntax that might be better

types.shape {
  cord = types.nil + (types.table + types.string / function(v)
    local x,y = v:match("^(%d+),(%d+)$")
    if not x then return end
    return { x = x, y = y}
  end) * types.shape { x = types.number, y = types.number }
}

In this example:

First we check if nil, if that passes then we are done. Otherwise cord must either be a table or a string. If it's a string then a transformation function is applied to it with /. Then the resulting value is passed to the final shape that checks if it's the table we want. The transform function can return nil if it fails, then the following shape check will fail.

This approach is much more robust, but returning a relevant error message might be a challenge.

leafo commented 6 years ago

For handling errors I think returning the first error, with additional message for compound types like + makes sense as a default option. There's no way to really create meaningful errors without it, so I think creating an operator to force override the error message to a string of your choice makes the most sense. I'm wondering if ^ should be used for that, or should I save it for something else. The precedence is high though, so parens are needed when chaining +, but I don't know if that's a more common use case than being specific about it. another option is minus (but then I can't use it for the lpeg equivalent

(types.nil + types.one_of {"red", "blue", "green"}) ^ "Invalid color"

types.nil + types.one_of { "red", "blue", "green"} - "invalid color"
leafo commented 6 years ago

In regards to error messages, I forgot there was a describe option that's available to describe what something is. That might be a better approach to building errors

(types.nil + types.one_of {"red", "blue", "green"}):describe("color")

I'm not sure that would give enough information to build error messages that tell you what's wrong

leafo commented 6 years ago

So I implemented transform mode for the next version, and I wanted to keep repair in there as deprecated functionality. I started going through the code seeing if I could clean it up with a repair type node, but there are so many edge cases coded into individual repair methods (that aren't even documented), it was too annoying to generalize it without breaking backwards compatibility. If I'm going to break backwards compatibility then I might as well just remove it since it's deprecated!

You can get 80% of the way there with a simple transform though, so I've replaced the on_repair method to just use transformations and return: @ + types.any / repair_fn * @.

This means: return the value if it matches, for anything else, transform with the repair function and test again with the original type.

And then the repair method is just an alias for transform.

leafo commented 6 years ago

This is now deployed to master. The biggest outstanding thing is how to handle extra fields in a shape object.

A few options:

leafo commented 6 years ago

I implemented extra_fields option to shape types that passes a single tuple table to a type checker to transform or validation.

The readme has been updated to reflect all the new functionality.

Everyone regarding transformations is now pushed to master! Good job me.