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

Upgrade PHPStan to 1.0 #10253

Merged
merged 5 commits into from Nov 14, 2021
Merged

Upgrade PHPStan to 1.0 #10253

merged 5 commits into from Nov 14, 2021

Conversation

Seldaek
Copy link
Member

@Seldaek Seldaek commented Nov 3, 2021

Still WIP for now, need to fix a few errors which are newly reported in 1.0 (if anyone wants to help feel free to send a PR against my PR branch, I won't have time to wrap this up in the next few days/weeks I think).

I am not sure what's up with all the ClassLoader failures though. I am thinking it may be a loading issue whereas the ClassLoader that is shipped with PHPStan is used and not the one from the Composer repo. Seemed to work fine in 0.12 though. @ondrejmirtes any clue about this? If not I can also just ignore these in baseline, it's not a huge deal as this class is pretty much written in stone.

@Seldaek Seldaek added this to the 2.1 milestone Nov 3, 2021
composer.json Outdated Show resolved Hide resolved
@ondrejmirtes
Copy link
Contributor

The ClassLoader thing is definitely a weird bug, I'm gonna look into it.

@ondrejmirtes
Copy link
Contributor

It should work with PHPStan 1.0.2, please upgrade and test it :) https://github.com/phpstan/phpstan/releases/tag/1.0.2

@Seldaek
Copy link
Member Author

Seldaek commented Nov 3, 2021

Yup that worked, thanks!

@@ -286,10 +286,10 @@ private function initDownload($resolve, $reject, $origin, $url, $options, $copyT
*/
public function abortRequest($id)
{
if (isset($this->jobs[$id], $this->jobs[$id]['handle'])) {
if (isset($this->jobs[$id], $this->jobs[$id]['curlHandle'])) {
Copy link
Member Author

Choose a reason for hiding this comment

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

That was actually a bug causing aborts to fail 👍🏻

@Seldaek
Copy link
Member Author

Seldaek commented Nov 13, 2021

@ondrejmirtes sorry to bother again but would you please look at the last failing one? I can't make any sense of it, and can't repro it in a minimalistic test case sadly https://phpstan.org/r/ef8e8b43-5e90-41c8-8453-6ab3c7b011d9

See https://github.com/composer/composer/runs/4199944625?check_suite_focus=true

@herndlm
Copy link
Contributor

herndlm commented Nov 13, 2021

@Seldaek I remember that one. Could it be that isset is not needed because it is used in the canceller, which is always called after startJob, where curl_id is always set? Which would narrow the type and then it's not optional any more.

Must be something like that.

@Seldaek
Copy link
Member Author

Seldaek commented Nov 13, 2021

I don't think it's that smart (great if it is tho;)) but could be something about the closures using $job by reference which somehow alerts the type? Not sure.

@Seldaek
Copy link
Member Author

Seldaek commented Nov 14, 2021

OK found a way to repro it, reported phpstan/phpstan#6008 and will add to baseline for now.

@Seldaek Seldaek marked this pull request as ready for review November 14, 2021 15:24
@Seldaek Seldaek merged commit f509c41 into composer:main Nov 14, 2021
@Seldaek Seldaek deleted the phpstan1 branch November 14, 2021 19:42
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.

None yet

4 participants