Skip to content

Add $allowEmpty parameter to URL validator#7

Merged
Meldiron merged 3 commits intomainfrom
copilot/add-empty-parameter-to-url-validator
Apr 27, 2026
Merged

Add $allowEmpty parameter to URL validator#7
Meldiron merged 3 commits intomainfrom
copilot/add-empty-parameter-to-url-validator

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

The URL validator had no way to permit empty values, making it impossible to use for optional URL fields without wrapping it.

Changes

  • URL.php: Added bool $allowEmpty = false constructor parameter. When true, isValid() short-circuits to true for '' and null. Uses strict equality (=== '', === null) rather than empty() to avoid false positives on '0'/0.
  • URLTest.php: Added testAllowEmpty() covering empty/null pass-through, valid URL still accepted, invalid non-empty URL still rejected, and default behavior (empty disallowed).

Usage

// Default — empty values rejected (no behaviour change)
$validator = new URL();
$validator->isValid('');  // false

// Opt-in — empty values allowed
$validator = new URL(allowEmpty: true);
$validator->isValid('');                   // true
$validator->isValid(null);                 // true
$validator->isValid('https://example.com'); // true
$validator->isValid('not-a-url');          // false

// Works alongside scheme restrictions
$validator = new URL(['https'], allowEmpty: true);

Comment thread src/Validator/URL.php Outdated
Comment thread tests/Validator/URLTest.php Outdated
Co-authored-by: Matej Bačo <matejbaco2000@gmail.com>
Copy link
Copy Markdown
Contributor

@Meldiron Meldiron left a comment

Choose a reason for hiding this comment

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

lgtm

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

This PR adds an $allowEmpty parameter to the URL validator so optional URL fields can pass through without being wrapped. The implementation is largely sound, but there is a discrepancy between the documented behaviour and the code: the PR description states isValid(null) returns true when allowEmpty: true, yet the guard only checks $value === '', and the test asserts false for null even in the allowEmpty path. The intended handling of null needs to be settled and made consistent across the implementation, tests, and documentation.

Confidence Score: 3/5

Not safe to merge until the null behaviour is clarified and made consistent between implementation, tests, and PR description.

A P1 inconsistency exists between the documented contract (null returns true with allowEmpty: true) and the actual code + test (null returns false). This must be resolved before the change is reliable for callers who pass null to optional URL fields.

src/Validator/URL.php (line 56) and tests/Validator/URLTest.php (line 60) — both need to be updated once the intended null behaviour is decided.

Important Files Changed

Filename Overview
src/Validator/URL.php Adds $allowEmpty constructor parameter; null is not short-circuited despite being listed as allowed in the PR description.
tests/Validator/URLTest.php New testAllowEmpty() added; assertion on line 60 contradicts the PR description's documented behaviour for null with allowEmpty: true.

Reviews (1): Last reviewed commit: "Apply suggestions from code review" | Re-trigger Greptile

Comment thread src/Validator/URL.php
@Meldiron Meldiron merged commit 6cce9f7 into main Apr 27, 2026
4 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.

2 participants