Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"issues": "https://issues.apache.org/jira/browse/AVRO"
},
"require": {
"php": "^8.1"
"php": "^8.1",
"nikic/php-parser": "^5.7"
},
"deps": [
"vendor/phpunit/phpunit"
Expand Down
374 changes: 374 additions & 0 deletions lang/php/lib/Generator/AvroCodeGenerator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,374 @@
<?php

/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

declare(strict_types=1);

namespace Apache\Avro\Generator;

use Apache\Avro\Schema\AvroArraySchema;
use Apache\Avro\Schema\AvroEnumSchema;
use Apache\Avro\Schema\AvroMapSchema;
use Apache\Avro\Schema\AvroPrimitiveSchema;
use Apache\Avro\Schema\AvroRecordSchema;
use Apache\Avro\Schema\AvroSchema;
use Apache\Avro\Schema\AvroUnionSchema;
use PhpParser\BuilderFactory;
use PhpParser\Node;
use PhpParser\Node\Scalar\String_;
use PhpParser\Node\Stmt;
use PhpParser\PrettyPrinter\Standard;

class AvroCodeGenerator
{
private BuilderFactory $factory;
private Standard $printer;

/** @var array<string, AvroSchema> */
private array $registry = [];

public function __construct()
{
$this->factory = new BuilderFactory();
$this->printer = new Standard(['shortArraySyntax' => true]);
}

/**
* @return array<string, string> Map of filename to file contents
*/
public function translate(
AvroSchema $schema,
string $path,
string $phpNamespace
): array {
$this->buildRegistry($schema);

$files = [];

foreach ($this->registry as $name => $registeredSchema) {
$node = match (true) {
$registeredSchema instanceof AvroEnumSchema => $this->buildEnum(
$registeredSchema,
$phpNamespace,
$registeredSchema->symbols()
),
$registeredSchema instanceof AvroRecordSchema => $this->buildRecord(
$registeredSchema,
$phpNamespace
),
default => null
};

if (null !== $node) {
$code = <<<PHP
<?php

declare(strict_types=1);

{$this->printer->prettyPrint([$node])}

PHP;
Comment on lines +79 to +86
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using indented flexible heredocs with empty lines can be fragile. In PHP, every line within the heredoc must have at least the same indentation as the closing identifier. If the 'empty' lines (81, 83, 85) do not contain the exact number of spaces as the closing PHP; tag, a ParseError will occur. It is safer to use a non-indented heredoc or ensure the empty lines are properly padded, though the former is much more maintainable.


$filename = $path.'/'.ucwords($name).'.php';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The filename is being generated using the Avro fullname ($name), but the class name inside the file is generated using the short name ($registeredSchema->name()). This mismatch will break PSR-4 autoloading. Additionally, ucwords on a fullname containing dots (e.g., com.example.User) will not produce a valid filename or directory structure as intended. You should use the short name for the filename to match the class name, especially since all classes are being placed in the same PHP namespace.

                $filename = $path.'/'.ucwords($registeredSchema->name()).'.php';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lang/php/lib/Generator/AvroCodeGenerator.php:88: translate() uses the registry key ($name = schema fullname) to form the filename, while the generated class/enum name is based on the unqualified schema name(). For schemas with an Avro namespace, this can produce filenames containing dots / not matching the declared type name, which can break common autoloading expectations.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Filename uses fullname instead of simple name

High Severity

The filename at line 88 is built from $name, which is the registry key — i.e., $schema->fullname(). For Avro schemas with a namespace (e.g., "namespace": "com.example"), fullname() returns "com.example.User", producing a filename like Com.example.User.php. Meanwhile, buildRecord and buildEnum derive the class name from $schema->name() (the simple name, "User"). This mismatch means the generated file won't match its class name, breaking autoloading and PSR-4 conventions for any namespaced Avro schema.

Additional Locations (1)
Fix in Cursor Fix in Web

$files[$filename] = $code;
Comment on lines +88 to +89
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

ucwords() may produce unexpected results for namespaced schema names.

ucwords($name) capitalizes the first letter of each word (space-delimited by default). For Avro fullnames like com.example.User, this produces Com.example.user.php instead of the expected User.php. The registry key is the fullname, but the file should use only the simple name.

Consider using $registeredSchema->name() (the simple name) instead of the fullname for the filename:

Proposed fix
-                $filename = $path.'/'.ucwords($name).'.php';
+                $filename = $path.'/'.ucfirst($registeredSchema->name()).'.php';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lang/php/lib/Generator/AvroCodeGenerator.php` around lines 88 - 89, The
filename generation currently uses ucwords($name) where $name is the fullname
(e.g., "com.example.User"), which produces incorrect filenames for namespaced
schemas; update the code in AvroCodeGenerator.php to use the schema's simple
name (e.g., $registeredSchema->name()) when building $filename so that
$files[$filename] contains the correct file path (ucwords can be removed or
applied to the simple name if desired) and ensure the registry key can remain
the fullname in $name.

}
}

return $files;
}

private function buildRegistry(AvroSchema $rootSchema): void
{
$this->registry = [];
$this->collectSchemas($rootSchema);
}

private function collectSchemas(AvroSchema $schema): void
{
switch ($schema::class) {
case AvroRecordSchema::class:
if (!array_key_exists($schema->fullname(), $this->registry)) {
$this->registry[$schema->fullname()] = $schema;
foreach ($schema->fields() as $field) {
$this->collectSchemas($field->type());
}
}

break;
case AvroEnumSchema::class:
$this->registry[$schema->fullname()] = $schema;

break;
case AvroArraySchema::class:
$this->collectSchemas($schema->items());

break;
case AvroMapSchema::class:
$this->collectSchemas($schema->values());

break;
case AvroUnionSchema::class:
foreach ($schema->schemas() as $unionSchema) {
$this->collectSchemas($unionSchema);
}

break;
}
}

private function buildRecord(
AvroRecordSchema $avroRecord,
string $phpNamespace
): Node {
$className = ucwords($avroRecord->name());
$class = $this->factory->class($className)->makeFinal()->implement('\\JsonSerializable');

foreach ($avroRecord->fields() as $field) {
$phpType = $this->avroTypeToPhp($field->type(), $phpNamespace);
$property = $this->factory->property($field->name())
->makePrivate()
->setType($phpType);

$phpDocType = $this->avroTypeToPhpDoc($field->type(), $phpNamespace);
if (null !== $phpDocType) {
$property->setDocComment('/** @var '.$phpDocType.' */');
}

if ($field->hasDefaultValue()) {
$property->setDefault($this->buildDefault($field->defaultValue()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lang/php/lib/Generator/AvroCodeGenerator.php:154: Default values are injected without considering the field schema, so defaults for non-scalar typed fields (notably enums and records) will likely emit invalid PHP (e.g., a string default for a property typed as a generated enum). This can cause a fatal error when the generated class is loaded.

Severity: high

Other Locations
  • lang/php/lib/Generator/AvroCodeGenerator.php:166

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

}

$class->addStmt($property);
}

$constructor = $this->factory->method('__construct')->makePublic();
$constructorParamDocs = [];
foreach ($avroRecord->fields() as $field) {
$phpType = $this->avroTypeToPhp($field->type(), $phpNamespace);
$param = $this->factory->param($field->name())->setType($phpType);
if ($field->hasDefaultValue()) {
$param->setDefault($this->buildDefault($field->defaultValue()));
}

$phpDocType = $this->avroTypeToPhpDoc($field->type(), $phpNamespace);
if (null !== $phpDocType) {
$constructorParamDocs[] = '@param '.$phpDocType.' $'.$field->name();
}

$constructor->addParam($param);
$constructor->addStmt(
new Node\Expr\Assign(
new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $field->name()),
new Node\Expr\Variable($field->name())
)
);
}
if ([] !== $constructorParamDocs) {
$docLines = "/**\n";
foreach ($constructorParamDocs as $doc) {
$docLines .= ' * '.$doc."\n";
}
$docLines .= ' */';
$constructor->setDocComment($docLines);
}
$class->addStmt($constructor);

foreach ($avroRecord->fields() as $field) {
$phpType = $this->avroTypeToPhp($field->type(), $phpNamespace);
$getter = $this->factory->method($field->name())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lang/php/lib/Generator/AvroCodeGenerator.php:194: Generating property/method/parameter names directly from Avro field names can produce invalid PHP in edge cases (e.g., field name colliding with reserved keywords or with generated methods like jsonSerialize / __construct). When that happens, the generated file will fail to parse or will redeclare methods.

Severity: medium

Other Locations
  • lang/php/lib/Generator/AvroCodeGenerator.php:144
  • lang/php/lib/Generator/AvroCodeGenerator.php:164
  • lang/php/lib/Generator/AvroCodeGenerator.php:218

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

->makePublic()
->setReturnType($phpType)
->addStmt(
new Stmt\Return_(
new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $field->name())
)
);

$phpDocType = $this->avroTypeToPhpDoc($field->type(), $phpNamespace);
if (null !== $phpDocType) {
$getter->setDocComment('/** @return '.$phpDocType.' */');
}

$class->addStmt($getter);
}

$arrayItems = [];
foreach ($avroRecord->fields() as $field) {
$arrayItems[] = new Node\ArrayItem(
$this->buildJsonSerializeValue($field->type(), $field->name()),
new String_($field->name())
);
Comment on lines +213 to +216
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The class Node\ArrayItem does not exist in php-parser. Based on your other usages of the Node\Expr namespace in this file, this should be Node\Expr\ArrayItem. This will cause a runtime error when generating code for records with fields.

            $arrayItems[] = new Node\Expr\ArrayItem(
                $this->buildJsonSerializeValue($field->type(), $field->name()),
                new String_($field->name())
            );

}
$jsonSerialize = $this->factory->method('jsonSerialize')
->makePublic()
->setReturnType('mixed')
->addStmt(
new Stmt\Return_(
new Node\Expr\Array_($arrayItems, ['kind' => Node\Expr\Array_::KIND_SHORT])
)
);
$class->addStmt($jsonSerialize);

return $this->factory->namespace($phpNamespace)
->addStmt($class)
->getNode();
}

/**
* Builds the expression used inside jsonSerialize() for a single field.
*
* - EnumSchema → $this->field->value (plain string for Avro + JSON)
* - union[null, Enum] → $this->field?->value (null-safe, still plain)
* - anything else → $this->field
*/
private function buildJsonSerializeValue(AvroSchema $fieldType, string $fieldName): Node\Expr
{
$propertyFetch = new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $fieldName);

if ($fieldType instanceof AvroEnumSchema) {
return new Node\Expr\PropertyFetch($propertyFetch, 'value');
}

if ($fieldType instanceof AvroUnionSchema) {
$nonNullSchemas = array_values(array_filter(
$fieldType->schemas(),
static fn (AvroSchema $s): bool => !($s instanceof AvroPrimitiveSchema && AvroSchema::NULL_TYPE === $s->type())
));

if (1 === count($nonNullSchemas) && $nonNullSchemas[0] instanceof AvroEnumSchema) {
return new Node\Expr\NullsafePropertyFetch($propertyFetch, 'value');
}
}

return $propertyFetch;
}
Comment on lines +240 to +260
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The jsonSerialize logic correctly handles single Enum fields by accessing the ->value property, but it does not handle collections (arrays or maps) of Enums. Since the generated Enums are Backed Enums and do not implement JsonSerializable, json_encode will not automatically convert them to their string values when they are nested in arrays or maps. This will result in invalid Avro JSON output for schemas containing collections of enums.


/**
* @param list<string> $values
*/
private function buildEnum(
AvroEnumSchema $avroEnum,
string $phpNamespace,
array $values
): Node {
$className = ucwords($avroEnum->name());
$enum = $this->factory->enum($className)->setScalarType('string');

foreach ($values as $value) {
$caseName = strtoupper($value);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lang/php/lib/Generator/AvroCodeGenerator.php:274: Enum case names are derived via strtoupper($value), which can collapse distinct Avro symbols that differ only by case (e.g., foo vs FOO) into duplicate PHP case identifiers. That would make the generated enum invalid.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Enum case names can collide with PHP reserved words

Medium Severity

strtoupper($value) on valid Avro enum symbols like "null", "true", "false", or "class" produces "NULL", "TRUE", "FALSE", "CLASS" — all reserved identifiers in PHP 8.1+ that cannot be used as enum case names. The generated PHP code would fail to compile for any Avro enum containing these common symbols.

Fix in Cursor Fix in Web

$enum->addStmt(
$this->factory->enumCase($caseName)->setValue($value)
);
Comment on lines +273 to +277
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential enum case name collision when symbols differ only by case.

strtoupper($value) will produce identical case names for symbols like ["red", "RED"] or ["Yes", "YES"], resulting in duplicate enum cases and a PHP parse error in the generated code.

Consider adding a check for collisions or preserving original casing:

Proposed fix - detect collisions
+    private function buildEnumCases(array $values): array
+    {
+        $caseNames = [];
+        $statements = [];
+        foreach ($values as $value) {
+            $caseName = strtoupper($value);
+            if (isset($caseNames[$caseName])) {
+                throw new AvroCodeGeneratorException(
+                    sprintf('Enum case collision: "%s" and "%s" both map to "%s"', $caseNames[$caseName], $value, $caseName)
+                );
+            }
+            $caseNames[$caseName] = $value;
+            $statements[] = $this->factory->enumCase($caseName)->setValue($value);
+        }
+        return $statements;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lang/php/lib/Generator/AvroCodeGenerator.php` around lines 273 - 277, The
enum generation uses strtoupper($value) to make $caseName which can collide for
symbols that differ only by case (e.g. "red" vs "RED"); update the logic in
AvroCodeGenerator (the loop building $caseName and calling $enum->addStmt) to
detect duplicate $caseName values before adding cases and handle them
deterministically—either throw a clear error listing the conflicting symbols or
disambiguate by appending a unique suffix/index or using a safer transformation
that preserves uniqueness (e.g., include original chars or an incremental
suffix); ensure the chosen approach prevents duplicate enum cases and that the
error/handling references the original $value strings so generated code remains
valid.

}

return $this->factory->namespace($phpNamespace)
->addStmt($enum)
->getNode();
}

private function avroTypeToPhp(AvroSchema $schema, string $phpNamespace): string
{
return match (true) {
$schema instanceof AvroPrimitiveSchema => $this->avroPrimitiveTypeToPhp($schema),
$schema instanceof AvroArraySchema, $schema instanceof AvroMapSchema => 'array',
$schema instanceof AvroRecordSchema, $schema instanceof AvroEnumSchema => '\\'.$phpNamespace.'\\'.ucwords($schema->name()),
$schema instanceof AvroUnionSchema => $this->unionToPhp($schema, $phpNamespace),
default => 'mixed'
};
}

private function avroPrimitiveTypeToPhp(AvroPrimitiveSchema $primitiveSchema): string
{
return match ($primitiveSchema->type()) {
AvroSchema::NULL_TYPE => 'null',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lang/php/lib/Generator/AvroCodeGenerator.php:299: Mapping Avro null to the standalone PHP type null will generate code that does not compile on PHP 8.1 (standalone null types are only allowed starting in PHP 8.2), which conflicts with the package’s php: ^8.1 requirement. This also means the record_with_all_primitive_types expectation currently codifies output that can’t be executed on PHP 8.1.

Severity: high

Other Locations
  • lang/php/test/Generator/AvroCodeGeneratorTest.php:593
  • lang/php/test/Generator/AvroCodeGeneratorTest.php:601
  • lang/php/test/Generator/AvroCodeGeneratorTest.php:612

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

AvroSchema::BOOLEAN_TYPE => 'bool',
AvroSchema::INT_TYPE, AvroSchema::LONG_TYPE => 'int',
AvroSchema::FLOAT_TYPE, AvroSchema::DOUBLE_TYPE => 'float',
AvroSchema::STRING_TYPE, AvroSchema::BYTES_TYPE => 'string',
default => throw new AvroCodeGeneratorException("Unknown primitive type: ".$primitiveSchema->type()),
};
}

private function unionToPhp(AvroUnionSchema $union, string $phpNamespace): string
{
$types = [];
foreach ($union->schemas() as $schema) {
$types[] = $this->avroTypeToPhp($schema, $phpNamespace);
}

return implode('|', array_unique($types));
}

private function buildDefault(mixed $value): mixed
{
if (is_array($value)) {
return $this->factory->val($value);
}

return $value;
}

/**
* Returns a PHPDoc type string for schemas that need richer type info than
* what PHP's native type system can express (arrays and maps), or null when
* the native type hint is sufficient.
*/
private function avroTypeToPhpDoc(AvroSchema $schema, string $phpNamespace): ?string
{
return match (true) {
$schema instanceof AvroArraySchema => 'list<'.$this->avroTypeToPhpDocInner($schema->items(), $phpNamespace).'>',
$schema instanceof AvroMapSchema => 'array<string, '.$this->avroTypeToPhpDocInner($schema->values(), $phpNamespace).'>',
$schema instanceof AvroUnionSchema => $this->unionToPhpDoc($schema, $phpNamespace),
default => null,
};
}

private function avroTypeToPhpDocInner(AvroSchema $schema, string $phpNamespace): string
{
return match (true) {
$schema instanceof AvroPrimitiveSchema => $this->avroPrimitiveTypeToPhp($schema),
$schema instanceof AvroArraySchema => 'list<'.$this->avroTypeToPhpDocInner($schema->items(), $phpNamespace).'>',
$schema instanceof AvroMapSchema => 'array<string, '.$this->avroTypeToPhpDocInner($schema->values(), $phpNamespace).'>',
$schema instanceof AvroRecordSchema, $schema instanceof AvroEnumSchema => '\\'.$phpNamespace.'\\'.ucwords($schema->name()),
$schema instanceof AvroUnionSchema => $this->unionToPhp($schema, $phpNamespace),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PHPDoc loses generic types for unions in nested contexts

Low Severity

avroTypeToPhpDocInner delegates to unionToPhp for union schemas, which in turn calls avroTypeToPhp — mapping arrays/maps to the bare 'array' string. This discards generic type information in PHPDoc. For example, a field typed as array<["null", {"type":"array","items":"int"}]> would produce list<null|array> instead of the correct list<null|list<int>>. The union branch here needs to call avroTypeToPhpDocInner on each member instead.

Fix in Cursor Fix in Web

default => 'mixed',
};
}

private function unionToPhpDoc(AvroUnionSchema $union, string $phpNamespace): ?string
{
$hasArrayOrMap = false;
$docParts = [];

foreach ($union->schemas() as $schema) {
if ($schema instanceof AvroArraySchema || $schema instanceof AvroMapSchema) {
$hasArrayOrMap = true;
$docParts[] = $this->avroTypeToPhpDocInner($schema, $phpNamespace);
} else {
$docParts[] = $this->avroTypeToPhp($schema, $phpNamespace);
}
}

if (!$hasArrayOrMap) {
return null;
}

return implode('|', array_unique($docParts));
}
}
Loading
Loading