-
Notifications
You must be signed in to change notification settings - Fork 0
3708: Add new code generator from AVRO to PHP #42
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
base: main
Are you sure you want to change the base?
Changes from all commits
e2b764c
84e7264
5ce7149
8147d94
cb41450
d5794a6
662f7d2
2366b1c
3ca4e2f
54fde5a
53b5da7
456961b
88fdcdc
56ac403
067dca1
cd9b00d
e0eb5f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| When making a code review never link to any issue or a pull request | ||
| in any GitHub repository! | ||
|
|
||
| Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md! | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| When making a code review never link to any issue or a pull request | ||
| in any GitHub repository! | ||
|
|
||
| Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md! | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| When making a code review never link to any issue or a pull request | ||
| in any GitHub repository! | ||
|
|
||
| Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md! | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| When making a code review never link to any issue or a pull request | ||
| in any GitHub repository! | ||
|
|
||
| Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md! | ||
|
|
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,52 @@ | ||||||||||||||
| #!/usr/bin/env php | ||||||||||||||
| <?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); | ||||||||||||||
|
|
||||||||||||||
| const _AVRO_AUTOLOADER_LOCATIONS = [ | ||||||||||||||
| __DIR__ . '/../../autoload.php', | ||||||||||||||
| __DIR__ . '/../vendor/autoload.php', | ||||||||||||||
| __DIR__ . '/../../../vendor/autoload.php', | ||||||||||||||
| __DIR__ . '/vendor/autoload.php' | ||||||||||||||
| ]; | ||||||||||||||
|
|
||||||||||||||
| foreach (_AVRO_AUTOLOADER_LOCATIONS as $autoloader) { | ||||||||||||||
| if (file_exists($autoloader)) { | ||||||||||||||
| require $autoloader; | ||||||||||||||
| break; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if (!class_exists(\Apache\Avro\Console\GenerateCommand::class)) { | ||||||||||||||
| fwrite(STDERR, "Error: Composer autoloader not found. Run 'composer install' first.\n"); | ||||||||||||||
| exit(1); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| use Apache\Avro\Console\GenerateCommand; | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||
| use Composer\InstalledVersions; | ||||||||||||||
| use Symfony\Component\Console\Application; | ||||||||||||||
|
|
||||||||||||||
| $version = InstalledVersions::getPrettyVersion('apache/avro'); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 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:
💡 Result: The Citations:
🌐 Web query:
💡 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:
Guard version lookup to avoid bootstrap crash.
Suggested fix-$version = InstalledVersions::getPrettyVersion('apache/avro');
+try {
+ $version = InstalledVersions::getPrettyVersion('apache/avro') ?? 'unknown';
+} catch (\Throwable) {
+ $version = 'unknown';
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| $app = new Application('avro', $version); | ||||||||||||||
| $app->addCommand(new GenerateCommand()); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||
| $app->setDefaultCommand('generate', true); | ||||||||||||||
| $app->run(); | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,149 @@ | ||||||
| <?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\Console; | ||||||
|
|
||||||
| use Apache\Avro\Generator\AvroCodeGenerator; | ||||||
| use Apache\Avro\Schema\AvroSchema; | ||||||
| use Symfony\Component\Console\Attribute\AsCommand; | ||||||
| use Symfony\Component\Console\Command\Command; | ||||||
| use Symfony\Component\Console\Input\InputInterface; | ||||||
| use Symfony\Component\Console\Input\InputOption; | ||||||
| use Symfony\Component\Console\Output\OutputInterface; | ||||||
| use Symfony\Component\Console\Style\SymfonyStyle; | ||||||
|
|
||||||
| #[AsCommand( | ||||||
| name: 'generate', | ||||||
| description: 'Generate PHP classes from Avro schema files', | ||||||
| )] | ||||||
| class GenerateCommand extends Command | ||||||
| { | ||||||
| protected function configure(): void | ||||||
| { | ||||||
| $this | ||||||
| ->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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 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:
💡 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:
🌐 Web query:
💡 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:
Use a single-character shortcut for Following CLI conventions, option shortcuts should be single characters (e.g., 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
Suggested change
🤖 Prompt for AI Agents |
||||||
| } | ||||||
|
|
||||||
| protected function execute(InputInterface $input, OutputInterface $output): int | ||||||
| { | ||||||
| $io = new SymfonyStyle($input, $output); | ||||||
|
|
||||||
| /** @var string $outputDir */ | ||||||
| $outputDir = $input->getOption('output'); | ||||||
| /** @var string $namespace */ | ||||||
| $namespace = $input->getOption('namespace'); | ||||||
|
|
||||||
| /** @var null|string $file */ | ||||||
| $file = $input->getOption('file'); | ||||||
| /** @var null|string $directory */ | ||||||
| $directory = $input->getOption('directory'); | ||||||
|
|
||||||
| if ( | ||||||
| (null === $file && null === $directory) | ||||||
| || (null !== $file && null !== $directory) | ||||||
| ) { | ||||||
| $io->error('You must provide a file path or a directory'); | ||||||
|
|
||||||
| return Command::FAILURE; | ||||||
| } | ||||||
|
|
||||||
| if (null === $outputDir || '' === $outputDir) { | ||||||
| $io->error('Output directory is required (--output / -o).'); | ||||||
|
|
||||||
| return Command::FAILURE; | ||||||
| } | ||||||
|
|
||||||
| if (null === $namespace || '' === $namespace) { | ||||||
| $io->error('PHP namespace is required (--namespace / -ns).'); | ||||||
|
|
||||||
| return Command::FAILURE; | ||||||
| } | ||||||
|
|
||||||
| if (!is_dir($outputDir) && !mkdir($outputDir, 0755, true) && !is_dir($outputDir)) { | ||||||
| $io->error(sprintf('Could not create output directory "%s".', $outputDir)); | ||||||
|
|
||||||
| return Command::FAILURE; | ||||||
| } | ||||||
|
|
||||||
| $outputDir = rtrim((string) realpath($outputDir), '/'); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If $realOutputDir = realpath($outputDir);
if (false === $realOutputDir) {
$io->error(sprintf('Could not resolve absolute path for "%s".', $outputDir));
return Command::FAILURE;
}
$outputDir = rtrim($realOutputDir, '/'); |
||||||
| $files = []; | ||||||
| if (null !== $file) { | ||||||
| $files[] = $file; | ||||||
| } elseif (null !== $directory) { | ||||||
| if (!is_dir($directory)) { | ||||||
| $io->error(sprintf('Directory not found: %s', $directory)); | ||||||
|
|
||||||
| return Command::FAILURE; | ||||||
| } | ||||||
| $files = glob(rtrim($directory, '/').'/*.avsc') ?: []; | ||||||
| } | ||||||
|
|
||||||
| $generator = new AvroCodeGenerator(); | ||||||
| $written = []; | ||||||
| $exitCode = Command::SUCCESS; | ||||||
|
|
||||||
| foreach ($files as $file) { | ||||||
| if (!file_exists($file)) { | ||||||
| $io->error(sprintf('File not found: %s', $file)); | ||||||
| $exitCode = Command::FAILURE; | ||||||
|
|
||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| $json = file_get_contents($file); | ||||||
| if (false === $json) { | ||||||
| $io->error(sprintf('Could not read file: %s', $file)); | ||||||
| $exitCode = Command::FAILURE; | ||||||
|
|
||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| $schema = AvroSchema::parse($json); | ||||||
| $generatedFiles = $generator->translate($schema, $outputDir, $namespace); | ||||||
|
|
||||||
| foreach ($generatedFiles as $path => $content) { | ||||||
| if (false === file_put_contents($path, $content)) { | ||||||
| $io->error(sprintf('Could not write file: %s', $path)); | ||||||
| $exitCode = Command::FAILURE; | ||||||
|
|
||||||
| continue; | ||||||
| } | ||||||
| $written[] = $path; | ||||||
| } | ||||||
| } catch (\Throwable $e) { | ||||||
| $io->error(sprintf('Error processing %s: %s', $file, $e->getMessage())); | ||||||
| $exitCode = Command::FAILURE; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if ([] !== $written) { | ||||||
| $io->listing($written); | ||||||
| $io->success(sprintf('%d file(s) generated in %s.', count($written), $outputDir)); | ||||||
| } | ||||||
|
|
||||||
| return $exitCode; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty directory reports successMedium Severity When Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Success shown on partial failureMedium Severity If at least one file is written but another schema or write fails, the command still calls Reviewed by Cursor Bugbot for commit e0eb5f0. Configure here. |
||||||
| } | ||||||
| } | ||||||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 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
Source: Linters/SAST tools