Skip to content

Conversation

@martonvago
Copy link
Contributor

@martonvago martonvago commented Nov 4, 2025

Description

This PR handles grouped errors at $.resources[x].schema.fields[x].constraints.enum. It's the worst one yet!

This schema node is deeply nested, so handling errors here is a little more complex.

The situation is the following: for each field, you can list the allowed values for that field under "constraints": {"enum": [...]}. Depending on the type of the field, the type of the values in enum can be different. Sometimes it's straightforward (e.g. for string fields, enum values have to be strings), but sometimes the schema for enum is more permissive (e.g. for number fields, enum values can be either numbers or strings). However, you cannot mix types.

If enum has mixed types or if some of the values are the wrong type, all sub-schemas under enum flag errors. These are usually contradictory, e.g., for a number field [1, "inf"] will alert both because 1 is not a string and because "inf" is not a number...

The logic in the function:

  • if the error is not even for the current field type -> remove all errors
  • if there are any errors on the enum property itself (not an array, duplicate items, etc.) -> keep only these
  • the remaining errors are all about the individual values in the enum array being the wrong type
    • if the values are not all the same type -> flag only this
    • if they are all the same type -> flag that this is the wrong type

Part of #15

Needs an in-depth review.

Checklist

  • Formatted Markdown
  • Ran just run-all

@martonvago martonvago self-assigned this Nov 4, 2025
@martonvago martonvago moved this from Todo to In Progress in Iteration planning Nov 4, 2025
Path components are separated by '/'.
jsonpath (str): The JSON path to the field that violates the check.
instance (Any): The part of the object that failed the check.
schema_value (Optional[Any]): The part of the schema violated by this error.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g., for type checks, this would be the name of the type. I'm using this below to list the possible types for enum values in the error message.

@martonvago martonvago moved this from In Progress to In Review in Iteration planning Nov 6, 2025
@martonvago martonvago marked this pull request as ready for review November 6, 2025 14:17
@martonvago martonvago requested a review from a team as a code owner November 6, 2025 14:17
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Just some fairly small comments ☺️

message = "All enum values must be the same type."
same_type = len(set(_map(parent_error.instance, lambda value: type(value)))) == 1
if same_type:
allowed_types = set(_map(value_errors, lambda error: str(error.schema_value)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the set() doing here? Why is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there can be more errors than allowed types.
E.g., if the allowed types are array and object and the enum array is [1, 2], then both enum items will cause errors for both allowed types. So 4 errors but only 2 allowed types.

@github-project-automation github-project-automation bot moved this from In Review to In Progress in Iteration planning Nov 7, 2025
@martonvago martonvago moved this from In Progress to In Review in Iteration planning Nov 7, 2025
@martonvago martonvago requested a review from lwjohnst86 November 7, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants