Skip to content
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.

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

Added support for PHP 8.0 promoted parameters #114

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

michaelpetri
Copy link
Contributor

@michaelpetri michaelpetri commented Nov 20, 2021

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC yes
QA no

Description

Adds support for promoted parameters

Solves (partially) #104

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Discussed this review in a 1-on-1 session: PHP 8.1 is currently a bit out of reach.

<?php

interface Bar {}
class Baz implements Bar {}
class Foo
{
    public function __construct(
        public Bar $bar = new Baz()
    ) {}
    
    public function doThing(Bar $bar = new Baz()) {
        return $bar;
    }
}

var_dump(new Foo());

var_dump((new Foo)->doThing());

https://3v4l.org/SoOPU

In practice, we should stop this patch at only supporting initializers (and visibility thereof), while we will skip new_in_initializers for now ( https://wiki.php.net/rfc/new_in_initializers ) because reflection for it does not give us enough information about the default value yet.

We will likely need AST-based reflector for that, and we're far from that: at that point, this library will either need phasing out, or a full rewrite.

src/Generator/ObjectGenerator.php Outdated Show resolved Hide resolved
@Ocramius Ocramius added this to the 4.5.0 milestone Nov 22, 2021
@michaelpetri michaelpetri force-pushed the feature/add-in-initializers branch 3 times, most recently from 91c0a44 to 54c1b6b Compare November 24, 2021 21:09
@michaelpetri michaelpetri changed the title Added first idea for in initializers Added support for promoted parameters Nov 24, 2021
@michaelpetri michaelpetri force-pushed the feature/add-in-initializers branch 3 times, most recently from a33b3dc to ed09e59 Compare November 24, 2021 21:28
@michaelpetri michaelpetri marked this pull request as ready for review November 24, 2021 21:28
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM, excellent work, @michaelpetri!

@Ocramius Ocramius self-assigned this Nov 29, 2021
@Ocramius Ocramius changed the title Added support for promoted parameters Added support for PHP 8.0 promoted parameters Nov 29, 2021
@Ocramius Ocramius merged commit fbdaecd into laminas:4.5.x Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants