Skip to content

Fix PHP numeric deserializer keys - #8012

Open
AayushP123 wants to merge 1 commit into
microsoft:mainfrom
AayushP123:agent/fix-7830-php-numeric-deserializer-keys
Open

Fix PHP numeric deserializer keys#8012
AayushP123 wants to merge 1 commit into
microsoft:mainfrom
AayushP123:agent/fix-7830-php-numeric-deserializer-keys

Conversation

@AayushP123

Copy link
Copy Markdown
Contributor

Summary

Fixes #7830.

PHP converts canonical numeric string array keys such as '0' and '500' to integers. As a result, PHPStan infers an integer-keyed array for generated deserializers even though Parsable::getFieldDeserializers() returns array<string, callable>.

Changes

  • Detect generated models whose serialized property names are canonical integer keys.
  • Materialize those deserializers in a locally annotated variable before returning it, preserving the public Parsable contract.
  • Leave the existing direct-return output unchanged for models without numeric property names.
  • Add writer coverage for a numeric serialized property name.

Testing

  • dotnet test tests/Kiota.Builder.Tests/Kiota.Builder.Tests.csproj --filter FullyQualifiedName~Writers.Php.CodeMethodWriterTests --no-restore
  • dotnet test tests/Kiota.Builder.Tests/Kiota.Builder.Tests.csproj --no-restore
  • dotnet format kiota.slnx --include src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs --no-restore --verify-no-changes
  • Generated a PHP client from the current Meraki OpenAPI document and verified the affected numeric-key deserializers use the annotated return variable.

Copilot AI review requested due to automatic review settings August 1, 2026 08:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the PHP writer to avoid PHPStan return-type inference failures for Parsable::getFieldDeserializers() when OpenAPI serialized property names are canonical numeric strings (e.g., "0", "500"), which PHP treats as integer array keys.

Changes:

  • Detect canonical numeric serialized property names in generated PHP deserializers and emit an annotated local $deserializers variable before returning it.
  • Preserve existing direct return [...] output for models without numeric serialized property names.
  • Add PHP writer test coverage for a numeric serialized property name case, and document the change in the changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs Adds canonical-numeric key detection and conditionally returns an annotated local deserializer array to satisfy static analysis.
tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs Adds a regression test asserting the annotated $deserializers path is used for a "500" serialized name.
CHANGELOG.md Notes the PHP static-analysis fix for numeric deserializer keys.

@AayushP123
AayushP123 force-pushed the agent/fix-7830-php-numeric-deserializer-keys branch from db6d851 to 1ab0b04 Compare August 1, 2026 09:10
@AayushP123
AayushP123 marked this pull request as ready for review August 2, 2026 16:33
@AayushP123
AayushP123 requested a review from a team as a code owner August 2, 2026 16:33
Copilot AI review requested due to automatic review settings August 2, 2026 16:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment on lines 692 to +693
writer.WriteLines(
$"return {((extendsModelClass) ? $"array_merge(parent::{method.Name.ToFirstCharacterLowerCase()}(), [" : " [")}");
$"{(hasNumericPropertyName ? "$deserializers =" : "return")} {((extendsModelClass) ? $"array_merge(parent::{method.Name.ToFirstCharacterLowerCase()}(), [" : " [")}");
@WolfgangHG

Copy link
Copy Markdown
Contributor

@AayushP123 Could you also remove the exclusion for this error in "config.json"?

kiota/it/config.json

Lines 165 to 175 in b9e6126

"ExcludePatterns": [
{
"Language": "php",
"Pattern": "/organizations/{organizationId}/apiRequests/overview",
"Rationale": "Fails for php with api dated 2026-06-24 - https://github.com/microsoft/kiota/issues/7830"
},
{
"Language": "php",
"Pattern": "/networks/{networkId}/clients/{clientId}/splashAuthorizationStatus",
"Rationale": "Fails for php with api dated 2026-06-24 (GET and PUT) - https://github.com/microsoft/kiota/issues/7830"
}

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.

[PHP] "getFieldDeserializers() should return array..." with MERAKI.COM api

3 participants