8000 DateComparator : operator does not fall back to its default value · Issue #59314 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

DateComparator : operator does not fall back to its default value #59314

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.

Sign up for GitHub

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

Jump to bottom
Closed
GlucNAc opened this issue Dec 28, 2024 · 0 comments
Closed

DateComparator : operator does not fall back to its default value #59314

GlucNAc opened this issue Dec 28, 2024 · 0 comments
Labels
Bug DX DX = Developer eXperience (anything that improves the experience of using Symfony) Finder Status: Needs Review

Comments

@GlucNAc
Copy link
GlucNAc commented Dec 28, 2024

Symfony version(s) affected

6.4.0

Description

This "bug" is present since version 2.1, so not sure if it's really a bug, but I think something must be done regarding at least the DX of the exception message thrown by the use case below.

How to reproduce

use Symfony\Component\Finder\Comparator\DateComparator;

new DateComparator('yesterday');

// leads to:
// Uncaught InvalidArgumentException: Invalid operator "".
// (thrown by base class Symfony\Component\Finder\Comparator\Comparator)

Possible Solution

In Symfony\Component\Finder\Comparator\DateComparator constructor, we could replace

$operator = $matches[1] ?? '==';

by

$operator = $matches[1] ?: '==';

as in this case

$matches[1] === ""

And maybe we could improve the Symfony\Component\Finder\Comparator\Comparator constructor signature with this anotation (to help phpstan raising error):

/**
 * @param non-empty-string $target
 */
public function __construct(string $target, string $operator = '==')
{
    if (!\in_array($operator, ['>', '<', '>=', '<=', '==', '!='])) {
        throw new \InvalidArgumentException(sprintf('Invalid operator "%s".', $operator));
    }

    $this->target = $target;
    $this->operator = $operator;
}

Additional Context

No response
@GlucNAc GlucNAc added the Bug label Dec 28, 2024
@xabbuh xabbuh added Finder DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Dec 28, 2024
@MatTheCat MatTheCat mentioned this issue Dec 29, 2024
Merged
fabpot added a commit that referenced this issue Dec 30, 2024
@fabpot
bug #59318 [Finder] Fix using == as default operator in `DateCompar…
1236323 
…ator` (MatTheCat)

This PR was merged into the 6.4 branch.

Discussion
----------

[Finder] Fix using `==` as default operator in `DateComparator`

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #59314
| License       | MIT

`$matches` can only contain `null` when passing the [`PREG_UNMATCHED_AS_NULL`](https://www.php.net/manual/en/pcre.constants.php#constant.preg-unmatched-as-null) flag. Since the culprit code line predates PHP 7.2 I made it check for an empty string rather than `null`, as `@GlucNAc` suggested.

Commits
-------

a247d58 [Finder] Fix using `==` as default operator in `DateComparator`
@fabpot fabpot closed this as completed Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DX DX = Developer eXperience (anything that improves the experience of using Symfony) Finder Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @xabbuh @carsonbot @GlucNAc
0