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.

Sign up for GitHub

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

Jump to bottom

Update dependency vimeo/psalm to v5 - abandoned #62

Open
wants to merge 11 commits into
base: 1.24.x
Choose a base branch
from
Open

Update dependency vimeo/psalm to v5 - abandoned #62

wants to merge 11 commits into from

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Nov 30, 2022 •
edited

Mend Renovate

This PR contains the following updates:

Package Type Update Change
vimeo/psalm require-dev major ^4.29.0 -> ^5.0.0

Release Notes

vimeo/psalm

v5.2.0

Compare Source

What's Changed

Features
Fixes
Internal changes
Typos

New Contributors

Full Changelog: vimeo/psalm@5.1.0...5.2.0

v5.1.0

Compare Source

What's Changed

Deprecations
Features
Fixes
Docs

New Contributors

Full Changelog: vimeo/psalm@5.0.0...5.1.0

v5.0.0: Psalm 5

Compare Source

Welcome to Psalm 5!

There's an accompanying post on psalm.dev, written by @​muglug & the current maintainers of Psalm.

What's Changed

Removed
Features
Fixes
Docs
Internal changes
Typos
Other changes

New Contributors

Full Changelog: vimeo/psalm@4.30.0...5.0.0

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻ Rebasing: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.

  • If you want to rebase/retry this PR, check this box

Read more information about the use of Renovate Bot within Laminas.

@bram123
Add new ElasticSearch
fa8c97e 
Check for cluster connection
Check for cluster health

Returns responsetime along with success/warning/failure.

Uses elasticsearch-php package
- Supports both v7 and v8 as both ES versions are still supported by Elastic

Signed-off-by: Bram Leeda <bram@123inkt.nl>
@bram123
Use Guzzle instead of elasticsearch-php to fetch the elasticsearch cl…
2d5d5c2 
…uster health status

Signed-off-by: Bram Leeda <bram@123inkt.nl>
@renovate renovate bot added the renovate label Nov 30, 2022
@renovate renovate bot force-pushed the renovate/vimeo-psalm-5.x branch 2 times, most recently from 8630ff2 to 4bc08e6 Compare December 9, 2022 00:46
@renovate renovate bot force-pushed the renovate/vimeo-psalm-5.x branch 2 times, most recently from 5f33748 to 8a47543 Compare December 11, 2022 01:30
bram123 and others added 2 commits December 13, 2022 13:49
@bram123
Signed-off-by: Bram Leeda <bram@123inkt.nl>
3debfac 
Add extra assertions for the ElasticSearch check result data
@Ocramius
Merge pull request #58 from bram123/add-elasticsearch-check
a397280 
Add new ElasticSearch check
@Ocramius Ocramius added this to the 1.21.0 milestone Dec 13, 2022
@Ocramius Ocramius self-assigned this Dec 13, 2022
@renovate
Copy link
Contributor Author

renovate bot commented Dec 13, 2022 •
edited

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

internalsystemerror
internalsystemerror reviewed Dec 13, 2022
View reviewed changes
Copy link
Member

@internalsystemerror internalsystemerror left a comment •
edited

Choose a reason for hiding this comment

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

Not targeting a new major, aren't these ctor signature changes a BC break? If existing subclasses extend the ctor, won't they also need to make this change?

@Ocramius
Copy link
Member

If existing subclasses extend the ctor, won't they also need to make this change?

__construct is special: it's not considered by LSP rules :)

https://3v4l.org/hGPu0

<?php

interface A {}
interface B {}


class NeedsA {
    function __construct(A $a) {}
}

class NeedsB extends NeedsA {
    function __construct(B $b) {}
}

var_dump(new NeedsB(new class implements B {}));

@internalsystemerror
Copy link
Member

__construct is special: it's not considered by LSP rules :)

🤯

renovate bot and others added 7 commits December 13, 2022 20:01
@renovate @Ocramius
Lock file maintenance
b866286 
Signed-off-by: Renovate Bot <bot@renovateapp.com>
@renovate @Ocramius
Update dependency vimeo/psalm to v5
d96362d 
| datasource | package     | from   | to    |
| ---------- | ----------- | ------ | ----- |
| packagist  | vimeo/psalm | 4.30.0 | 5.1.0 |


Signed-off-by: Renovate Bot <bot@renovateapp.com>
@Ocramius
Drop PHP 7 support, upgrade installed dependencies
a99791e
@Ocramius
Simplified and documented data structures of all checks and their con…
86e2436 
…structors

This mostly replaces many docblock-only union types with PHP 8 union types (now usable),
and removes many `Traversable` inputs where unusable.

Specifically, in order to operate properly, many of these checks do things like
`$this->settings[0]` or `count($this->settings)`, even in happy path scenarios,
leading to invalid execution.

This means that the accepted input type is not `array|Traversable` for many of our checks,
but rather `non-empty-list` or stricter.

While this could potentially mean that this is a BC break, these checks did **NOT**
work, when a `Traversable` was given as input, and the errors change from being an
`Exception` to being a more specific `TypeError`.

Tests verifying impossible types/scenarios have been removed.
@Ocramius
Regenerated psalm baseline off newly introduced type improvements
48ce68a
@Ocramius
Removed unnecessary @var annotation
5e57799
@Ocramius
Adjusted return type of ElasticSearchTest data provider
d53fc6c
@Ocramius
Copy link
Member

PHP 8.2 failing puzzles me 🤔

internalsystemerror
internalsystemerror requested changes Dec 13, 2022
View reviewed changes
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.

I assume a lot of those removed tests are no longer necessary thanks to psalm and type checks?

src/Check/AbstractFileCheck.php
protected $files;

/**
* @param string|array|Traversable $files Path name or an array / Traversable of paths
* @param iterable<string>|string $files Path name or an array / Traversable of paths
* @throws InvalidArgumentException
Copy link
Member

Choose a reason for hiding this comment

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

This could be removed now that it's a type check

src/Check/Callback.php
* @throws InvalidArgumentException
*/
public function __construct($callback, $params = [])
/** @throws InvalidArgumentException */
Copy link
Member

Choose a reason for hiding this comment

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

This could be removed now that it's a type check

src/Check/CpuPerformance.php
@@ -120,11 +120,11 @@ public static function calcPi($precision)
$limit = ceil(log($precision) / log(2)) - 1;
bcscale($precision + 6);
$a = 1;
$b = bcdiv(1, bcsqrt(2));
$b = bcdiv('1', bcsqrt(2));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$b = bcdiv('1', bcsqrt(2));
$b = bcdiv('1', bcsqrt('2'));

src/Check/CpuPerformance.php
$t = 1 / 4;
$p = 1;
for ($n = 0; $n < $limit; $n++) {
$x = bcdiv(bcadd($a, $b), 2);
$x = bcdiv(bcadd($a, $b), '2');
$y = bcsqrt(bcmul($a, $b));
$t = bcsub($t, bcmul($p, bcpow(bcsub($a, $x), 2)));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$t = bcsub($t, bcmul($p, bcpow(bcsub($a, $x), 2)));
$t = bcsub($t, bcmul($p, bcpow(bcsub($a, $x), '2')));

src/Check/DirReadable.php Show resolved Hide resolved
src/Check/PhpFlag.php Show resolved Hide resolved
src/Check/PhpFlag.php Show resolved Hide resolved
src/Check/PhpVersion.php
@@ -23,64 +18,47 @@
*
* This test accepts a single version and an operator or an array of
* versions to test for.
*
* @psalm-type Operator = '<'|'lt'|'<='|'le'|'>'|'gt'|'>='|'ge'|'=='|'='|'eq'|'!='|'<>'|'ne'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @psalm-type Operator = '<'|'lt'|'<='|'le'|'>'|'gt'|'>='|'ge'|'=='|'='|'eq'|'!='|'<>'|'ne'
* @psalm-type Operator = value-of<self::VALID_OPERATORS>

The result would be the same and this could be reused later on?

Copy link
Member

Choose a reason for hiding this comment

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

True that 👍

src/Check/StreamWrapperExists.php Show resolved Hide resolved
src/Runner/Runner.php
@@ -214,17 +215,13 @@ public function run($checkAlias = null)
/**
* Set config values from an array.
*
* @param array|Traversable $config
* @param iterable<string, mixed> $config
* @throws InvalidArgumentException
Copy link
Member

Choose a reason for hiding this comment

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

Could be removed, no longer thrown.

@Ocramius
Copy link
Member

Seems like PHP 8.2 considers a chmod($dir, 0) to still be an is_readable($dir) and is_writable($dir) 🤔

internalsystemerror
internalsystemerror reviewed Dec 13, 2022
View reviewed changes
src/Check/SecurityAdvisory.php
@@ -51,19 +49,14 @@ public function __construct($lockFilePath = null)
));
}

if (! $lockFilePath) {
if ($lockFilePath === null) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ($lockFilePath === null) {
if ($lockFilePath === null || $lockFilePath === '') {

Maybe also check for an empty string here

@Ocramius Ocramius removed this from the 1.21.0 milestone Dec 13, 2022
@Ocramius Ocramius changed the base branch from 1.20.x to 1.22.x December 13, 2022 20:45
@Ocramius Ocramius added this to the 1.22.0 milestone Dec 13, 2022
gsteel
gsteel approved these changes Dec 13, 2022
View reviewed changes
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Nothing to add to @internalsystemerror review, otherwise LGTM!

src/Check/PhpVersion.php
* @param string|array|Traversable $expectedVersion The expected version
* @param string $operator One of: <, lt, <=, le, >, gt, >=, ge, ==, =, eq, !=, <>, ne
* @param string|iterable<array-key, string> $expectedVersion The expected version
* @param value-of<self::VALID_OPERATORS> $operator
Copy link
Member

Choose a reason for hiding this comment

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

Just @param Operator $operator ?

@Ocramius Ocramius modified the milestones: 1.22.0, 1.23.0 Dec 15, 2022
@Ocramius Ocramius changed the base branch from 1.22.x to 1.23.x December 15, 2022 17:10
@Ocramius
Copy link
Member

Thought some more about this: needs to revert the Traversable support removal.

That's because ArrayObject is in use most of the time, so the use-case is probably array|(Traversable&Countable), which I hadn't thought about before.

Don't have the energy for it though :D

@Ocramius Ocramius mentioned this pull request Dec 20, 2022
Merged
@Ocramius Ocramius removed this from the 1.23.0 milestone Dec 20, 2022
@Ocramius Ocramius changed the base branch from 1.23.x to 1.24.x December 20, 2022 15:48
@IonBazan
Copy link
Contributor

How about skipping that Countable part completely and simply using [...$path] to cast whatever iterable is inside into an array so we can count it? Anyway we are going to exhaust the iterator at some point:

public function __construct(string|iterable $path)
{
    $this->path = is_string($path) ? [$path] : [...$path];
}

If [...$path] is considered too heavy to be in constructor, you can do that in check() instead.

@Ocramius
Copy link
Member

I don't think it's a heavy cast: we expect this to be configuration, not large data.

If that works for iterable types, I'll just have to add tests for it.

@renovate renovate bot changed the title Update dependency vimeo/psalm to v5 Update dependency vimeo/psalm to v5 - abandoned Jan 13, 2023
@renovate
Copy link
Contributor Author

renovate bot commented Jan 13, 2023

Autoclosing Skipped

This PR has been flagged for autoclosing. However, it is being skipped due to the branch being already modified. Please close/delete it manually or report a bug if you think this is in error.

@bram123 bram123 mentioned this pull request Feb 1, 2023
Closed
@Xerkus Xerkus mentioned this pull request Dec 12, 2023
Open
1 task
@gjuric
Copy link

gjuric commented Mar 26, 2024

Is anybody still working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius left review comments

@internalsystemerror internalsystemerror internalsystemerror requested changes

@gsteel gsteel gsteel approved these changes

Assignees

@Ocramius Ocramius

Labels
Awaiting Maintainer Response renovate Review Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Ocramius @internalsystemerror @IonBazan @gjuric @gsteel @bram123