Skip to content

Conversation

@martonvago
Copy link
Contributor

@martonvago martonvago commented Oct 16, 2025

Description

This PR adds some logic to handle excluding required issues at a given JSON path.

The main problem here comes down to the following:
Given Exclude(type="required", jsonpath="$.resources[*].name") and Issue(type="required", jsonpath="$.resources[3].name"), how can I detect that $.resources[*].name matches $.resources[3].name? I cannot get all matches in the descriptor for $.resources[*].name because, when the field is missing, there will be none.

One approach would be to decide it based on JSON path syntax. This will get thorny quite quickly and would be a partial reimplementation of jsonpath anyway.

Here, I went with the approach of taking $.resources[3].name and building an object that has this field: {"resources": [{}, {}, {}, {"name": {}}]}. It doesn't matter what the field contains, as long as it exists. Testing $.resources[*].name on this object (using jsonpath) works nicely.

I initially thought this was very hacky, but I'm coming around to it. But if anyone has better ideas, don't hold back!

Closes #121

Needs an in-depth review.

Checklist

  • Formatted Markdown
  • Ran just run-all

@martonvago martonvago moved this from Todo to In Review in Iteration planning Oct 16, 2025
@martonvago martonvago marked this pull request as ready for review October 16, 2025 12:36
@martonvago martonvago requested a review from a team as a code owner October 16, 2025 12:36
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.

Just some initial comments and questions before doing a deeper dive ☺️

@github-project-automation github-project-automation bot moved this from In Review to In Progress in Iteration planning Oct 21, 2025
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.

Hmm, ok, there's more confusion then 🤔 😵‍💫

@martonvago martonvago moved this from In Progress to In Review in Iteration planning Oct 27, 2025
@martonvago martonvago requested a review from lwjohnst86 October 27, 2025 15:53
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.

Great progress!! 🎉 🎉 some minor edits and comments, otherwise 👍

Comment on lines +124 to +126
def _get_array_parts(path_part: str) -> Optional[re.Match[str]]:
"""Extract the name and index from a JSON path part representing an array."""
return re.search(r"(\w+)\[(\d+)\]$", path_part)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this what you meant?

@martonvago martonvago moved this from In Progress to In Review in Iteration planning Oct 28, 2025
@martonvago martonvago requested a review from lwjohnst86 October 28, 2025 12:03
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.

Excluding required rules

3 participants