Use plugin-check GitHub workflow in favor of brittle plugin-check PHPCS checks

[felixarntz]

Fixes #138.

Relevant technical choices

• The problem in Composer dependency setup is broken #138 is caused due to some issues with the brittle plugin-check plugin inclusion just to use its PHPCS rules. That approach leads to problems because of including a plugin that itself depends on other PHPCS foundational pieces, without being able to declare that (since it's a plugin served from wpackagist, so it can't express its own dependencies properly).
• The PR replaces the setup with a proper plugin-check.yml GitHub workflow that uses wordpress/plugin-check-action to run the full Plugin Check scanner. In addition to fixing the issue, it is more comprehensive.

Test using WordPress Playground

The changes in this pull request can be previewed and tested using this WordPress Playground instance:

Click here to test this pull request.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: JasonTheAdams <jason_the_adams@git.wordpress.org>
Co-authored-by: justlevine <justlevine@git.wordpress.org>
Co-authored-by: dkotter <dkotter@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[justlevine]

@felixarntz I originally took this pattern from wordpress/performance (where it looks like @westonruter is currently battling the same issue in WordPress/performance#2257 ).

What (if any) are the practical implications of this change to detection? Should this be the defacto pattern for the Performance plugin too, all canonical plugins, or at least our other core-ai repos?

[justlevine]

I'd assume we'd just be changing this to "^10.0.0-alpha," why remove it entirely?

[dkotter]

Note there's some discussion about this on #138 but I agree. I wouldn't remove this entirely. I had recommended aliasing on #138 but I think we can also change this to phpcompatibility/php-compatibility:10.0.0-alpha2 as 9.99.99 to fix the problem

[justlevine]

AFAIK we don't even need the aliasing anymore:

This works just fine for me:

		"phpcompatibility/php-compatibility": "^10.0.0-alpha",
		"phpcompatibility/phpcompatibility-wp": "^3.0.0-alpha",

[felixarntz]

+1, I don't think we should do the aliasing, that feels quite hacky. We need to ensure our dependencies are compatible.

[dkotter]

This approach is fine by me as well, though noting it's not much different than the aliasing approach we have now (still ends up pulling in dev releases). I think the important piece here that we don't have now is the "phpcompatibility/phpcompatibility-wp": "^3.0.0-alpha", as without that I think the automattic/vipwpcs package will complain

[felixarntz]

I addressed this in 0e3d827: We should only need to include "phpcompatibility/phpcompatibility-wp" because we don't use "phpcompatibility/php-compatibility" anywhere explicitly, in other words, it's an indirect dependency - no need to specify it.

By requiring "phpcompatibility/phpcompatibility-wp": "^3.0.0-alpha", "phpcompatibility/php-compatibility": "^10.0.0-alpha" will be installed anyway.

[westonruter]

I originally took this pattern from wordpress/performance (where it looks like @westonruter is currently battling the same issue in WordPress/performance#2257 ).

Indeed! I was also thinking of switching to using the PCP action. Nevertheless, I've also opened WordPress/plugin-check#966 so that hopefully the sniffs from PCP can be more elegantly added to existing PHPCS rulesets without the redundancy of running PCP.

[felixarntz]

@justlevine

What (if any) are the practical implications of this change to detection? Should this be the defacto pattern for the Performance plugin too, all canonical plugins, or at least our other core-ai repos?

I'm not sure I understand your question, can you elaborate? "change to detection" of what?

There are two reasons I opened this PR:

• Replace brittle usage of Plugin Check PHPCS rules with actually running Plugin Check end to end.
• By doing that, address the current problem where a fresh composer install won't work.

[justlevine]

@justlevine

What (if any) are the practical implications of this change to detection?

I'm not sure I understand your question, can you elaborate? "change to detection" of what?

Sorry, yeah that was horrible phrasing.
I'm asking if the workflow detects the same/same+more/more+different/fewer smells as the internal rulesets we're removing.

(Depending on the answer, we might want to e.g. pin ~1.6.0 temporarily and fix the brittleness directly so we can still get the existing sniff coverage and be able to run it locally, and/or add PCP to our wp-env with the matching package.json WPCLI command, and/or prioritize backporting GH workflow to other repos, etc)

[felixarntz]

@justlevine

Sorry, yeah that was horrible phrasing. I'm asking if the workflow detects the same/same+more/more+different/fewer smells as the internal rulesets we're removing.

I answered something like that in #138 (comment): The PHPCS sniffs from Plugin Check are only a subset of what Plugin Check checks.

(Depending on the answer, we might want to e.g. pin ~1.6.0 temporarily and fix the brittleness directly so we can still get the existing sniff coverage and be able to run it locally, and/or add PCP to our wp-env with the matching package.json WPCLI command, and/or prioritize backporting GH workflow to other repos, etc)

I think including sniffs from Plugin Check is always going to be problematic because it's a plugin, not a proper package with composer.json etc, particularly not when you include it via wpackagist. Setting up PCP in wp-env may be a worthwhile effort, but I think that could be explored separately. It's most important to check this in CI, so unless any of us has previous experience in setting up PCP alongside the main plugin in wp-env for easy local usage, I'd suggest we make this a separate issue/PR to not block this.

[justlevine]

Once plugin-check-action is pinned, LGTM 🚀