Skip to content

Fix #822: Validate rules inside StopOnError, Composite and AnyRule in the current scope - #824

Open
vjik wants to merge 3 commits into
masterfrom
fix-822
Open

vjik wants to merge 3 commits into
masterfrom
fix-822

Conversation

@vjik

@vjik vjik commented Sep 25, 2026

Copy link
Copy Markdown
Member
Q A
Is bugfix? ✔️
New feature? ✔️
Breaks BC? ❌
Fix #822

@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.37%. Comparing base (cde9506) to head (aa3839b).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #824      +/-   ##
============================================
+ Coverage     96.35%   96.37%   +0.02%     
- Complexity     1152     1158       +6     
============================================
  Files           124      124              
  Lines          3540     3563      +23     
============================================
+ Hits           3411     3434      +23     
  Misses          129      129              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Context parameters are not restored when current-scope validation throws an exception.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes #822 by validating wrapped rules within the existing validation scope.

Changes:

  • Adds ValidationContext::validateInCurrentScope().
  • Updates three rule wrappers to preserve dataset and property context.
  • Adds documentation and regression tests.
File Description
src/​ValidationContext.php Adds current-scope validation API.
src/​Validator.php Implements scoped rule validation and error paths.
src/​Rule/​StopOnErrorHandler.php Uses current-scope validation.
src/​Rule/​CompositeHandler.php Uses current-scope validation.
src/​Rule/​AnyRuleHandler.php Uses current-scope validation.
tests/​ValidationContextTest.php Tests the new context API.
tests/​Rule/​StopOnErrorTest.php Adds scope regression coverage.
tests/​Rule/​CompositeTest.php Adds scope regression coverage.
tests/​Rule/​AnyRuleTest.php Adds scope regression coverage.
tests/​Support/​Data/​PostValidationHookCounter.php Supports hook invocation tests.
docs/​guide/​en/​creating-custom-rules.md Documents current-scope validation.
CHANGELOG.md Records the feature and bug fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ValidationContext.php Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The implementation consistently preserves validation scope and includes comprehensive regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@vjik
vjik requested a review from a team September 25, 2026 08:44
@vjik vjik added the status:code review The pull request needs review. label Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dataset is not passed to context in StopOnError()

2 participants