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

Drop PHP 7.4 and PHP 8.0 support #154

Merged
merged 53 commits into from
Nov 19, 2022

Conversation

Ocramius
Copy link
Member

@Ocramius Ocramius commented Nov 19, 2022

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

This patch overhauls the various types in use in this package, and requires PHP 8.1 to do so, and to achieve parity with the current ENUM integrations.

TODOs:

  • make the build green (yes, I did break something)
  • adjust CI so that it no longer runs PHP 8.0

Improvement:

-Psalm was able to infer types for 95.8537% of the codebase
+Psalm was able to infer types for 96.5310% of the codebase

Because we operate with reflection symbols for attributes,
we want to be on at least PHP 8.0, in order to
have Psalm and PHPUnit access all the relevant
types related to attributes.

Signed-off-by: Marco Pivetta <[email protected]>
This API is old, bad, and not really useful, as it produces a string representation
of functions/methods that doesn't bring much to the table: in fact,
it is easier to inspect the AST of a symbol, than to look at its
string or array prototype.

These methods will be dropped in the next major release, as they
are prone to crashes due to union/intersection types on PHP 8.1
and newer, and it is not worth investing the time to fix them.

Signed-off-by: Marco Pivetta <[email protected]>
This is because we really do a lot of stuff with ENUMs, and it is a massive
pain to run all checks on an older inferred PHP version.

By doing this, we can refine the types of various ENUM structures
in our core logic in subsequent commits.

Signed-off-by: Marco Pivetta <[email protected]>
…` around constants

This is because we were still using the array-style logic for creating constants
inside `ClassGenerator::fromReflection()`: switching to the programmatic API makes
things much more readable

Signed-off-by: Marco Pivetta <[email protected]>
Note: because psalm has imprecise stubs around ENUM reflection symbols,
some issues cannot currently be prevented.

Ref: vimeo/psalm#8720
Signed-off-by: Marco Pivetta <[email protected]>
… static analysis issues

Signed-off-by: Marco Pivetta <[email protected]>
…d by `laminas/laminas-server`

This method attempted to discover the return type of a function, but in a world where return types
didn't exist yet (before PHP 7.0).

This method has outlived its usefulness, and is now targeted for removal.

Signed-off-by: Marco Pivetta <[email protected]>
Signed-off-by: Marco Pivetta <[email protected]>
This API is not suitable for maintenance: the `ValueGenerator` attempts
to load defined constants into its scope, and then uses them to generate
constant expressions (rather than values).

This is risky business, and should be avoided: it will be removed in the
next major release.

Signed-off-by: Marco Pivetta <[email protected]>
composer.json Show resolved Hide resolved
Signed-off-by: Marco Pivetta <[email protected]>
@Ocramius
Copy link
Member Author

@gsteel @internalsystemerror I tried keeping my changes limited to internals: wherever the API would even change in behavior, I'd instead adjust the docblocks.

This is mostly preparation work for the shower of 4.8.0 PRs that I need to deal with over the weekend @_@

Copy link
Member

@internalsystemerror internalsystemerror left a comment

Choose a reason for hiding this comment

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

Some requests but mostly suggestions. This must have taken you some time (given how long it took to review) so a big thank you to you!

src/Generator/ClassGenerator.php Show resolved Hide resolved
src/Generator/ClassGenerator.php Outdated Show resolved Hide resolved
src/Generator/ClassGenerator.php Outdated Show resolved Hide resolved
src/Generator/EnumGenerator/EnumGenerator.php Outdated Show resolved Hide resolved
src/Generator/ValueGenerator.php Show resolved Hide resolved
src/Reflection/MethodReflection.php Outdated Show resolved Hide resolved
src/Reflection/ParameterReflection.php Show resolved Hide resolved
src/Reflection/ParameterReflection.php Outdated Show resolved Hide resolved
src/Reflection/ParameterReflection.php Show resolved Hide resolved
Copy link
Member

@internalsystemerror internalsystemerror left a comment

Choose a reason for hiding this comment

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

LGTM

@internalsystemerror internalsystemerror merged commit 83d449f into 4.8.x Nov 19, 2022
@Ocramius Ocramius deleted the feature/php-7.4-and-8.0-removal branch November 19, 2022 15:51
nicolas-grekas added a commit to nicolas-grekas/laminas-code that referenced this pull request Nov 25, 2022
This was removed in laminas#154 but this phpdoc is useful to some static analyzers, so that they can know what the future return-type is going to be.

Signed-off-by: Nicolas Grekas <[email protected]>
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.

3 participants