Improve type of Collection Bulk Write operations #1694

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

Jump to bottom

Open

GromNaN wants to merge 1 commit into mongodb:v2.x

from GromNaN:bulk-types

Open

Improve type of Collection Bulk Write operations #1694

GromNaN wants to merge 1 commit into mongodb:v2.x from GromNaN:bulk-types

Conversation

@GromNaN

Copy link

Member

@GromNaN GromNaN commented May 20, 2025 •

edited

Loading

The Collection::bulkWrite($operations) method requires a complex array structure for the operations list. Describing this type using psalm custom type feature to helps static analysis.

PHPStorm is able to make the completion only with the full @param annotation, not @phpstan-param nor @psaml-param. And only when I start typing the first letter.
image
image

Both Psalm and PHPStan detect when the argument is invalid. The PHPStorm inspection does not detect issues when the array does not match the expected @param type.

image

@GromNaN GromNaN requested a review from a team as a code owner

May 20, 2025 11:47

@GromNaN GromNaN requested a review from alcaeus

May 20, 2025 11:47

github-advanced-security[bot]

github-advanced-security bot found potential problems

May 20, 2025

View reviewed changes

src/Operation/BulkWrite.php Fixed Show fixed Hide fixed

github-advanced-security[bot]

github-advanced-security bot found potential problems

May 22, 2025

View reviewed changes

src/Operation/BulkWrite.php

// instance, there is no need to re-validate the returned value here.

if ($codec) {

$operations[$i][$type][0] = $codec->encode($args[0]);

$operation[$type][0] = $codec->encode($args[0]);

Check notice

Code scanning / Psalm

PossiblyInvalidArgument Note

Argument 1 of MongoDB\Codec\DocumentCodec::encode expects object, but possibly different type array<array-key, mixed>|null|object provided

src/Operation/BulkWrite.php

case self::DELETE_MANY:

case self::DELETE_ONE:

$operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]);

$operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

src/Operation/BulkWrite.php

case self::REPLACE_ONE:

$operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]);

$operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

src/Operation/BulkWrite.php

if ($codec) {

$operations[$i][$type][1] = $codec->encode($args[1]);

$operation[$type][1] = $codec->encode($args[1]);

Check notice

Code scanning / Psalm

PossiblyInvalidArgument Note

Argument 1 of MongoDB\Codec\DocumentCodec::encode expects object, but possibly different type array<array-key, mixed>|null|object provided

src/Operation/BulkWrite.php

case self::UPDATE_MANY:

case self::UPDATE_ONE:

$operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]);

$operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

src/Operation/BulkWrite.php

}

$operations[$i][$type][1] = $args[1] = $builderEncoder->encodeIfSupported($args[1]);

$operation[$type][1] = $args[1] = $builderEncoder->encodeIfSupported($args[1]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

src/Operation/BulkWrite.php

}

$operations[$i][$type][1] = $args[1] = $builderEncoder->encodeIfSupported($args[1]);

$operation[$type][1] = $args[1] = $builderEncoder->encodeIfSupported($args[1]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

@GromNaN GromNaN force-pushed the bulk-types branch from 8c96884 to f4a3b44 Compare

May 22, 2025 14:10

jmikola

jmikola reviewed

Aug 12, 2025

View reviewed changes

src/Operation/BulkWrite.php

* @see \MongoDB\Collection::bulkWrite()

* @psalm-type Document = object|array

* @psalm-type OperationType = array{deleteMany: array{0: Document, 1?: array}}|array{deleteOne: array{0: Document, 1?: array}}|array{insertOne: array{0: Document}}|array{replaceOne: array{0: Document, 1: Document, 2?: array}}|array{updateMany: array{0: Document, 1: Document, 2?: array}}|array{updateOne: array{0: Document, 1: Document, 2?: array}}

Copy link

Member

@jmikola jmikola Aug 12, 2025

Choose a reason for hiding this comment

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

Per our conversation yesterday, I think we still need confirmation on whether this can be utilized by the IDE. If not, is this worth adding at all?

Copy link

Member Author

@GromNaN GromNaN Aug 14, 2025

Choose a reason for hiding this comment

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

Looking at the baseline, it definitely improves the psalm analysis.

Copy link

Member Author

@GromNaN GromNaN Sep 23, 2025

Choose a reason for hiding this comment

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

PHPStorm has supports autocompletion of array keys thanks to the generic @param annotation on Collection::bulkWrite. The psalm annotation is here to improve static analysis.

Copilot AI review requested due to automatic review settings

August 27, 2025 09:31

@GromNaN GromNaN force-pushed the bulk-types branch from f4a3b44 to 95eda6c Compare

August 27, 2025 09:31

Copilot AI reviewed

Aug 27, 2025

View reviewed changes

Copy link

Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves type safety for MongoDB Collection bulk write operations by introducing a custom Psalm type definition for the complex array structure required by the bulkWrite() method. The change adds static analysis support to help catch type-related issues at development time.

Introduces a detailed OperationType custom type that describes all supported bulk write operation formats
Updates method signatures to use the new type annotations for better static analysis
Refactors the validation loop to use references instead of array indexing for improved performance
Adds a test case for empty operation validation

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/Operation/BulkWrite.php	Defines the OperationType custom type and updates method signatures and validation logic
src/Collection.php	Imports and uses the OperationType in the bulkWrite method signature
tests/Operation/BulkWriteTest.php	Adds test coverage for empty operation validation
psalm-baseline.xml	Updates baseline to reflect improved type analysis with fewer mixed type issues

Comments suppressed due to low confidence (1)

src/Operation/BulkWrite.php:1

The Document type is defined twice (lines 49 and 51). Remove the duplicate definition to avoid confusion and maintain clean code.

<?php

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/Collection.php Outdated Show resolved Hide resolved

@alcaeus alcaeus removed their request for review

September 19, 2025 12:22

@alcaeus

Copy link

Member

alcaeus commented Sep 19, 2025

FWIW, I think this is a good intermediate step until we provide a better solution (e.g. by allowing to pass a MongoDB\BulkWrite instance to Collection::bulkWrite.

@jmikola jmikola self-requested a review

September 23, 2025 14:13

@GromNaN


 Improve type of Collection Bulk Write operations

b37c1c4

@GromNaN GromNaN force-pushed the bulk-types branch from fb97ddd to b37c1c4 Compare

September 23, 2025 15:00

Labels

None yet

3 participants

@GromNaN @alcaeus @jmikola

Uh oh!

Improve type of Collection Bulk Write operations #1694

Are you sure you want to change the base?

Improve type of Collection Bulk Write operations #1694

Uh oh!

Conversation

@GromNaN GromNaN commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

@jmikola jmikola Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

@GromNaN GromNaN Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

@GromNaN GromNaN Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

alcaeus commented Sep 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

@GromNaN GromNaN commented May 20, 2025 •

edited

Loading