Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NoMethodError when trying to get AST of a Builder's result. #107

Merged
merged 3 commits into from
Feb 14, 2023

Conversation

estum
Copy link
Contributor

@estum estum commented Jan 15, 2023

In the first pair of commits of the PR there are a couple of shared examples and updates of the Dry::Logic::Builder's unit specs which reproduces a raised exception in scenarios like the following :

Dry::Logic::Builder.() { key?(:speed) }.to_ast
#     NoMethodError:
#       undefined method `name' for #<Proc:0x000000010c8982a0 (lambda)>
#     
#                 predicate.name
#                          ^^^^^

This is fixed by an avoiding of extra predicate to proc coercion in Dry::Logic::Builder::Context#initialize. So now it is just passed into the Dry::Logic::Builder::Context#predicate which lets to keep a predicate's name.

estum added 2 commits January 15, 2023 03:33
To describe AST related behavior of rules produced by Dry::Logic::Builder
Now it expectedly fails with NoMethodError produced by ./lib/dry/logic/rule/predicate.rb:16:in `name', but the main cause is how the Builder yields a predicate into context.
@estum estum force-pushed the feature/ast-builder branch from d493b39 to ec6d2b1 Compare January 15, 2023 20:39
To keep a predicate's name and resolve an issue with ast of predicates produced by Dry::Logic::Builder.
@estum estum force-pushed the feature/ast-builder branch from fcc7887 to 6d3c70f Compare January 16, 2023 20:23
@estum
Copy link
Contributor Author

estum commented Feb 5, 2023

👀

@solnic solnic merged commit ba1767e into dry-rb:main Feb 14, 2023
@solnic
Copy link
Member

solnic commented Feb 14, 2023

@estum thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants