Skip to content
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

adds changes for regex constraint implementation #96

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

desaikd
Copy link
Contributor

@desaikd desaikd commented Jul 13, 2022

Issue #9 #10

Description of changes:
This PR works on adding implementation of regex constraint.

Grammar:

<REGEX> ::= regex: <STRING>
          | regex: i::<STRING>
          | regex: m::<STRING>
          | regex: i::m::<STRING>

Ion Schema specification:
https://amzn.github.io/ion-schema/docs/spec.html#regex

List of changes:

  • adds regex crate dependency
  • adds Regex enum variants for IslConstraint and Constraint
  • adds RegexConstraint implementations
    (Note: Not all features of regex are supported as per Ion schema specification hence there is a preprocessing done on the regex to allow only the supported features)
  • reference for preprocessing regex is as per ion-schema-kotlin implementation.
  • adds validation logic for regex
  • adds ViolationCode for RegexMismatched

Tests:
added unit tests for regex implementation.

  • adds tests for programmatically creating regex constraint
  • adds tests for loading a schema using regex constraint
  • adds validation tests for regex constraint

* adds regex crate dependency
* adds Regex enum variants for IslConstraint and Constraint
* adds RegexConstraint implementations
* adds validation logic for regex
* adds ViolationCode for RegexMismatched
* adds tests for programmatically creating regex constraint
* adds tests for loading a schema using regex constraint
* adds validation tests for regex constraint
@desaikd desaikd requested a review from popematt July 13, 2022 20:10
Copy link
Contributor

@popematt popematt left a comment

Choose a reason for hiding this comment

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

This isn't a reflection on your code at all, but it's disappointing that we need handwritten code for validating the regex pattern. Maybe there's a crate that we could use to generate a parser from a grammar that can validate the regex pattern (e.g. Pest). 🤷‍♂️

That being said, I recommend you follow Clippy's advice about the loop. Otherwise it looks good to me.

src/constraint.rs Outdated Show resolved Hide resolved
src/constraint.rs Show resolved Hide resolved
@desaikd
Copy link
Contributor Author

desaikd commented Jul 14, 2022

I will open up an issue to investigate on whether something like Pest could be used here.
Thanks for the suggestion!

This isn't a reflection on your code at all, but it's disappointing that we need handwritten code for validating the regex pattern. Maybe there's a crate that we could use to generate a parser from a grammar that can validate the regex pattern (e.g. Pest). 🤷‍♂️

Issue reference: #97

* adds clippy suggestion
* adds changes to consider flags for regex equality
@desaikd desaikd merged commit af6f2b5 into main Jul 14, 2022
@desaikd desaikd deleted the desaikd-regex-constraint branch July 28, 2022 22:54
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