Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add tests for laravel/serializable-closure#78 #79

Closed
wants to merge 2 commits into from
Closed

Add tests for laravel/serializable-closure#78 #79

wants to merge 2 commits into from

Conversation

JakeBooher
Copy link

@JakeBooher JakeBooher commented Nov 3, 2023

Serializable Closure Version

1.3.2

PHP Version

8.2

Description

It seems that when there is a switch statement that contains a class reference the namespace resolution that happens during serialization causes the colon to be removed at the end of the case line, steps to reproduce contains a CLI command to replicate this behavior.

syntax error, unexpected token "return"

Steps To Reproduce

<?php

namespace App\Console\Commands;

use Illuminate\Console\Command;
use Laravel\SerializableClosure\Serializers\Native;

class DebugCommand extends Command
{
    /**
     * The name and signature of the console command.
     *
     * @var string
     */
    protected $signature = 'debug';

    /**
     * The console command description.
     *
     * @var string
     */
    protected $description = 'Command description';

    /**
     * Execute the console command.
     */
    public function handle()
    {
        $cacheClosure = static function(): array {
            $var = new \stdClass();

            switch(true) {
                case $var instanceof \stdClass:
                    return true;

                default:
                    return false;
            }
        };

        $cacheData = [
           'helloWorld' =>  $cacheClosure
        ];

        Native::wrapClosures($cacheData, new \SplObjectStorage());
        $cacheContent = serialize($cacheData);

        dump($cacheContent);exit;

        unserialize($cacheContent);exit;
    }
}

@taylorotwell
Copy link
Member

Mark this as ready when it has a full description.

@taylorotwell taylorotwell marked this pull request as draft November 3, 2023 15:23
@JakeBooher
Copy link
Author

This PR was created to add the tests requested in Issue #78 so I've copied the description from that. It's not something intended to be merged as currently these tests do fail, it is intended as a reproduction case for that issue

@JakeBooher JakeBooher marked this pull request as ready for review November 6, 2023 15:36
@taylorotwell
Copy link
Member

All the tests in this PR pass?

@JakeBooher
Copy link
Author

Looks like I missed adding the test to the phpunit.xml.dist file, so it was only being run locally because I explicitly targeted the tests directory. Should be able to replicate now @taylorotwell

@driesvints driesvints reopened this Nov 7, 2023
@taylorotwell taylorotwell marked this pull request as draft November 7, 2023 13:10
@nunomaduro
Copy link
Member

Thank you for this. Started to work on it: #80.

@nunomaduro nunomaduro closed this Nov 7, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants