Skip to content

feat: Notify Invalid RegExp Patterns #261

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

aedenmurray
Copy link

This PR allows users to be notified when an invalid regex pattern is encountered.
This is useful if a JSON schema is published that contains patterns that are not supported by new RegExp().
For example, as of time of writing, the version pattern for the composer.json schema includes a possessive quantifier (*+) that is not supported by JavaScript (see composer.json schema definition).

@aedenmurray
Copy link
Author

@microsoft-github-policy-service agree company="Shadowscape, Inc."

@aeschli
Copy link
Collaborator

aeschli commented Apr 25, 2025

The JSON schema spec says that regexes must match the Regular Expression specification from JavaScript. https://json-schema.org/understanding-json-schema/reference/regular_expressions. So likely this is an issue with the composer.json

The error should be shown on the schema document and not on the document that is being validated.
I would say it's correct to ignore such regexes.
If we want to show a problem, then it would be a warning, and the message needs to be clear that this is an issue of the schema.

Something like 'Unable to validate value. Pattern {0} provided by the schema is not supported.'

@aedenmurray
Copy link
Author

Thanks @aeschli!
You are correct - this is an issue with composer.json.
However, the current implementation attempts to blame the user, which is misleading:
Screenshot 2025-04-25 at 7 55 29 PM
I agree with the sentiment that invalid patterns should be ignored.
I have updated the PR to match.

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