FriendsOfCake / cakephp-upload

CakePHP: Handle file uploading sans ridiculous automagic
https://cakephp-upload.readthedocs.io/
MIT License
551 stars 255 forks source link

remove optional parameter value #582

Closed saeideng closed 2 years ago

saeideng commented 2 years ago

I changed also the interface not sure if it is BC Closes #581

codecov[bot] commented 2 years ago

Codecov Report

Merging #582 (99e2f5a) into master (cf3bff0) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #582   +/-   ##
=========================================
  Coverage     95.00%   95.00%           
  Complexity      110      110           
=========================================
  Files            11       11           
  Lines           280      280           
=========================================
  Hits            266      266           
  Misses           14       14           
Impacted Files Coverage Δ
src/File/Writer/DefaultWriter.php 100.00% <ø> (ø)
src/Model/Behavior/UploadBehavior.php 94.73% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cf3bff0...99e2f5a. Read the comment docs.

davidyell commented 2 years ago

Seems sensible, although with it typed as ?UploadedFileInterface will the default type be null ?

saeideng commented 2 years ago

Seems sensible, although with it typed as ?UploadedFileInterface will the default type be null ?

no default value , because you have to pass all parameters (except when using named Argument)

saeideng commented 2 years ago

test with named Argument:

class A
{
    public string $first;
    public ?string $nullableString;
    public int $third;
    public function __construct(
        string $first,
        ?string $nullableString = null,
        int $third,
    ) {

        $this->first = $first;
        $this->nullableString = $nullableString;
        $this->third = $third;
    }
    public function dump(){
        var_dump($this->first);
        var_dump($this->nullableString);
        var_dump($this->third);
    }
}
$data = new A(
    'a string',
    third: 20,

);
$data->dump();

current implementation only works in php 8.0 when you use named Argument

//result php 8.0
string(8) "a string"
NULL
int(20)
//result php 8.1
Deprecated: Optional parameter $nullableString declared before required parameter $third is implicitly treated as a required parameter
Fatal error: Uncaught ArgumentCountError: A::__construct(): Argument #2 ($nullableString) not passed
saeideng commented 2 years ago

extending class test:

class A
{
    public  $p;
    public function func(
        int $p = null
    ){
        $this->p = $p;
    }
}
class B extends A
{
    public  $p;
    public function func(
        ?int $p = null
    ) {
        $this->p = $p;
    }
    public function dump(){
        var_dump($this->p);
    }
}
$data = new B();
$data->func(50);
$data->dump();

php 7.1+

int(50)

another test: implement interface ( testing __construct())

interface AInterface
{
    public function __construct(
        string $p = null
    );
}
class A implements AInterface
{
    public $p;
    public function __construct(
        ?string $p = null
    ) {
        $this->p = $p;
    }
    public function dump(){
        var_dump($this->p);
    }
}
$data = new A(
    'a string'
);
$data->dump();

php 7.1+

string(8) "a string"