decimal validation rule failed when set strict_types = 1

[noname007]

setting declare(strict_types=1) will result to a exception throwing at the preg_match pos

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[timacdonald]

The added tests pass without the string cast. What is the motivation behind the string cast here?

[noname007]

sorry ,yesterday i forgot to add the descreption

[timacdonald]

declare(strict_types=1) is local to the file that you specify it in, so it should not have any impact on Laravel's code.

The following test passes in a fresh Laravel application...

<?php

declare(strict_types=1);

namespace Tests\Feature;

use Illuminate\Support\Facades\Validator;
use Tests\TestCase;

class ExampleTest extends TestCase
{
    /**
     * A basic test example.
     */
    public function test_the_application_returns_a_successful_response(): void
    {
        $data = Validator::validate(['foo' => 1], ['foo' => 'Decimal:0,1']);

        $this->assertSame([
            'foo' => 1,
        ], $data);
    }
}

[noname007]

php info

PHP 8.3.10 (cli) (built: Jul 30 2024 13:44:37) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.3.10, Copyright (c) Zend Technologies
    with Xdebug v3.3.2, Copyright (c) 2002-2024, by Derick Rethans
    with Zend OPcache v8.3.10, Copyright (c), by Zend Technologies

some bad case scripts

<?php
declare(strict_types=1);

$matches = [];
$value = 1111;

if (preg_match('/^[+-]?\d*\.?(\d*)$/', $value, $matches) !== 1) {
    return false;
}

var_dump($value,$matches);

output

/opt/homebrew/Cellar/php/8.3.10/bin/php /Users/x/x/preg_match.2.php
PHP Fatal error:  Uncaught TypeError: preg_match(): Argument #2 ($subject) must be of type string, int given in /Users/x/x/preg_match.2.php:7
Stack trace:
#0 /Users/x/x/preg_match.2.php(7): preg_match('/^[+-]?\\d*\\.?(\\...', 1111, Array)
#1 {main}
  thrown in /Users/x/x/preg_match.2.php on line 7

Fatal error: Uncaught TypeError: preg_match(): Argument #2 ($subject) must be of type string, int given in /Users/x/xpreg_match.2.php:7
Stack trace:
#0 /Users/x/x/preg_match.2.php(7): preg_match('/^[+-]?\\d*\\.?(\\...', 1111, Array)
#1 {main}
  thrown in /Users/x/x/preg_match.2.php on line 7

[timacdonald]

@noname007, that is because declare(strict_types=1); is in the same file as the preg_match call.

declare(strict_types=1); only applies to the file which you add it to. So unless you are adding declare(strict_types=1); to Laravel's vendor files, this should not be happening.

[noname007]

to Laravel's vendor files, this should not be happening.

I don't think that's good. As a framework, the code should be able to run in more situations. It's even worse that the code can still run only because it is in vendor files and not because it has good enough quality.

[noname007]

I still have a question. Why don't we open the strict type check for the so popular framework?

[GrahamCampbell]

@timacdonald is 100% correct, and I think this PR should be closed.

[GrahamCampbell]

Why don't we open the strict type check for the so popular framework?

Because it doesn't add any value (and as you point out, is incorrect to do, as the framework core is relying on implicit casting). You can already call the framework with strict checking from your application code if you want the checks applied at the boundary, and/or use a static analyzer to help you find mistakes in your application code (and how it calls the framework code).

[noname007]

the framework core is relying on implicit casting

I got it. Thanks for your reply