Skip to content

PHPLIB-1113: Update to psalm 5 #1061

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

Closed
wants to merge 1 commit into from
Closed

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Apr 13, 2023

PHPLIB-1113

This is necessary to better support array/object shapes for the codec pull request, and is necessary to get updated stub maps when we add stubs for new BSON classes to the map for ext-mongodb.

I have fixed the easy-to-fix errors, and a number of template errors (mostly relating to iterators) have been added to the baseline. The additional config values for findUnusedBaselineEntry and findUnusedCode are set to the current default value (false) in anticipation of those defaults changing to true in psalm 6. All of these issues can be revisited separately at a later date.

@alcaeus alcaeus requested a review from jmikola April 13, 2023 07:08
@alcaeus alcaeus self-assigned this Apr 13, 2023
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Note the two "Unchanged files with check annotations" errors in the diff.

  • src/functions.php:84:12: MixedInferredReturnType: Could not verify return type 'array<array-key, mixed>|object' for MongoDB\apply_type_map_to_document (see https://psalm.dev/047)
  • src/functions.php:93:12: MixedReturnStatement: Could not infer a return type (see https://psalm.dev/138)

/**
* Return the index name.
*/
/** Return the index name. */
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that this wasn't caught by the phpcbf: fix RequireOneLineDocComment commit in #1058. I suppose that only applied when the doc block contained annotations.

@jmikola
Copy link
Member

jmikola commented Apr 17, 2023

@alcaeus: The following should explain the various CI failures. IMO only the last paragraph needs to be addressed in this PR (sorry for missing that with my previous LGTM).

The one GitHub Action failure is related to libmongoc using QEv2 and running against a pre-7.0 server that still requires QEv1. Two related operations were aborted prematurely after the first task failed, so I can't discern what their exactl failures are but we can assume it's related.

I do see a few Evergreen test failures related to what I'm tracking in PHPLIB-1115 (snapshot queries doc example).

Most of the Evergreen task failures pertain to CSFLE spec/prose tests. Those should be addressed by #1063 for PHPLIB-1088, which will skip the tests until they are implemented in PHPLIB-878.

At least two Evergreen task failures (example) are failing due to a Composer requirement conflict. I think this is for the PHP 7.2 and 7.3 environments:

Your requirements could not be resolved to an installable set of packages.
Problem 1
     - vimeo/psalm[5.0.0, ..., 5.9.0] require php ^7.4 || ~8.0.0 || ~8.1.0 || ~8.2.0 -> your php version (7.2.34) does not satisfy that requirement.
     - Root composer.json requires vimeo/psalm ^5 -> satisfiable by vimeo/psalm[5.0.0, ..., 5.9.0].

@alcaeus
Copy link
Member Author

alcaeus commented Apr 18, 2023

Closing this PR to avoid shenanigans with composer.json to retain support for PHP 7.2 and 7.3 while also supporting psalm 5 which requires PHP 7.4. We'll revisit this after dropping support for PHP 7.2 and 7.3.

@alcaeus alcaeus closed this Apr 18, 2023
@alcaeus alcaeus deleted the psalm-5 branch April 18, 2023 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants