Runtime assertions with binding.assert
#341
Conversation
lib/literal.rb
Outdated
| def assert(**kwargs) | ||
| kwargs.each do |name, type| | ||
| value = local_variable_get(name) | ||
| Literal.check(value, type) |
There was a problem hiding this comment.
We can add more context to the error message since we know what the key was.
|
Things to consider adding:
PR also needs some minimal docs explaining how the method works (what error it raises if an assertion fails, what kinds of types you can pass, etc.) But, this feels like a perfect next step for Literal. I love it |
joeldrapper
force-pushed
the
push-lkwxpzkmyvyt
branch
4 times, most recently
from
4c5ee73 to
8edcea7
Compare
joeldrapper
force-pushed
the
push-lkwxpzkmyvyt
branch
from
8edcea7 to
379a078
Compare
joeldrapper
changed the title
binding.assert
| end | ||
| end | ||
|
|
||
| ::Binding.include(Literal::BindingAssert) |
There was a problem hiding this comment.
We should make a literal/pure entry point that people can use if they want to opt out of patches.
There was a problem hiding this comment.
Personally would rather opt-in than opt-out, eg literal/binding_assert
There was a problem hiding this comment.
Elaborate? It has no perf cost, no overhead. And this DX is superior to Literal.assert(binding, **constraints).
There was a problem hiding this comment.
It is mixing something in Binding, which I would find somewhat unexpected unless I explicitly wanted that feature
There was a problem hiding this comment.
I think the main use case will be people opting in. I was only thinking of adding /pure for gem authors who might not want a transient dependency to do patching.
fractaledmind
left a comment
fractaledmind
left a comment
There was a problem hiding this comment.
I think this is a great feature that needs one more pass to be ready.
| value = INSTANCE_VARIABLE_GET_METHOD.bind_call(bind.receiver, name) | ||
| end | ||
| elsif first_byte == 36 # $foo | ||
| raise if string_name.match?(/[\s\.]/) |
There was a problem hiding this comment.
Need to raise a named exception here with a clear message.
| end | ||
|
|
||
| unless type === value | ||
| raise ::TypeError.new(<<~MESSAGE) |
There was a problem hiding this comment.
Joel and I discussed that Literal::TypeError needs a refactor to be more generic to allow for simpler errors like this. Personally, I think make TypeError simple like this and move the property stuff into a PropertyError class that inherits from TypeError
Co-authored-by: Stephen Margheim <[email protected]>
This PR introduces a
Bindingpatch that allows you to make assertions about local variables at any point.You would ideally include
Literal::TypesintoObjectif you wanted to make the best use of this, since otherwise the_types are not available in the context of an instance.It would be nice if there was an equally clean way to make assertions about instance variables.