Skip to content

fix: Ensure unmatched optional capture group is treated as null in parseRowColumnSelection#578

Open
zigzagdev wants to merge 2 commits into
thephpleague:masterfrom
zigzagdev:fix/parse-row-column-selection
Open

fix: Ensure unmatched optional capture group is treated as null in parseRowColumnSelection#578
zigzagdev wants to merge 2 commits into
thephpleague:masterfrom
zigzagdev:fix/parse-row-column-selection

Conversation

@zigzagdev
Copy link
Copy Markdown
Contributor

Summary

What I have done

  • Added PREG_UNMATCHED_AS_NULL flag to the preg_match call in parseRowColumnSelection()
  • Without this flag, PHP sets unmatched named capturing groups to '' (empty string) rather than leaving them absent, causing $found['end'] ?? null to return ''instead of null
  • This bug caused single-number selections (e.g. row=1, col=2) to always fail validation when combined with semicolon-separated expressions (e.g. row=1;3, col=1;3)

Where I have changed

  • src/FragmentFinder.php: Add PREG_UNMATCHED_AS_NULL to preg_match and simplify $end assignment from $found['end'] ?? null to $found['end']
  • src/TabularDataReaderTestCase.php: Add test cases covering following cases
    • Multiple single-number row/column selections (row=1;3, col=1;3)
    • Mixed single and range selections (row=1;3-5)
    • All-invalid single selections (row=0;0)
    • Mixed valid/invalid selections — matchingFirst returns partial results, matchingFirstOrFail throws FragmentNotFound

Test Plan

Expression What is verified
row=1;3 Two single-number row selections both resolve correctly without a range endpoint
row=1;3-5 A single-number selection and a range selection can coexist in one expression
col=1;3 Two single-number column selections both resolve correctly without a range endpoint
row=0;0 All-invalid single selections yield null from matchingFirst and throw from matchingFirstOrFail
row=0;3 (×2) When valid and invalid selections are mixed, matchingFirst returns only the valid result while matchingFirstOrFail throws FragmentNotFound

Test Cases Result

screen shot 2026-05-29 8 57 44

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.

1 participant