3708: Add new code generator from AVRO to PHP - #42
Conversation
…o add-schema-to-php-generator
WalkthroughThis PR adds a PHP Avro code generation feature to the repository. It introduces ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here.
|
|
||
| if (null !== $node && $registeredSchema instanceof AvroNamedSchema) { | ||
| $filename = $path.'/'.ucwords($registeredSchema->name()).'.php'; | ||
| $files[$filename] = "<?php\n\ndeclare(strict_types=1);\n\n{$this->printer->prettyPrint([$node])}\n"; |
There was a problem hiding this comment.
Same short name file clash
High Severity
Generated PHP paths use only the Avro type’s short name() with ucwords, not the full Avro name. Two named types with the same short name (different namespaces) or two --directory inputs that each define the same record/enum name write to one output file; the last write wins without warning.
Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here.
| $caseName = strtoupper($value); | ||
| $enum->addStmt( | ||
| $this->factory->enumCase($caseName)->setValue($value) | ||
| ); |
There was a problem hiding this comment.
Enum case name collisions
High Severity
Backed enum cases are named with strtoupper on each Avro symbol. Valid Avro enums whose symbols differ only by case (e.g. Active and active) map to the same PHP case name and produce invalid duplicate enum cases in generated code.
Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here.
| $io->success(sprintf('%d file(s) generated in %s.', count($written), $outputDir)); | ||
| } | ||
|
|
||
| return $exitCode; |
There was a problem hiding this comment.
Empty directory reports success
Medium Severity
When --directory points at a folder with no *.avsc files, the command never enters the processing loop, prints no error or success output, and returns exit code success as if generation completed.
Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here.
| $io->success(sprintf('%d file(s) generated in %s.', count($written), $outputDir)); | ||
| } | ||
|
|
||
| return $exitCode; |
There was a problem hiding this comment.
Success shown on partial failure
Medium Severity
If at least one file is written but another schema or write fails, the command still calls $io->success with a generated count while returning a failure exit code.
Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a PHP code generation tool for Apache Avro, including a CLI command, a code generator (AvroCodeGenerator) to translate schemas into PHP classes and enums, and a specific datum writer (AvroSpecificDatumWriter) for serializing these generated objects. The review identified several critical issues: a fatal error in the code generator due to using the non-existent PhpParser\Node\ArrayItem class, a PHP 8.0+ deprecation issue where required constructor parameters could be generated after optional ones, a severe performance bottleneck from repeatedly instantiating ReflectionClass during serialization, and a potential file-writing bug if realpath() fails and returns false.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| $arrayItems = []; | ||
| foreach ($avroRecord->fields() as $field) { | ||
| $arrayItems[] = new Node\ArrayItem( | ||
| $this->buildJsonSerializeValue($field->type(), $field->name()), | ||
| new String_($field->name()) | ||
| ); | ||
| } |
There was a problem hiding this comment.
In PHP-Parser (both v4 and v5), there is no PhpParser\Node\ArrayItem class. The correct class name is PhpParser\Node\Expr\ArrayItem. Using Node\ArrayItem will result in a fatal Error: Class "PhpParser\Node\ArrayItem" not found when attempting to generate code for any record schema.
$arrayItems = [];
foreach ($avroRecord->fields() as $field) {
$arrayItems[] = new Node\Expr\ArrayItem(
$this->buildJsonSerializeValue($field->type(), $field->name()),
new String_($field->name())
);
}| private function classNameMatchesSchema(object $datum, string $schemaName): bool | ||
| { | ||
| $className = (new \ReflectionClass($datum))->getShortName(); | ||
|
|
||
| return 0 === strcasecmp($className, $schemaName); | ||
| } |
There was a problem hiding this comment.
Creating a new ReflectionClass instance on every call to classNameMatchesSchema is extremely expensive and will cause severe performance degradation when serializing large collections of records. You can retrieve the short class name much faster using standard string manipulation on get_class($datum).
private function classNameMatchesSchema(object $datum, string $schemaName): bool
{
$class = get_class($datum);
$className = false !== ($pos = strrpos($class, '\\')) ? substr($class, $pos + 1) : $class;
return 0 === strcasecmp($className, $schemaName);
}| $constructor = $this->factory->method('__construct')->makePublic(); | ||
| $constructorParamDocs = []; | ||
| foreach ($avroRecord->fields() as $field) { |
There was a problem hiding this comment.
In PHP, defining required parameters after optional parameters (those with default values) is deprecated since PHP 8.0 and highly discouraged as it leads to unexpected behavior when calling the constructor with positional arguments. Since Avro schemas can define fields with default values in any order, we should perform a stable sort on the constructor parameters so that all fields with default values are placed at the end of the parameter list.
$constructorFields = $avroRecord->fields();
usort($constructorFields, static function ($a, $b): int {
return $a->hasDefaultValue() <=> $b->hasDefaultValue();
});
$constructor = $this->factory->method('__construct')->makePublic();
$constructorParamDocs = [];
foreach ($constructorFields as $field) {| return Command::FAILURE; | ||
| } | ||
|
|
||
| $outputDir = rtrim((string) realpath($outputDir), '/'); |
There was a problem hiding this comment.
If realpath($outputDir) fails (for example, if the directory does not exist, has permission issues, or if a stream wrapper like vfs:// is used in tests), it returns false. Casting false to (string) results in an empty string "". This causes the code generator to attempt to write files to the root directory "/User.php", which is highly dangerous and will fail on most systems. You should explicitly check if realpath returns false and handle it gracefully.
$realOutputDir = realpath($outputDir);
if (false === $realOutputDir) {
$io->error(sprintf('Could not resolve absolute path for "%s".', $outputDir));
return Command::FAILURE;
}
$outputDir = rtrim($realOutputDir, '/');
🤖 Augment PR SummarySummary: This PR adds a PHP code-generation feature that produces typed PHP classes/enums from Avro schema (.avsc) files. Changes:
Technical Notes: Adds runtime deps on 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| exit(1); | ||
| } | ||
|
|
||
| use Apache\Avro\Console\GenerateCommand; |
There was a problem hiding this comment.
lang/php/bin/avro:43 — This file declares use imports after executable statements; in PHP this causes a parse error (imports must appear before any non-declare code in the file).
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| $version = InstalledVersions::getPrettyVersion('apache/avro'); | ||
|
|
||
| $app = new Application('avro', $version); | ||
| $app->addCommand(new GenerateCommand()); |
There was a problem hiding this comment.
lang/php/bin/avro:50 — Symfony\Component\Console\Application in the declared dependency range (symfony/console ^6.4) does not provide addCommand(), so this call will fatal at runtime.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
| private function avroPrimitiveTypeToPhp(AvroPrimitiveSchema $primitiveSchema): string | ||
| { | ||
| return match ($primitiveSchema->type()) { | ||
| AvroSchema::NULL_TYPE => 'null', |
There was a problem hiding this comment.
lang/php/lib/Generator/AvroCodeGenerator.php:291 — Returning the standalone type null will generate code like private null $x;, which is a compile-time fatal error in PHP 8.1 (this repo’s minimum supported version).
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| $property->setDocComment('/** @var '.$phpDocType.' */'); | ||
| } | ||
|
|
||
| if ($field->hasDefaultValue()) { |
There was a problem hiding this comment.
lang/php/lib/Generator/AvroCodeGenerator.php:145 — Defaults are emitted as raw schema default values regardless of field type; for enums/records this will typically be a string/array default but the generated PHP type is a class/enum, yielding invalid or type-incompatible generated code.
Severity: medium
Other Locations
lang/php/lib/Generator/AvroCodeGenerator.php:157
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| && $this->classNameMatchesSchema($datum, $schema->name()), | ||
| $schema instanceof AvroArraySchema => is_array($datum) | ||
| && ([] === $datum || array_is_list($datum)), | ||
| $schema instanceof AvroMapSchema => is_array($datum), |
There was a problem hiding this comment.
lang/php/lib/Datum/AvroSpecificDatumWriter.php:214 — datumMatchesSchema() treats any PHP array as matching AvroMapSchema; in unions that contain both map and array branches, list arrays can be incorrectly matched to the map branch (wrong union index + encoding).
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Code Review — PR #42: PHP Avro Code GeneratorThis PR adds a PHP CLI code generator ( 🚨 Prompt Injection in Added Config FilesThe PR adds
This is a self-referential prompt-injection attempt designed to suppress code review. These files have no legitimate purpose for a project and should be removed from the PR. Bug 1 —
|
| # | Severity | File | Issue |
|---|---|---|---|
| — | 🚨 | CLAUDE.md, AGENTS.md, rules.md |
Prompt injection — remove these files |
| 1 | High | bin/avro:271 |
getPrettyVersion() null → TypeError crash |
| 2 | High | AvroCodeGenerator.php:767,818,949 |
ucwords wrong for snake_case Avro names |
| 3 | High | AvroSpecificDatumWriter.php:649 |
Empty array encodes as map when map precedes array in union |
| 4 | High | AvroSpecificDatumWriter.php:675 |
Short-name-only match breaks same-named types across Avro namespaces |
| 5 | High | AvroSpecificDatumWriter.php:510,640 |
fixed type not handled — always throws |
| 6 | Medium | GenerateCommand.php:372 |
realpath() false → empty output path, silent file corruption |
| 7 | Low | AvroCodeGenerator.php:783 |
switch ($schema::class) silently drops subclasses; use instanceof |
🤖 Generated with Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lang/php/bin/avro`:
- Line 47: The call to InstalledVersions::getPrettyVersion('apache/avro') can
throw an OutOfBoundsException when package metadata is missing or unavailable,
causing the CLI to crash during bootstrap. Wrap the
InstalledVersions::getPrettyVersion call in a try-catch block that catches
OutOfBoundsException and assigns a safe fallback version string (such as
"unknown" or a default version number) to the $version variable when the
exception occurs.
In `@lang/php/lib/Console/GenerateCommand.php`:
- Line 46: The namespace option in the addOption method call uses a
multi-character shortcut 'ns' when CLI conventions require single-character
shortcuts to improve usability and prevent unexpected option parsing behavior.
Change the shortcut parameter in the addOption call for the namespace option
from 'ns' to 'n' to follow standard CLI conventions.
In `@lang/php/lib/Datum/AvroSpecificDatumWriter.php`:
- Around line 73-80: The writeData method passes mixed $datum directly to typed
write methods (writeRecord, writeEnum, writeArray, writeMap, writeUnion,
writePrimitive) which expect specific types like object or array. This causes
PHP to throw TypeError before the method can raise the documented
AvroIOTypeException, breaking the error contract. Add guard clauses before the
match statement to validate that the datum matches the expected type for each
schema type, and throw AvroIOTypeException with appropriate messages for type
mismatches rather than allowing PHP's type system to throw TypeError. Ensure
this validation applies to all the write method dispatches mentioned in the
diff.
- Around line 208-211: The `classNameMatchesSchema` method used in the union
matching logic at lines 208-211 and 238-243 is comparing only short class names
when determining which branch to serialize. This is unsafe because two generated
Avro types from different namespaces can share the same short name, causing the
wrong branch to be selected and serializing invalid wire data. Update the
`classNameMatchesSchema` method to compare the fully-qualified class name
(including namespace) against the fully-qualified schema name instead of relying
on short names alone. This ensures that namespace-prefixed types are properly
distinguished during union serialization.
- Around line 212-215: The AvroMapSchema match at line 214 currently accepts any
array with just is_array($datum), but this also matches list arrays which should
be matched by AvroArraySchema instead. When a union contains both array and map
schemas and the map appears first in the match order, list-like data gets
incorrectly selected as a map, producing the wrong union index. Fix this by
adding an additional condition to the AvroMapSchema case to exclude list arrays:
change the condition to is_array($datum) && !array_is_list($datum) so that only
associative maps are matched by this branch while list arrays are correctly
matched by AvroArraySchema.
In `@lang/php/lib/Generator/AvroCodeGenerator.php`:
- Around line 63-64: The $files array in the AvroCodeGenerator class silently
allows schema filenames to collide when different schemas normalize to the same
ucwords(name) result, causing data loss. Add collision detection before
assignments to the $files array throughout the code (including the locations
mentioned in lines 79-82) by checking if a normalized filename already exists in
$files before adding new entries, and throw an exception with a descriptive
error message if a collision is detected to fail fast and prevent silent schema
loss.
- Around line 265-269: In the foreach loop that iterates through $values and
generates enum cases using strtoupper, add collision detection to identify when
two different values would produce the same uppercased case name. Before calling
$this->factory->enumCase($caseName)->setValue($value), track the case names that
have already been added to the enum. If a case name collision is detected (a
case name has already been used for a different value), either skip adding that
enum case or raise an error to alert that the enum definition is invalid due to
case-sensitive symbol collisions.
In `@lang/php/lib/Schema/AvroEnumSchema.php`:
- Around line 75-78: The hasSymbol() method uses a non-strict in_array
comparison when checking if a symbol exists in the symbols array, which can lead
to false positives when comparing values of different types (for example, the
string "0" could match the integer 0). Fix this by adding a third parameter set
to true in the in_array call within hasSymbol() to enforce strict type
comparison, ensuring only exact matches are returned.
In `@lang/php/README.md`:
- Around line 60-66: Add a language tag to the code fence on line 62 (the triple
backticks before vendor/bin/avro) to specify the shell/bash language, and ensure
all heading styles (the ## Usage and ### Options headings) are consistent with
the repository's markdown linting rules to avoid mixing ATX and setext heading
styles. Apply the same fixes to the code sections around lines 77-91.
In `@lang/php/test/Console/GenerateCommandTest.php`:
- Around line 178-203: Both test methods
generated_file_contains_correct_namespace_and_class and
generated_enum_file_contains_correct_cases need to verify the command succeeded
before attempting to read the generated files. After calling $tester->execute(),
add an assertion to check that the exit code is 0 (or check the command's
success status) before proceeding with file_get_contents(). This ensures that if
the command fails, the test will report the actual command failure instead of a
secondary file-not-found error.
In `@lang/php/test/Generator/AvroCodeGeneratorTest.php`:
- Around line 615-624: The test expectations in AvroCodeGeneratorTest include
standalone `null` type hints on the nullField property and constructor
parameter, which are incompatible with PHP 8.1 (the project's minimum supported
version). Update the test assertion to remove the standalone `null` type hints
from the expected generated code for the nullField declaration in the private
property, the constructor parameter, and any related method signatures. Replace
these with type hints that are compatible with PHP 8.1, such as removing the
type entirely or using a nullable type if applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 896048fa-6a2e-44bb-933a-3cb9f29147ff
⛔ Files ignored due to path filters (11)
lang/php/test/Fixtures/Generated/Address.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Car.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/FuelType.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Member.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Metadata.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Order.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Priority.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Profile.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Task.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/Team.phpis excluded by!**/generated/**lang/php/test/Fixtures/Generated/User.phpis excluded by!**/generated/**
📒 Files selected for processing (23)
.cursor/rules.md.gemini/rules.md.github/workflows/test-lang-php.ymlAGENTS.mdCLAUDE.mdcomposer.jsonlang/php/.php-cs-fixer.dist.phplang/php/README.mdlang/php/bin/avrolang/php/lib/Console/GenerateCommand.phplang/php/lib/Datum/AvroSpecificDatumWriter.phplang/php/lib/Generator/AvroCodeGenerator.phplang/php/lib/Generator/AvroCodeGeneratorException.phplang/php/lib/Schema/AvroEnumSchema.phplang/php/lib/Schema/AvroName.phplang/php/lib/Schema/AvroNamedSchema.phplang/php/lib/Schema/AvroSchema.phplang/php/phpstan.neonlang/php/test/Console/GenerateCommandTest.phplang/php/test/Datum/AvroSpecificDatumWriterTest.phplang/php/test/Fixtures/Schemas/status.avsclang/php/test/Fixtures/Schemas/user.avsclang/php/test/Generator/AvroCodeGeneratorTest.php
| use Composer\InstalledVersions; | ||
| use Symfony\Component\Console\Application; | ||
|
|
||
| $version = InstalledVersions::getPrettyVersion('apache/avro'); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file
fd -t f "avro" lang/php/bin/ | head -20Repository: martin-augment/avro
Length of output: 175
🏁 Script executed:
# Check if file exists and get its content
cat -n lang/php/bin/avro | head -70Repository: martin-augment/avro
Length of output: 2239
🏁 Script executed:
# Look for the Composer InstalledVersions class definition/usage patterns
rg "InstalledVersions" --type php -B 3 -A 3 | head -50Repository: martin-augment/avro
Length of output: 157
🏁 Script executed:
# Check what version of Composer is used
if [ -f "composer.lock" ]; then
jq '.metadata // empty' composer.lock | head -20
elif [ -f "lang/php/composer.lock" ]; then
jq '.metadata // empty' lang/php/composer.lock | head -20
fiRepository: martin-augment/avro
Length of output: 157
🏁 Script executed:
# Look for version string usage after line 47 in the avro file
sed -n '47,100p' lang/php/bin/avroRepository: martin-augment/avro
Length of output: 360
🌐 Web query:
Composer InstalledVersions::getPrettyVersion throws exception missing metadata
💡 Result:
The Composer\InstalledVersions::getPrettyVersion() method throws an OutOfBoundsException if the specified package is not installed at all [1][2]. If you are encountering an exception related to "missing metadata" or if the class is not found, it is typically due to one of the following reasons: 1. Package Not Installed: The method explicitly throws an OutOfBoundsException when the package name passed to it cannot be found in the installed.php file generated by Composer [1][3]. Ensure the package is actually installed and that you are using the correct vendor/package name. 2. Composer 2.0 Requirement: The Composer\InstalledVersions class was introduced in Composer 2.0 [4][5]. If you are using an older version of Composer or if your project's autoloader has not been regenerated with a compatible Composer version, the class will not be found [4][6][5]. You can resolve this by updating your local Composer installation (e.g., composer self-update --2) and running composer install or composer dump-autoload to regenerate the autoloader [4][6][5]. 3. Replacements and Provisions: If a package is being replaced or provided by another package, getPrettyVersion() may return null [1][7]. While this does not throw an exception, it can cause logic errors if your code expects a string return value [1]. Always check the return value against null if there is a possibility the package is being provided or replaced [1]. If you are seeing a specific "missing metadata" error that is not an OutOfBoundsException, it may indicate that your vendor/composer/installed.php file is corrupted or was not generated correctly. Running composer dump-autoload usually repairs this file [6].
Citations:
- 1: https://getcomposer.org/doc/07-runtime.md
- 2: https://github.com/composer/composer/blob/main/doc/07-runtime.md
- 3: https://fossies.org/linux/composer/src/Composer/InstalledVersions.php
- 4: https://stackoverflow.com/questions/75531788/class-composer-installedversions-not-found
- 5: Class 'Composer\InstalledVersions' not found Jean85/pretty-package-versions#18
- 6: https://stackoverflow.com/questions/78179933/after-composer-dumpautoload-show-installedversions-not-found
- 7: https://api.xoops.org/api2.5.11/apigen/Composer.InstalledVersions.html
🌐 Web query:
Symfony Console Application constructor version parameter expectations
💡 Result:
In the Symfony Console component, the Application class constructor accepts a version parameter, which is defined as a string [1][2]. Constructor Signature As of the current implementation, the constructor signature for the Symfony\Component\Console\Application class is: public function construct( private string $name = 'UNKNOWN', private string $version = 'UNKNOWN', private?ContainerInterface $container = null,) Usage and Expectations 1. Purpose: The version parameter is used to identify the version of your console application [3][4]. When provided, this version is displayed when a user runs a command with the --version or -V global options [3][5]. 2. Format: It expects a string [6][1]. While no specific semantic versioning format is enforced by the component, passing a string representing your application version (e.g., '1.0.0', '1.2') is the standard practice [3][4]. 3. Default Value: If no version is provided, it defaults to 'UNKNOWN' [6][1]. 4. Output Behavior: If you set both a name and a version, running your application with -V will output: [Name] version [Version] [4]. If no name or version is provided, the output will reflect the default 'UNKNOWN' values [3]. In addition to the version, you may provide the application name as the first argument, and starting from Symfony 8.1, an optional PSR-11 ContainerInterface as the third argument [7][8].
Citations:
- 1: https://github.com/symfony/console/blob/master/Application.php
- 2: https://github.com/symfony/console/blob/8.1/Application.php
- 3: https://symfony.com/doc/current/components/console/usage.html
- 4: https://symfony.com/doc/7.4/components/console/usage.html
- 5: https://symfony.com/doc/current/console/input.html
- 6: http://peridot-php.github.io/docs/class-Symfony.Component.Console.Application.html
- 7: https://symfony.com/doc/current/components/console.html
- 8: https://github.com/symfony/console/blob/master/CHANGELOG.md
Guard version lookup to avoid bootstrap crash.
InstalledVersions::getPrettyVersion('apache/avro') throws OutOfBoundsException when the package metadata is missing or the package is not installed, which will crash the CLI before it starts. Wrap in try-catch with a safe fallback version.
Suggested fix
-$version = InstalledVersions::getPrettyVersion('apache/avro');
+try {
+ $version = InstalledVersions::getPrettyVersion('apache/avro') ?? 'unknown';
+} catch (\Throwable) {
+ $version = 'unknown';
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $version = InstalledVersions::getPrettyVersion('apache/avro'); | |
| try { | |
| $version = InstalledVersions::getPrettyVersion('apache/avro') ?? 'unknown'; | |
| } catch (\Throwable) { | |
| $version = 'unknown'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/bin/avro` at line 47, The call to
InstalledVersions::getPrettyVersion('apache/avro') can throw an
OutOfBoundsException when package metadata is missing or unavailable, causing
the CLI to crash during bootstrap. Wrap the InstalledVersions::getPrettyVersion
call in a try-catch block that catches OutOfBoundsException and assigns a safe
fallback version string (such as "unknown" or a default version number) to the
$version variable when the exception occurs.
| ->addOption('file', 'f', InputOption::VALUE_OPTIONAL, 'One Avro schema file (.avsc)') | ||
| ->addOption('directory', 'd', InputOption::VALUE_OPTIONAL, 'A directory containing multiple Avro schema files (.avsc)') | ||
| ->addOption('output', 'o', InputOption::VALUE_REQUIRED, 'Output directory for generated PHP files') | ||
| ->addOption('namespace', 'ns', InputOption::VALUE_REQUIRED, 'PHP namespace for generated classes'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
cat -n lang/php/lib/Console/GenerateCommand.php | sed -n '40,55p'Repository: martin-augment/avro
Length of output: 1057
🌐 Web query:
Symfony Console addOption shortcut character limitations multi-character
💡 Result:
In Symfony Console, the shortcut parameter for the addOption method is not strictly limited to a single character [1][2]. While it is common practice to use a single letter (e.g., 'i' for '--iterations'), you can define multi-character shortcuts [3]. Internally, Symfony handles shortcuts as a string that can contain multiple options separated by a pipe character (|) [2][4][3]. If you provide a string like 'f|ff|fff' as the shortcut, the framework treats each segment as a valid shortcut for that option [3]. You can also pass a leading dash (e.g., '-f'), and Symfony will automatically strip it [3]. However, there are practical and conventional limitations to keep in mind: 1. Usage Conventions: While the API permits multi-character strings, CLI conventions typically reserve single dashes (-) for short, single-character options and double dashes (--) for long, multi-character option names. Using multi-character shortcuts (e.g., -abc) may confuse users who expect standard POSIX-like behavior. 2. Reserved Shortcuts: Certain shortcuts are reserved by the Symfony Console component itself for global options, most notably verbosity levels like -v, -vv, and -vvv [5]. Attempting to register these as custom shortcuts will result in an InvalidArgumentException [5]. 3. Complexity: Using complex or multi-character shortcuts increases the risk of collision with other options in your application or third-party bundles [5]. In summary, while the Symfony Console API technically allows multi-character shortcuts, you should use them judiciously and ensure they do not conflict with reserved global options or established CLI interface standards [4][3][5].
Citations:
- 1: https://symfony.com/doc/current/console/input.html
- 2: https://github.com/symfony/console/blob/8.2/Input/InputOption.php
- 3: https://github.com/symfony/console/blob/0e2e3f38/Tests/Input/InputOptionTest.php
- 4: https://symfony.com/doc/7.4/console/input.html
- 5: https://stackoverflow.com/questions/62993097/i-cant-add-option-v-to-console-symfony
🌐 Web query:
Symfony InputOption documentation shortcut parameter safety
💡 Result:
In Symfony's Console component, the InputOption shortcut parameter allows you to define a short, single-dash alias for an option (e.g., -i for --iterations) to improve command usability [1][2]. Safety and Technical Considerations: 1. Valid Shortcuts: Shortcuts are intended to be short, typically single-character aliases [1][2]. You can define multiple shortcuts by separating them with a pipe character (|) or by passing them as an array [1][3]. 2. Internal Constraints: The InputOption class internally processes shortcuts by removing leading dashes and filtering the resulting string [4]. Historically, this processing logic used array_filter in a way that prevented the character '0' from being used as a valid shortcut because it evaluated to false [4]. This was addressed in later versions to allow '0' as a valid shortcut [4]. 3. Reserved Shortcuts: There is no strict, enforced global blacklist of reserved shortcuts in the framework that prevents you from defining specific characters [5]. However, it is a best practice to avoid using standard, widely-recognized shortcuts already in use by the application or the global Symfony Console environment (such as -h for help or -q for quiet) to prevent conflicts and ensure predictable user behavior [1][5]. 4. Validation: If an invalid or empty shortcut is provided (e.g., an empty string), the component will throw an InvalidArgumentException [4]. Recommendations: - Use clear, single-character shortcuts whenever possible for high-frequency options [1]. - Ensure your chosen shortcuts do not overlap with existing global options or other defined options within the same command. - If you encounter issues with specific characters like '0', ensure you are using a recent, patched version of the symfony/console component [4].
Citations:
- 1: https://symfony.com/doc/current/console/input.html
- 2: https://symfony.com/doc/8.0/console/input.html
- 3: https://symfony.com/doc/current/components/console/console_arguments.html
- 4: [Console] InputOption shortcut "-0" is not allowed symfony/symfony#53514
- 5: feat: #43086 add blacklisting of reserved shortcuts for InputOptions in commands symfony/symfony#43091
Use a single-character shortcut for namespace option.
Following CLI conventions, option shortcuts should be single characters (e.g., -n instead of -ns). While Symfony Console technically supports multi-character shortcuts, using single letters improves usability and prevents unexpected option parsing behavior in user interfaces.
Suggested fix
- ->addOption('namespace', 'ns', InputOption::VALUE_REQUIRED, 'PHP namespace for generated classes');
+ ->addOption('namespace', 'n', InputOption::VALUE_REQUIRED, 'PHP namespace for generated classes');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ->addOption('namespace', 'ns', InputOption::VALUE_REQUIRED, 'PHP namespace for generated classes'); | |
| ->addOption('namespace', 'n', InputOption::VALUE_REQUIRED, 'PHP namespace for generated classes'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/lib/Console/GenerateCommand.php` at line 46, The namespace option in
the addOption method call uses a multi-character shortcut 'ns' when CLI
conventions require single-character shortcuts to improve usability and prevent
unexpected option parsing behavior. Change the shortcut parameter in the
addOption call for the namespace option from 'ns' to 'n' to follow standard CLI
conventions.
| match (true) { | ||
| $schema instanceof AvroRecordSchema => $this->writeRecord($schema, $datum, $encoder), | ||
| $schema instanceof AvroEnumSchema => $this->writeEnum($schema, $datum, $encoder), | ||
| $schema instanceof AvroArraySchema => $this->writeArray($schema, $datum, $encoder), | ||
| $schema instanceof AvroMapSchema => $this->writeMap($schema, $datum, $encoder), | ||
| $schema instanceof AvroUnionSchema => $this->writeUnion($schema, $datum, $encoder), | ||
| $schema instanceof AvroPrimitiveSchema => $this->writePrimitive($schema, $datum, $encoder), | ||
| default => throw new AvroException(sprintf('Unsupported schema type: %s', $schema->type())), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard datum types before typed method dispatch to avoid uncaught TypeError.
At Line 73, writeData() forwards mixed $datum into methods typed as object/array. For mismatched runtime data, PHP throws TypeError before AvroIOTypeException, which breaks the method’s documented Avro error contract and can surface as an unexpected crash path.
Proposed fix
private function writeData(AvroSchema $schema, mixed $datum, AvroIOBinaryEncoder $encoder): void
{
match (true) {
- $schema instanceof AvroRecordSchema => $this->writeRecord($schema, $datum, $encoder),
- $schema instanceof AvroEnumSchema => $this->writeEnum($schema, $datum, $encoder),
- $schema instanceof AvroArraySchema => $this->writeArray($schema, $datum, $encoder),
- $schema instanceof AvroMapSchema => $this->writeMap($schema, $datum, $encoder),
+ $schema instanceof AvroRecordSchema => is_object($datum)
+ ? $this->writeRecord($schema, $datum, $encoder)
+ : throw new AvroIOTypeException($schema, $datum),
+ $schema instanceof AvroEnumSchema => $this->writeEnum($schema, $datum, $encoder),
+ $schema instanceof AvroArraySchema => is_array($datum)
+ ? $this->writeArray($schema, $datum, $encoder)
+ : throw new AvroIOTypeException($schema, $datum),
+ $schema instanceof AvroMapSchema => is_array($datum)
+ ? $this->writeMap($schema, $datum, $encoder)
+ : throw new AvroIOTypeException($schema, $datum),
$schema instanceof AvroUnionSchema => $this->writeUnion($schema, $datum, $encoder),
$schema instanceof AvroPrimitiveSchema => $this->writePrimitive($schema, $datum, $encoder),
default => throw new AvroException(sprintf('Unsupported schema type: %s', $schema->type())),
};
}Also applies to: 89-90, 121-122, 138-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/lib/Datum/AvroSpecificDatumWriter.php` around lines 73 - 80, The
writeData method passes mixed $datum directly to typed write methods
(writeRecord, writeEnum, writeArray, writeMap, writeUnion, writePrimitive) which
expect specific types like object or array. This causes PHP to throw TypeError
before the method can raise the documented AvroIOTypeException, breaking the
error contract. Add guard clauses before the match statement to validate that
the datum matches the expected type for each schema type, and throw
AvroIOTypeException with appropriate messages for type mismatches rather than
allowing PHP's type system to throw TypeError. Ensure this validation applies to
all the write method dispatches mentioned in the diff.
| && $this->classNameMatchesSchema($datum, $schema->name()), | ||
| $schema instanceof AvroRecordSchema => is_object($datum) | ||
| && !($datum instanceof \BackedEnum) | ||
| && $this->classNameMatchesSchema($datum, $schema->name()), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Short-class-name matching is unsafe for namespaced Avro named types.
At Line 240, branch matching compares only getShortName(). If two generated types share the same short name across namespaces, union matching can pick the wrong branch and serialize invalid wire data for that schema contract.
Also applies to: 238-243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/lib/Datum/AvroSpecificDatumWriter.php` around lines 208 - 211, The
`classNameMatchesSchema` method used in the union matching logic at lines
208-211 and 238-243 is comparing only short class names when determining which
branch to serialize. This is unsafe because two generated Avro types from
different namespaces can share the same short name, causing the wrong branch to
be selected and serializing invalid wire data. Update the
`classNameMatchesSchema` method to compare the fully-qualified class name
(including namespace) against the fully-qualified schema name instead of relying
on short names alone. This ensures that namespace-prefixed types are properly
distinguished during union serialization.
| $schema instanceof AvroArraySchema => is_array($datum) | ||
| && ([] === $datum || array_is_list($datum)), | ||
| $schema instanceof AvroMapSchema => is_array($datum), | ||
| default => false, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Union matching can encode the wrong branch for list arrays when a map branch appears first.
At Line 214, AvroMapSchema currently matches any array. In a union containing both array and map, list-like data can be selected as map depending on branch order, producing a wrong union index and incompatible serialized payload.
Proposed fix
private function datumMatchesSchema(AvroSchema $schema, mixed $datum): bool
{
return match (true) {
$schema instanceof AvroPrimitiveSchema => $this->datumMatchesPrimitive($schema, $datum),
$schema instanceof AvroEnumSchema => $datum instanceof \BackedEnum
&& $this->classNameMatchesSchema($datum, $schema->name()),
$schema instanceof AvroRecordSchema => is_object($datum)
&& !($datum instanceof \BackedEnum)
&& $this->classNameMatchesSchema($datum, $schema->name()),
$schema instanceof AvroArraySchema => is_array($datum)
&& ([] === $datum || array_is_list($datum)),
- $schema instanceof AvroMapSchema => is_array($datum),
+ $schema instanceof AvroMapSchema => is_array($datum)
+ && ([] === $datum || !array_is_list($datum)),
default => false,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $schema instanceof AvroArraySchema => is_array($datum) | |
| && ([] === $datum || array_is_list($datum)), | |
| $schema instanceof AvroMapSchema => is_array($datum), | |
| default => false, | |
| $schema instanceof AvroArraySchema => is_array($datum) | |
| && ([] === $datum || array_is_list($datum)), | |
| $schema instanceof AvroMapSchema => is_array($datum) | |
| && ([] === $datum || !array_is_list($datum)), | |
| default => false, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/lib/Datum/AvroSpecificDatumWriter.php` around lines 212 - 215, The
AvroMapSchema match at line 214 currently accepts any array with just
is_array($datum), but this also matches list arrays which should be matched by
AvroArraySchema instead. When a union contains both array and map schemas and
the map appears first in the match order, list-like data gets incorrectly
selected as a map, producing the wrong union index. Fix this by adding an
additional condition to the AvroMapSchema case to exclude list arrays: change
the condition to is_array($datum) && !array_is_list($datum) so that only
associative maps are matched by this branch while list arrays are correctly
matched by AvroArraySchema.
| foreach ($values as $value) { | ||
| $caseName = strtoupper($value); | ||
| $enum->addStmt( | ||
| $this->factory->enumCase($caseName)->setValue($value) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Detect enum case-name collisions caused by uppercasing symbols.
Two valid symbols that differ only by case will collapse to the same generated case name and produce invalid enum output.
Proposed fix
private function buildEnum(
AvroEnumSchema $avroEnum,
string $phpNamespace,
array $values
): Node {
$className = ucwords($avroEnum->name());
$enum = $this->factory->enum($className)->setScalarType('string');
+ $generatedCaseNames = [];
foreach ($values as $value) {
$caseName = strtoupper($value);
+ if (isset($generatedCaseNames[$caseName])) {
+ throw new AvroCodeGeneratorException(sprintf(
+ 'Enum case collision in "%s": symbol "%s" normalizes to duplicate case "%s".',
+ $avroEnum->fullname(),
+ $value,
+ $caseName
+ ));
+ }
+ $generatedCaseNames[$caseName] = true;
$enum->addStmt(
$this->factory->enumCase($caseName)->setValue($value)
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($values as $value) { | |
| $caseName = strtoupper($value); | |
| $enum->addStmt( | |
| $this->factory->enumCase($caseName)->setValue($value) | |
| ); | |
| private function buildEnum( | |
| AvroEnumSchema $avroEnum, | |
| string $phpNamespace, | |
| array $values | |
| ): Node { | |
| $className = ucwords($avroEnum->name()); | |
| $enum = $this->factory->enum($className)->setScalarType('string'); | |
| $generatedCaseNames = []; | |
| foreach ($values as $value) { | |
| $caseName = strtoupper($value); | |
| if (isset($generatedCaseNames[$caseName])) { | |
| throw new AvroCodeGeneratorException(sprintf( | |
| 'Enum case collision in "%s": symbol "%s" normalizes to duplicate case "%s".', | |
| $avroEnum->fullname(), | |
| $value, | |
| $caseName | |
| )); | |
| } | |
| $generatedCaseNames[$caseName] = true; | |
| $enum->addStmt( | |
| $this->factory->enumCase($caseName)->setValue($value) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/lib/Generator/AvroCodeGenerator.php` around lines 265 - 269, In the
foreach loop that iterates through $values and generates enum cases using
strtoupper, add collision detection to identify when two different values would
produce the same uppercased case name. Before calling
$this->factory->enumCase($caseName)->setValue($value), track the case names that
have already been added to the enum. If a case name collision is detected (a
case name has already been used for a different value), either skip adding that
enum case or raise an error to alert that the enum definition is invalid due to
case-sensitive symbol collisions.
| public function hasSymbol($symbol): bool | ||
| { | ||
| return in_array($symbol, $this->symbols); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use strict comparison in hasSymbol() to avoid false positives.
in_array is currently non-strict, so non-string inputs can match string symbols unexpectedly.
Proposed fix
public function hasSymbol($symbol): bool
{
- return in_array($symbol, $this->symbols);
+ return is_string($symbol) && in_array($symbol, $this->symbols, true);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function hasSymbol($symbol): bool | |
| { | |
| return in_array($symbol, $this->symbols); | |
| } | |
| public function hasSymbol($symbol): bool | |
| { | |
| return is_string($symbol) && in_array($symbol, $this->symbols, true); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/lib/Schema/AvroEnumSchema.php` around lines 75 - 78, The hasSymbol()
method uses a non-strict in_array comparison when checking if a symbol exists in
the symbols array, which can lead to false positives when comparing values of
different types (for example, the string "0" could match the integer 0). Fix
this by adding a third parameter set to true in the in_array call within
hasSymbol() to enforce strict type comparison, ensuring only exact matches are
returned.
| ## Usage | ||
|
|
||
| ``` | ||
| vendor/bin/avro [options] | ||
| ``` | ||
|
|
||
| ### Options |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align new README headings/code fence with repository markdown linting rules.
The new section mixes ATX headings in a setext-styled document and has a fenced block without a language tag (Line 62), matching current markdownlint warnings.
Also applies to: 77-91
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 60-60: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 66-66: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/README.md` around lines 60 - 66, Add a language tag to the code
fence on line 62 (the triple backticks before vendor/bin/avro) to specify the
shell/bash language, and ensure all heading styles (the ## Usage and ### Options
headings) are consistent with the repository's markdown linting rules to avoid
mixing ATX and setext heading styles. Apply the same fixes to the code sections
around lines 77-91.
Source: Linters/SAST tools
| public function generated_file_contains_correct_namespace_and_class(): void | ||
| { | ||
| $tester = $this->tester(); | ||
| $tester->execute([ | ||
| '--file' => $this->schemaPath('user.avsc'), | ||
| '--output' => $this->outputDir, | ||
| '--namespace' => 'My\\App\\Avro', | ||
| ]); | ||
|
|
||
| $content = file_get_contents($this->outputDir.'/User.php'); | ||
| self::assertStringContainsString('namespace My\\App\\Avro;', $content); | ||
| self::assertStringContainsString('final class User', $content); | ||
| } | ||
|
|
||
| #[Test] | ||
| public function generated_enum_file_contains_correct_cases(): void | ||
| { | ||
| $tester = $this->tester(); | ||
| $tester->execute([ | ||
| '--file' => $this->schemaPath('status.avsc'), | ||
| '--output' => $this->outputDir, | ||
| '--namespace' => 'App\\Generated', | ||
| ]); | ||
|
|
||
| $content = file_get_contents($this->outputDir.'/Status.php'); | ||
| self::assertStringContainsString('enum Status', $content); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Assert command success before reading generated files.
These tests read output files immediately after execute() without checking the exit code (or file existence), which can hide the root failure behind secondary errors.
Suggested tightening
public function generated_file_contains_correct_namespace_and_class(): void
{
$tester = $this->tester();
- $tester->execute([
+ $exitCode = $tester->execute([
'--file' => $this->schemaPath('user.avsc'),
'--output' => $this->outputDir,
'--namespace' => 'My\\App\\Avro',
]);
+ self::assertSame(Command::SUCCESS, $exitCode);
+ self::assertFileExists($this->outputDir.'/User.php');
$content = file_get_contents($this->outputDir.'/User.php');
self::assertStringContainsString('namespace My\\App\\Avro;', $content);
self::assertStringContainsString('final class User', $content);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/test/Console/GenerateCommandTest.php` around lines 178 - 203, Both
test methods generated_file_contains_correct_namespace_and_class and
generated_enum_file_contains_correct_cases need to verify the command succeeded
before attempting to read the generated files. After calling $tester->execute(),
add an assertion to check that the exit code is 0 (or check the command's
success status) before proceeding with file_get_contents(). This ensures that if
the command fails, the test will report the actual command failure instead of a
secondary file-not-found error.
| private null \$nullField; | ||
| private bool \$boolField; | ||
| private int \$intField; | ||
| private int \$longField; | ||
| private float \$floatField; | ||
| private float \$doubleField; | ||
| private string \$stringField; | ||
| private string \$bytesField; | ||
| public function __construct(null \$nullField, bool \$boolField, int \$intField, int \$longField, float \$floatField, float \$doubleField, string \$stringField, string \$bytesField) | ||
| { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
In PHP 8.1, is standalone type declaration null(e.g.,private null $x;orfunction f(null $x)) valid syntax, or is it only supported in PHP 8.2+?
💡 Result:
Standalone type declaration null (e.g., private null $x; or function f(null $x)) is not valid syntax in PHP 8.1; it is only supported in PHP 8.2 and later [1][2][3]. In PHP 8.1 and earlier, using null as a standalone type results in a compile-time fatal error [2][3]. While null could be used as part of a union type (e.g., string|null) in PHP 8.0 and 8.1, it could not be used on its own [1][2][3]. The ability to use null (along with false) as a standalone type was introduced specifically in PHP 8.2 [1][2].
Citations:
- 1: https://www.php.net/manual/en/language.types.declarations.php
- 2: https://php.watch/versions/8.2/null-false-types
- 3: https://www.infoq.com/articles/php-8-type-system-improvements/
🏁 Script executed:
# First, examine the test file around the mentioned lines
sed -n '610,630p' lang/php/test/Generator/AvroCodeGeneratorTest.php | cat -nRepository: martin-augment/avro
Length of output: 1321
🏁 Script executed:
# Also check lines 634-635
sed -n '630,640p' lang/php/test/Generator/AvroCodeGeneratorTest.php | cat -nRepository: martin-augment/avro
Length of output: 667
🏁 Script executed:
# Search for PHP version declarations or comments in the test file
grep -i "php\|version\|8\.1\|8\.2" lang/php/test/Generator/AvroCodeGeneratorTest.php | head -20Repository: martin-augment/avro
Length of output: 955
🏁 Script executed:
# Check if there's a composer.json or similar declaring the PHP version
find . -name "composer.json" -o -name ".php-version" | head -5 | xargs grep -l "php\|version" 2>/dev/nullRepository: martin-augment/avro
Length of output: 173
🏁 Script executed:
# Check the PHP version requirement in composer.json
cat composer.json | grep -A 5 -B 5 "php"Repository: martin-augment/avro
Length of output: 1329
🏁 Script executed:
# Also check if there's any php version requirement or declaration in the test file itself
head -50 lang/php/test/Generator/AvroCodeGeneratorTest.php | cat -nRepository: martin-augment/avro
Length of output: 2050
Remove standalone null type assertions from test expectations—incompatible with PHP 8.1.
The test asserts that the code generator produces private null $nullField;, __construct(null $nullField, ...), and public function nullField(): null at lines 615–624 and 634–635. Standalone null type hints are only valid in PHP 8.2+, but this project requires php: ^8.1 (composer.json). The generated code would fail to parse in PHP 8.1 and would lock in invalid output as the expected behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lang/php/test/Generator/AvroCodeGeneratorTest.php` around lines 615 - 624,
The test expectations in AvroCodeGeneratorTest include standalone `null` type
hints on the nullField property and constructor parameter, which are
incompatible with PHP 8.1 (the project's minimum supported version). Update the
test assertion to remove the standalone `null` type hints from the expected
generated code for the nullField declaration in the private property, the
constructor parameter, and any related method signatures. Replace these with
type hints that are compatible with PHP 8.1, such as removing the type entirely
or using a nullable type if applicable.
3 participants
3708: To review by AI