Fix vendor autoloader dev entries when target is vendor #200

BrianHenryIE · 2025-06-25T23:51:36Z

No description provided.

gemini-code-assist

Summary of Changes

Hello @BrianHenryIE, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an issue where development dependencies were incorrectly excluded from the autoloader when the build target directory was the vendor directory itself. The changes introduce conditional logic to ensure dev entries are properly retained in this specific scenario and optimize the autoloader generation process by skipping unnecessary prefixing. A new integration test has been added to validate the fix.

Highlights

Autoloader Dev Entry Handling: The core logic for generating the autoloader in DumpAutoload.php has been modified. Previously, devMode was unconditionally set to false. Now, devMode is only set to false if the target_directory is different from the vendor_directory. This change ensures that development dependencies are correctly included in the autoloader when the autoloader is being generated directly into the vendor directory, addressing the issue of dev entries being removed.
Autoloader Prefixing Optimization: A new conditional check has been added to the prefixNewAutoloader method in DumpAutoload.php. This method will now return early and skip the prefixing process if the target_directory is the same as the vendor_directory. This prevents unnecessary prefixing operations in scenarios where the autoloader is being built directly within the vendor folder.
New Integration Test: A new integration test, StraussIssue198Test.php, has been introduced. This test specifically validates the fix by setting the target_directory to vendor and asserting that both regular and development dependencies are correctly present in the generated installed.json and autoload_psr4.php files, confirming that dev entries are no longer removed.
Dependency Update: The composer/composer dependency in composer.json has been updated from a wildcard * to a specific version constraint of ^2.6.0.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

The pull request fixes an issue where dev dependencies were being incorrectly removed during autoloader generation when the target directory is the vendor directory. The changes in DumpAutoload.php are well-reasoned, and the new integration test validates the fix. The review focuses on improving test robustness and code clarity.

gemini-code-assist · 2025-06-25T23:53:00Z

tests/Issues/StraussIssue198Test.php

+        $php_string = file_get_contents($this->testsWorkingDir . 'vendor/composer/installed.json');
+        $this->assertStringContainsString("Company\\\\Project\\\\Psr\\\\Log\\\\", $php_string);
+        $this->assertStringContainsString("\"Psr\\\\SimpleCache\\\\", $php_string);
+
+        $php_string = file_get_contents($this->testsWorkingDir . 'vendor/composer/autoload_psr4.php');
+        $this->assertStringContainsString("Company\\\\Project\\\\Psr\\\\Log\\\\", $php_string);
+        $this->assertStringContainsString("Psr\\\\SimpleCache\\\\", $php_string);


The file paths for file_get_contents are constructed without a path separator. This will cause the function to look for a file in the wrong location (e.g., /path/to/workdirvendor/composer/installed.json instead of /path/to/workdir/vendor/composer/installed.json), causing the test to fail. Please add a directory separator. Using / is generally safe for cross-platform compatibility in PHP.

$php_string = file_get_contents($this->testsWorkingDir . '/vendor/composer/installed.json'); $this->assertStringContainsString("Company\\Project\\Psr\\Log\\", $php_string); $this->assertStringContainsString("\"Psr\\SimpleCache\\", $php_string); $php_string = file_get_contents($this->testsWorkingDir . '/vendor/composer/autoload_psr4.php'); $this->assertStringContainsString("Company\\Project\\Psr\\Log\\", $php_string); $this->assertStringContainsString("Psr\\SimpleCache\\", $php_string);

gemini-code-assist · 2025-06-25T23:53:01Z

tests/Issues/StraussIssue198Test.php

+
+        file_put_contents($this->testsWorkingDir . '/composer.json', $composerJsonString);
+
+        exec('composer install');


The exec('composer install') call does not check for failure. If this command fails, the test will proceed and likely fail on a later assertion with a less clear error message. It would be better to capture the exit code and assert its success immediately to make debugging easier.

exec('composer install', $composerOutput, $composerExitCode); $this->assertSame(0, $composerExitCode, "'composer install' failed. Output: \n" . implode("\n", $composerOutput));

gemini-code-assist · 2025-06-25T23:53:01Z

tests/Issues/StraussIssue198Test.php

+
+        exec('composer install');
+        $exitCode = $this->runStrauss($output);
+        assert(0 === $exitCode, $output);


The assert() language construct should generally be avoided in tests. Its behavior depends on the zend.assertions php.ini setting, and it might not be executed at all, causing a failing test to pass silently. It's more reliable and idiomatic to use PHPUnit's assertion methods, like $this->assertSame(), which also provide more descriptive failure messages.

$this->assertSame(0, $exitCode, 'Strauss execution failed. Output: ' . $output);

gemini-code-assist · 2025-06-25T23:53:01Z

src/Pipeline/Autoload/DumpAutoload.php

+        if ($this->config->getVendorDirectory() !== $this->config->getTargetDirectory()) {
+            $generator->setDevMode(false);


Consider adding a comment explaining why setDevMode(false) is called when the directories are different. This will help future developers understand the intent of this condition.

// Disable dev mode if target directory is not vendor $generator->setDevMode(false);

github-actions · 2025-06-26T01:10:03Z

strauss.phar.zip @ dd3af8d \n composer require brianhenryie/strauss:"dev-master#dd3af8db6fbf8475a93fa2dd098757af740dc4cf" --dev

BrianHenryIE added 3 commits June 25, 2025 16:41

Fix missing $ in test PHP string

c325492

Require composer/composer ^2.6.0

d8b3f90

Only run dump autoload no-dev if target is not vendor

9f74995

gemini-code-assist bot reviewed Jun 25, 2025

View reviewed changes

BrianHenryIE added 2 commits June 25, 2025 17:03

Update StraussIssue200Test.php

98c6625

Update DumpAutoloadTest.php

dd3af8d

BrianHenryIE merged commit 7ba8a8a into master Jun 26, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix vendor autoloader dev entries when target is vendor #200

Fix vendor autoloader dev entries when target is vendor #200

Uh oh!

BrianHenryIE commented Jun 25, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jun 25, 2025

Uh oh!

gemini-code-assist bot Jun 25, 2025

Uh oh!

gemini-code-assist bot Jun 25, 2025

Uh oh!

gemini-code-assist bot Jun 25, 2025

Uh oh!

github-actions bot commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!


		file_put_contents($this->testsWorkingDir . '/composer.json', $composerJsonString);

		exec('composer install');

		if ($this->config->getVendorDirectory() !== $this->config->getTargetDirectory()) {
		$generator->setDevMode(false);

Fix vendor autoloader dev entries when target is vendor #200

Fix vendor autoloader dev entries when target is vendor #200

Uh oh!

Conversation

BrianHenryIE commented Jun 25, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!