Skip to content

feat: Notify Invalid RegExp Patterns#261

Merged
aeschli merged 7 commits intomicrosoft:mainfrom
aedenmurray:feature/notify-invalid
Jun 6, 2025
Merged

feat: Notify Invalid RegExp Patterns#261
aeschli merged 7 commits intomicrosoft:mainfrom
aedenmurray:feature/notify-invalid

Conversation

@aedenmurray
Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

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

Comment thread src/test/parser.test.ts Outdated
@aeschli
Copy link
Copy Markdown
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
Copy Markdown
Contributor 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.

@vs-code-engineering vs-code-engineering bot added this to the April 2025 milestone Apr 29, 2025
@aeschli aeschli merged commit 4d43e6e into microsoft:main Jun 6, 2025
3 checks passed
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.

4 participants