Treat invalid pattern property as unevaluated

[ikonst]

This addresses an "error message" issue, not a validation issue, so no need to update the test suite.

For regular properties, a property which failed validation would be also deemed "unevaluated", thus resulting in two messages, e.g.

validate(
    schema={"properties": {"foo": {"type": "string"}}},
    instance={"foo": 42},
)

would result in 2 errors:

• 42 is not of type 'string'
• unevaluated property 'foo'

With this change, pattern properties would have the same behavior:

validate(
    schema={"patternProperties": {"^foo": {"type": "string"}}},
    instance={"foo": 42},
)

---

📚 Documentation preview 📚: https://python-jsonschema--1073.org.readthedocs.build/en/1073/

[Julian]

Thanks for the PR. Though as I said I'm not sure I'll be comfortable merging it strictly for improving the error at the minute -- unevaluatedProperties already does lots of unnecessary work (re-walking schemas) which is in need of fixing to support JSON Schema's "new computation model" that includes stateful keywords in core. Doing a whole second validation pass is going to be even more inefficient, not to mention that unevaluatedProperties is used in the default dialects' metaschemas, so that performance penalty is going to be paid everywhere essentially, for any schemas. Way better is finally addressing that unfortunate truth (needing state during validation now), which will change how evaluating unevaluatedProperties looks in the codebase.

I'll re-evaluate after 4.18 is out to decide, so I won't close this one way or the other yet -- so acking that I saw this, and benchmarking some schemas certainly couldn't hurt, but don't feel offended if I close.

[ikonst]

@Julian

I'm confused about what it does right now, though.

Given my example, it'll be validating instance {"foo": 42} against schema {"^foo": {"type": "number"}} that doesn't look like a valid schema, and in fact valid for all inputs. For sake of argument, if it's valid for all inputs, why call is_valid at all?

[Julian]

Oh I see this isn't related to #1068 -- I hadn't looked at the example carefully and assumed it was what we were previously discussing. OK, I'll still probably not be able to focus on this until after 4.18, but if there's some actual bug and this doesn't add new performance impact that's a lot more likely to be mergeable.

[ikonst]

I think it's a pretty straightforward change (what's there right now looks like a mix-up).

Then again, if you want to validate less, maybe instead of this PR you'd just want to remove the is_valids since the tests seem to pass even in their absence:
#1075

[ikonst]

Replaced by #1075