Add Evaluator#schema_error? and Evaluator#rule_error?

[waiting-for-dev]

This tries to implement what has been discussed in #615

@flash-gordon @solnic please, take a look at the WIP code... the spec is failing because #error?(:email) returns false, even when it has an error added in the first rule. It's because Evaluator#error? delegates to Result#error?, which in turn delegates to Dry::Schema::Result#error?. Is it intended behaviour? I mean, we are looking just to the errors set in the schema phase and ignoring rules.

[solnic]

It's because Evaluator#error? delegates to Result#error?, which in turn delegates to Dry::Schema::Result#error?. Is it intended behaviour? I mean, we are looking just to the errors set in the schema phase and ignoring rules.

Ah, yes it is intended. I forgot this method is used for checking schema errors :( Not sure what to do with this. Maybe leave it as-is for now and introduce rule_error? instead. @flash-gordon ??

[flash-gordon]

@solnic then rule_error? should only look up rule errors but it's not clear whether it should a single-rule check or all-rules check. I would say the name suggests it's current-rule-only.

[waiting-for-dev]

I think that current name is misleading... what do you think about adding for now #schema_error? and #rule_error?, and adding a deprecation warning for #error?? I think the latest should perform both checks in a future dry-schema release...

[waiting-for-dev]

In order to keep a thin interface another option is to overload #error? method to accept a keyword argument from:, expecting it to take :schema, :rule or :all as values. To keep backward compatibility it should default to :schema, but I'd change it to :all in 2.0

[solnic]

@waiting-for-dev I was going to suggest rule_error? + schema_error? too. I think it's fine that rule_error? would check all-the-rules. We can always extend it later on to be able to target a specific rule

[flash-gordon]

I'm for it

[waiting-for-dev]

Hmm not sure I'm following here... Currently, #error? accepts a path as argument, so it's already capable of targeting a single key. Should #schema_error? be an alias of #error?. It'd make sense to me. Or are you talking about checking all keys at once? The same applies to #rule_error?. Why not accept a path as argument from the beginning?

[solnic]

Should #schema_error? be an alias of #error?

Yes, and #error? should be deprecated and removed in 2.0.0.

The same applies to #rule_error?. Why not accept a path as argument from the beginning?

My understanding is that we want to check if a rule failed, doesn't matter under which key, because a rule can fail due to multiple reasons.

@flash-gordon have you considered that ☝️ ?

[flash-gordon]

I don't think having path argument for rule_error? would be particularly useful. This is based on my experience. If you want to check if other rules failed you likely want to do it explicitly. For that, we need a means of nesting rules or something like this. If you want to check failures in the current rule then it's more likely you don't care about paths. This is what I learned from my experience. The main argument for my reasoning is adding new rules should not affect existing ones.

[waiting-for-dev]

Hey @solnic @flash-gordon you can check it now

[solnic]

Please don't use hard-wrapping in docs, it can actually screw up MD formatting AND it makes writing docs really annoying

[waiting-for-dev]

Apologies... I fixed it 😅

[solnic]

Thanks, approved but with a minor doc-related comment 😄

[flash-gordon]

Thank you @waiting-for-dev!