Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[ErrorHandler] improve parsing of phpdoc by DebugClassLoader#42149

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:5.4fromnicolas-grekas:eh-ret-types
Aug 18, 2021

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedJul 16, 2021
edited
Loading

QA
Branch?5.4
Bug fix?no
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

The goal of this PR is to improveDebugClassLoader to make it able:

  • to parse union types,
  • to trigger a deprecation when an@return defines a narrower type than the native return type,
  • to patch those return types in "force" mode
  • populate INTERNAL_TYPES from 8.1'sReturnTypeWillChange

rvanlaak reacted with hooray emoji
@nicolas-grekasnicolas-grekasforce-pushed theeh-ret-types branch 2 times, most recently from99a48fb toad03e81CompareJuly 20, 2021 13:46
nicolas-grekas added a commit that referenced this pull requestJul 21, 2021
…thods (nicolas-grekas)This PR was merged into the 6.0 branch.Discussion----------Add return type unions to private/internal/final/test methods| Q             | A| ------------- | ---| Branch?       | 6.0| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Spotted thanks to progress made on#42149Commits-------af7ff7e Add return type unions to private/internal/final/test methods
@nicolas-grekasnicolas-grekas changed the base branch from6.0 to5.4July 22, 2021 12:38
@nicolas-grekas
Copy link
MemberAuthor

PR updated. It now contains a script that parses the source code of PHP 8.1 to extract all new tentative return types. This is used to generate the new internalTentativeTypes class, which is in turn used to trigger deprecations when a method misses a return type or annotation.

As you can see, this makes tests fail because we're missing many such annotations.

In a next step, we should add the missing annotations and add also the missing#[\ReturnTypeWillChange] attributes. The code shouldn't be that far from being able to do it on its own.

@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedJul 23, 2021
edited
Loading

I'm going on a break for the following 2 weeks and will be mostly AFK.
The code in this PR is working, but needs more testing.

If you want to play locally with it, check it out, emptyexclude-from-classmap incomposer.json, then runcomposer i -o. The-o is important as that's how existing classes are discovered.

Then runSYMFONY_PATCH_TYPE_DECLARATIONS=force=1 php .github/patch-types.php to add return types, orSYMFONY_PATCH_TYPE_DECLARATIONS=force=docblock php .github/patch-types.php to add phpdoc.

What is missing here:

  • fix tests
  • makeDebugClassLoader able to add#[ReturnTypeWillChange] attributes where needed for PHP 8.1
  • look at current failures in the CI and figure out which annotations/return types are missing. E.g. anonymous classes aren't parsed right now but would need to.

The plan I have in mind for Symfony 6 relies heavily on this work.

I want to help the community by providing in 5.4 aDebugClassLoader that they can turn on in their CI to spot where they're missing return-types (or@return annotations before breaking BC.)

The plan for the OSS community would then be: turn this on, fix deprecations, plan a major version bump if needed, then allow Symfony 6 in their composer.json file.

I fear that allowing Symfony 6.0 right now in OSS libraries might create a huge WTF moment for the community: the day we'll add the return types in our 6.0 branch, we'll potentially break all those libs that allowed^6.

We might need a similar approach for public properties btw. Help wanted on all those topics.

nicolas-grekas added a commit that referenced this pull requestAug 10, 2021
…est methods (nicolas-grekas)This PR was merged into the 6.0 branch.Discussion----------Narrow existing return types on private/internal/final/test methods| Q             | A| ------------- | ---| Branch?       | 6.0| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -More progress from#42149Commits-------d67a927 Narrow existing return types on private/internal/final/test methods
@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedAug 10, 2021
edited
Loading

@nicolas-grekasnicolas-grekasforce-pushed theeh-ret-types branch 5 times, most recently from3126d88 toe8a5d5bCompareAugust 11, 2021 13:01
@nicolas-grekas
Copy link
MemberAuthor

PR is ready. It requires the last two PRs listed above to be green for tests. Psalm/fabbot reports are false positives.

nicolas-grekas added a commit that referenced this pull requestAug 11, 2021
This PR was merged into the 5.4 branch.Discussion----------More return type fixes| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Extracted from#42149Commits-------f7abdec More return type fixes
@nicolas-grekasnicolas-grekas modified the milestones:6.0,5.4Aug 17, 2021
@nicolas-grekasnicolas-grekasforce-pushed theeh-ret-types branch 3 times, most recently fromf49d877 toeb72183CompareAugust 17, 2021 16:37
@nicolas-grekas
Copy link
MemberAuthor

This PR is ready, remaining failures are false positives.

nicolas-grekas added a commit that referenced this pull requestAug 18, 2021
… (nicolas-grekas)This PR was merged into the 5.4 branch.Discussion----------Add missing return types to tests/internal/final methods| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Thanks to#42149 againCommits-------0f39a0f Add missing return types to tests/internal/final methods
@nicolas-grekasnicolas-grekasforce-pushed theeh-ret-types branch 3 times, most recently from9d8c5e8 to11a81d3CompareAugust 18, 2021 08:30
@nicolas-grekasnicolas-grekas merged commit7347f98 intosymfony:5.4Aug 18, 2021
@nicolas-grekasnicolas-grekas deleted the eh-ret-types branchAugust 18, 2021 09:00
nicolas-grekas added a commit that referenced this pull requestAug 25, 2021
…cations by default + add mode to turn them into native types (nicolas-grekas)This PR was merged into the 5.4 branch.Discussion----------[ErrorHandler] Turn return-type annotations into deprecations by default + add mode to turn them into native types| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Leverages#42149We need extensive doc on the topic for sure, a whole new chapter.DebugClassLoader allows patching an app or a lib by going through these steps:- require `symfony/error-handler` if not already there- run `composer install -q --optimize-autoloader`- copy/paste the below script to the root of the app/lib, in `patch-types.php`- run the script with `SYMFONY_PATCH_TYPE_DECLARATIONS='force=phpdoc' php patch-types.php``SYMFONY_PATCH_TYPE_DECLARATIONS` can be set to:- `'force=phpdoc'` to copy ``@return`` annotations from parent classes, to express that the next major version of that lib is going to add a native return types;- `'force=1'` to turn ``@return`` annotations into native return types, but only on tests/private/final/internal methods;- `'force=2'` to turn ``@return`` annotations into native return types, for all possible methods.```php<?phpif (false === getenv('SYMFONY_PATCH_TYPE_DECLARATIONS')) {    echo "Please define the SYMFONY_PATCH_TYPE_DECLARATIONS env var when running this script.\n";    exit(1);}$loader = require __DIR__.'/vendor/autoload.php';Symfony\Component\ErrorHandler\DebugClassLoader::enable();foreach ($loader->getClassMap() as $class => $file) {    switch (true) {        case false !== strpos($file = realpath($file), '/vendor/'):        case false !== strpos($file, '/src/path-to-exclude/'): // add as many as required            continue;    }    class_exists($class);}Symfony\Component\ErrorHandler\DebugClassLoader::checkClasses();```Commits-------bb11e62 [ErrorHandler] Turn return-type annotations into deprecations by default, add mode to turn them into native types
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@dunglasdunglasAwaiting requested review from dunglas

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@chalasrchalasrAwaiting requested review from chalasr

@srozesrozeAwaiting requested review from sroze

@wouterjwouterjAwaiting requested review from wouterj

@jderussejderusseAwaiting requested review from jderusse

@lyrixxlyrixxAwaiting requested review from lyrixx

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@xabbuhxabbuhAwaiting requested review from xabbuh

+1 more reviewer

@ro0NLro0NLro0NL left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

4 participants

@nicolas-grekas@stof@ro0NL@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp