Add new Offloading_Files_Check check

[davidperezgar]

Related #486

This adds a new PHPCS Offloading_Files_Check check for detecting asset offloading (styles, scripts, images, etc.)

It does so by creating 2 custom PHPCS sniffs that are inspired by ones from WPCS:

• EnqueuedResourceOffloadingSniff - Detect arbitrary URLs in wp_enqueue_* calls
• OffloadingSniff - Detect arbitrary URLs in HTML strings

These new sniffs are developed according to PHP_CodeSniffer requirements in a new Composer package that is symlinked from the phpcs-sniffs folder. We should consider moving those to a new repository or contribute them to WPCS.

The sniffs need to be available in the final Plugin Check plugin, but I don't think we need to include the phpcs-sniffs folder in there. It should be included as part of vendor. Needs testing though!

To-do:

• Add PHPCS-style tests for these sniffs
• Figure out vendor situation for final plugin ZIP
• Manual testing
• Polishing
• Consider proper location for these new sniffs
• Run tests for these sniffs in CI

[davidperezgar]

I'd need some help. I don't find functions that will help to do:

1. Detect functions and their arguments even if they are multiline. Our internal scanner has a function from the parser that detects all functions from the plugin.
2. Detect if an URL is external (where to do it?)

[swissspidy]

This is the first time we're writing custom sniffs for Plugin Check so there's lots to figure out still.

[davidperezgar]

Yes, for sure we have to figure out deeply how it fits with the checks. This is a good start.

[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: davidperezgar <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: ernilambar <[email protected]>

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

[swissspidy]

Run tests for these sniffs in CI

FYI, this is not done yet.

What I meant with that to-do is that there ideally should be dedicated PHPCS-style unit tests under phpcs-sniffs/PluginCheck

We probably also need to add phpcs-sniffs to .distignore (needs testing)

[swissspidy]

This is getting closer. Unit tests now run and even code coverage works.

What's left is just manual testing. Ideally by generating a production-ready ZIP file and then testing that on a new site to see whether the sniffs are really included.

[ernilambar]

Amazing. I did not even know we could use separate composer package list this. 💗

[davidperezgar]

I've made a zip for plugin-check and run in a clean site, and I couldn't manage to work.

For test purposes, I use this plugin, and I'm afraid it does not detect offloaded.

[swissspidy]

I've made a zip for plugin-check and run in a clean site

How did you do it? Trying to reproduce.

Which means it would be an issue with the build process.

I am trying the following steps:

1. composer install --no-dev
2. wp dist-archive . /path/to/write/the/plugin-check.zip
3. Upload break-guidelines-test plugin to new site
4. Upload generated plugin-check.zip to new site
5. Run Plugin Check against break-guidelines-test in the admin

Result:

[davidperezgar]

That's how I do it! Let me test it again.

[davidperezgar]

Anyway, if you manage to work, it's ok.

[ernilambar]

I tested it in my local setup. PluginCheck.CodeAnalysis.EnqueuedResourceOffloading.OffloadedContent errors are there with that test plugin.