Skip to content

[php-nextgen]: Fix ArrayAccess parameter types#23315

Open
coffeemakr wants to merge 1 commit intoOpenAPITools:masterfrom
coffeemakr:feature/better-array-access-parameter-types
Open

[php-nextgen]: Fix ArrayAccess parameter types#23315
coffeemakr wants to merge 1 commit intoOpenAPITools:masterfrom
coffeemakr:feature/better-array-access-parameter-types

Conversation

@coffeemakr
Copy link
Contributor

@coffeemakr coffeemakr commented Mar 23, 2026

The model classes implement the ArrayAccess interface. The phpdoc specifies the key type to be a string. In the methods, the key type is defined as int in the PHPDoc. This PR changes it to int|string. I'm not 100% sure what the correct type would be, but including string is correct for sure.

Also I think maybe the methods to access the fields could be moved to the ModelInterface, with stronger typing than the ArrayAccess interface.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR solves a reported issue, reference it using GitHub's linking syntax (e.g., having "fixes #123" present in the PR description)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Summary by cubic

Corrects ArrayAccess offset PHPDoc types in php-nextgen models to accept both strings and integers, matching PHP behavior and reducing IDE/static analysis warnings. Updates the generator template and regenerates samples.

  • Bug Fixes
    • Updated modules/openapi-generator/src/main/resources/php-nextgen/model_generic.mustache to use int|string for offset parameters (offsetExists, offsetGet, offsetUnset) and int|string|null for offsetSet.
    • Regenerated php-nextgen and php-nextgen-streaming sample models to reflect the new PHPDoc types.

Written for commit a5f6543. Summary will update on new commits.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 71 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="samples/client/petstore/php-nextgen/OpenAPIClient-php/src/Model/FormatTest.php">

<violation number="1" location="samples/client/petstore/php-nextgen/OpenAPIClient-php/src/Model/FormatTest.php:973">
P3: ArrayAccess key type docs are inconsistent: class-level `@implements ArrayAccess<string, mixed>` conflicts with updated method PHPDocs that accept `int|string` offsets. Update the class-level generic to match the new key type.</violation>
</file>

<file name="samples/client/petstore/php-nextgen/OpenAPIClient-php/src/Model/NullableClass.php">

<violation number="1" location="samples/client/petstore/php-nextgen/OpenAPIClient-php/src/Model/NullableClass.php:739">
P2: ArrayAccess key type docs are inconsistent: class-level `@implements ArrayAccess<string, mixed>` conflicts with updated method docs allowing `int|string` offsets.</violation>
</file>

<file name="samples/client/echo_api/php-nextgen/src/Model/DefaultValue.php">

<violation number="1" location="samples/client/echo_api/php-nextgen/src/Model/DefaultValue.php:582">
P2: Class-level ArrayAccess template still declares string-only keys, but updated method docs now allow int|string (and offsetSet allows null append), creating a conflicting type contract that can confuse static analysis and API usage.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

* Returns true if offset exists. False otherwise.
*
* @param integer $offset Offset
* @param int|string $offset Offset
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2: ArrayAccess key type docs are inconsistent: class-level @implements ArrayAccess<string, mixed> conflicts with updated method docs allowing int|string offsets.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/php-nextgen/OpenAPIClient-php/src/Model/NullableClass.php, line 739:

<comment>ArrayAccess key type docs are inconsistent: class-level `@implements ArrayAccess<string, mixed>` conflicts with updated method docs allowing `int|string` offsets.</comment>

<file context>
@@ -736,7 +736,7 @@ public function setObjectItemsNullable(?array $object_items_nullable): static
      * Returns true if offset exists. False otherwise.
      *
-     * @param integer $offset Offset
+     * @param int|string $offset Offset
      *
      * @return boolean
</file context>
Fix with Cubic

* Returns true if offset exists. False otherwise.
*
* @param integer $offset Offset
* @param int|string $offset Offset
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2: Class-level ArrayAccess template still declares string-only keys, but updated method docs now allow int|string (and offsetSet allows null append), creating a conflicting type contract that can confuse static analysis and API usage.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/echo_api/php-nextgen/src/Model/DefaultValue.php, line 582:

<comment>Class-level ArrayAccess template still declares string-only keys, but updated method docs now allow int|string (and offsetSet allows null append), creating a conflicting type contract that can confuse static analysis and API usage.</comment>

<file context>
@@ -579,7 +579,7 @@ public function setStringNullable(?string $string_nullable): static
      * Returns true if offset exists. False otherwise.
      *
-     * @param integer $offset Offset
+     * @param int|string $offset Offset
      *
      * @return boolean
</file context>
Fix with Cubic

@@ -970,7 +970,7 @@ public function setArrayRef(?array $array_ref): static
/**
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 23, 2026

Choose a reason for hiding this comment

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

P3: ArrayAccess key type docs are inconsistent: class-level @implements ArrayAccess<string, mixed> conflicts with updated method PHPDocs that accept int|string offsets. Update the class-level generic to match the new key type.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/php-nextgen/OpenAPIClient-php/src/Model/FormatTest.php, line 973:

<comment>ArrayAccess key type docs are inconsistent: class-level `@implements ArrayAccess<string, mixed>` conflicts with updated method PHPDocs that accept `int|string` offsets. Update the class-level generic to match the new key type.</comment>

<file context>
@@ -970,7 +970,7 @@ public function setArrayRef(?array $array_ref): static
      * Returns true if offset exists. False otherwise.
      *
-     * @param integer $offset Offset
+     * @param int|string $offset Offset
      *
      * @return boolean
</file context>
Fix with Cubic

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.

1 participant