Composer dependency setup is broken

[felixarntz]

Description

You can't do a clean composer install based on the current composer.json.

This seems to be the case due to this dependency:

"phpcompatibility/php-compatibility": "10.x-dev as 9.99.99",

Looks like that was configured as such in 9c7d4ca#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34R13. @dkotter Curious why this is needed? Seems like an unusual configuration.

Step-by-step reproduction instructions

1. If you have an existing vendor dir, delete it.
2. Delete composer.lock.
3. Run composer install.
4. It should complain about an incompatibility regarding PHPCodeSniffer.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

• Yes

Please confirm that you have tested with all plugins deactivated except AI Experiments plugin.

• Yes

Please confirm which theme type you used for testing.

• Block
• Classic
• Hybrid (e.g. classic with theme.json)
• Not sure

[dkotter]

So I ran into this same issue while trying to pull in the newer version of the WP AI Client (see notes in the PR description: #122).

Did some quick digging then and seems the phpcompatibility/php-compatibility package has had recent changes that require the use of PHP_CodeSniffer >= 4.0.1 which conflicts with the version requirements of the automattic/vipwpcs package.

Curious why this is needed? Seems like an unusual configuration.

The phpcompatibility/php-compatibility package hasn't had an official release since 2019, so in order to take advantage of all the newer PHP 8+ sniffs, you have to pull in a development version, which is what the 10.x-dev as 9.99.99 require statement is doing.

On projects I manage, I normally will do this as dev-develop as 9.99.99 (see https://docs.wpvip.com/wordpress-on-vip/php/versions/phpcs-scans/#Upcoming-releases-of-PHPCompatibility) but I copied this configuration from both the Abilities API and MCP Adapter repos for consistencies sake.

I am not a composer expert but I think we can fix the issue by including a newer version of squizlabs/php_codesniffer ourselves and aliasing that to an older version:

composer require --dev squizlabs/php_codesniffer:"4.0.1 as 3.13.4"

That fixed the issue for me but not sure if there's any downside to that approach

[felixarntz]

@dkotter I'm not sure this is needed. Did you see my PR #139? That avoids the problem entirely by not trying to use the Plugin Check plugin just for PHPCS sniffs. That in itself leads to the problem because it's a plugin and so it cannot express its dependencies properly.

Instead of including PHPCS sniffs from the plugin, I implemented a GitHub workflow that actually runs the full plugin check, so that's more reliable and it avoids the problem.

[jeffpaul]

@felixarntz wont Plugin Check duplicate the checks already happening in https://github.com/WordPress/ai/blob/develop/.github/workflows/test.yml?

[dkotter]

I'm not sure this is needed. Did you see my PR #139? That avoids the problem entirely by not trying to use the Plugin Check plugin just for PHPCS sniffs. That in itself leads to the problem because it's a plugin and so it cannot express its dependencies properly.

Instead of including PHPCS sniffs from the plugin, I implemented a GitHub workflow that actually runs the full plugin check, so that's more reliable and it avoids the problem.

No concerns from me if we want to use the GitHub Action version of Plugin Check (I've left a few comments on that PR noting this workflow was copied from both the MCP Adapter and Abilities API plugins). The concern here is if we remove the dev installation of phpcompatibility/php-compatibility, I don't think we're running PHP 8+ sniffs anymore, though correct me if I'm wrong on that

[felixarntz]

@jeffpaul

wont Plugin Check duplicate the checks already happening in https://github.com/WordPress/ai/blob/develop/.github/workflows/test.yml?

No, those workflows do not include running Plugin Check as far as I can tell.

@dkotter

The concern here is if we remove the dev installation of phpcompatibility/php-compatibility, I don't think we're running PHP 8+ sniffs anymore, though correct me if I'm wrong on that

I'm not sure, but I don't think it's a good idea to rely on a dev product in the first place. Why should we do that? I think we should maintain high quality / stablity expectations for our tooling, so I'd say if there are sniffs in there we would benefit from, we could still wait until they're out in a stable release? If other dependencies are incompatible with them because they're not stable, it's a trade-off - we have to choose one over the other, since currently it leads to the setup being broken.

[jeffpaul]

@felixarntz isn't Plugin Check effectively running a set of grouped PHPCS sniffs which would otherwise be covered in the existing PHPCS checks in the test.yml runes?

[felixarntz]

@felixarntz isn't Plugin Check effectively running a set of grouped PHPCS sniffs which would otherwise be covered in the existing PHPCS checks in the test.yml runes?

It's a lot more than that. It also has static checks that aren't PHPCS sniffs, and even checks that run certain parts of the code.

So PHPCS sniffs alone do not cover it.

[jeffpaul]

@felixarntz in that case should we remove the PHPCS checks in the test workflow file so we're not duplicating things?

[felixarntz]

@felixarntz in that case should we remove the PHPCS checks in the test workflow file so we're not duplicating things?

I don't think so? Not all regular PHPCS sniffs are also encompassed by Plugin Check.

I think it makes sense to have both. One is our own PHPCS sniffs and best practices (which may be more or less strict than what Plugin Check does), the other one is the formal Plugin Check requirements.