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

Resolves improper anchoring of patternProperties #783

Merged
merged 2 commits into from
May 23, 2023
Merged

Resolves improper anchoring of patternProperties #783

merged 2 commits into from
May 23, 2023

Conversation

fdutton
Copy link
Contributor

@fdutton fdutton commented May 22, 2023

Fixes #782

@fdutton fdutton linked an issue May 22, 2023 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented May 22, 2023

Codecov Report

Merging #783 (0b6f318) into master (3cf9bb6) will decrease coverage by 0.19%.
The diff coverage is 75.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff              @@
##             master     #783      +/-   ##
============================================
- Coverage     75.05%   74.87%   -0.19%     
- Complexity     1185     1191       +6     
============================================
  Files           122      122              
  Lines          4097     4103       +6     
  Branches        787      790       +3     
============================================
- Hits           3075     3072       -3     
- Misses          698      705       +7     
- Partials        324      326       +2     
Impacted Files Coverage Δ
...m/networknt/schema/regex/JDKRegularExpression.java 85.71% <75.00%> (-14.29%) ⬇️

... and 6 files with indirect coverage changes

@fdutton fdutton marked this pull request as draft May 22, 2023 11:48
@fdutton
Copy link
Contributor Author

fdutton commented May 22, 2023

This change reintroduces an issue identified in #495

@fdutton fdutton marked this pull request as ready for review May 22, 2023 22:46
@fdutton
Copy link
Contributor Author

fdutton commented May 22, 2023

@stevehu I did not make MULTILINE configurable. If this change breaks something I'll add the configuration parameter but I suspect no one is actually expecting a property name to contain a newline.

@stevehu stevehu merged commit f09740a into master May 23, 2023
@stevehu stevehu deleted the issue782 branch May 23, 2023 03:43
@Stephan202 Stephan202 mentioned this pull request Nov 3, 2023
stevehu pushed a commit that referenced this pull request Dec 2, 2023
Summary of changes:
- Fix the test resources introduced by #783 by moving the `regex`
  fields, such that the test framework does not skip them with a "Not a
  valid test case" message.
- Revert the changes introduced by #815, as those are simply incorrect.
- Extend the test coverage introduced by #815 by (a) updating the test
  regexes to match their intended semantics and (b) include a few
  negative test cases.
- Partially revert the change introduced by #783: the use of
  `Matcher#find()` is correct, but the `hasStartAnchor` and
  `hasEndAnchor` logic introduces more bugs than the issue it aims to
  solve.
- Extend the test coverage introduced by #783, by introducing regexes
  that are not covered by the `hasStartAnchor`/`hasEndAnchor` logic.
- Update the Joni regular expression integration such that it passes
  more of the test cases.
- Disable the "trailing newline" test cases, as these are currently not
  handled correctly by either regex implementation.
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.

Problem validating pattern properties
3 participants