Add Enqueued Resources check

[ernilambar]

Fixes #25

Note:

• I have added tests to cover my change.
• All new and existing tests pass.

[mukeshpanchal27]

Thank you for addressing the checks. I have provided some feedback for your consideration.

Regarding this matter, we are currently in the process of evaluating the acceptance criteria for this check. I don't believe it's fully prepared for engineering work yet. @felixarntz, could you please share your thoughts on this?

[felixarntz]

@ernilambar @mukeshpanchal27 I think it might be okay to merge this as it's a simple check using WPCS and it's certainly a best practice. I would however prefer to check with the plugin review team whether they're okay with it.

I think in any case it could be merged as a warning, but right now it is an error and I would like to validate whether that's okay with the review team.

The other thing is that the category of this check should IMO be a different one.

[mukeshpanchal27]

I think in any case it could be merged as a warning, but right now it is an error and I would like to validate whether that's okay with the review team.

The WPCS itself return error https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/Sniffs/WP/EnqueuedResourcesSniff.php#L65 so we should return error for this check. Please correct me if i miss anything. Thanks!

[ernilambar]

In the Acceptance Criteria in #25 it is mentioned that warnings should be displayed.

[mukeshpanchal27]

It seems the AC are not currently up to date and haven't been approved by any team member. Perhaps this was mistakenly labeled as a notice when, in fact, the sniff itself returns an error, indicating that we should display an error message.

[ernilambar]

PR has been rebased. GENERAL category is assigned for this check. Message is kept as Error.

[davidperezgar]

Hello,
This check is part of Plugins Team, so the category should be Plugin repo. In our reviews, we actually enforce users to enqueue all assets, so it should be an error.

[ernilambar]

PR has been updated and ready for review.

[davidperezgar]

Hello, I have compared Internal Scanner and this check and is not giving same results.

For example is not detecting this code:

// Add the inline footer JS into an external JS file
function inline_footer_js() {
    // Create a nonce
    $nonce = wp_create_nonce('currency_converter_nonce');
    ?>
    <script type="text/javascript">
       var ajaxurl = '<?php echo esc_url(admin_url('admin-ajax.php')); ?>';
       var currencyConverterNonce = '<?php echo esc_js($nonce); ?>';
    </script>
    <?php
}
add_action('wp_footer', 'inline_footer_js');

What we are doing in our scanner is using regex. @frantorres could help as well on this. I give you the regex that we are using:
script_src: /<script\s[^>]*?src\s*=\s*["|\']?([^>"\']*)["|\']?[^>]*?>/i

script_type: /<script\s[^>]*?type\s*=\s*["|\']?([^>"\']*(script)[^>"\']*)["|\']?[^>]*?>/i

script_only: /<script\s*>/i

link: /<link\s[^>]*?rel\s*=\s*["|\']?([^>"\']*(stylesheet)[^>"\']*)["|\']?[^>]*?>/i

link-type: /<link\s[^>]*?type\s*=\s*["|\']?([^>"\']*(text\\/css)[^>"\']*)["|\']?[^>]*?>/i

style: /<style(\s*|\s[^>]*)>/i

They were tested and working correctly. I don't know the best approach, but WPCS seems not to be the only one.

[davidperezgar]

I've reviewed with Fran, and We could suggest to WPCS to have better detection of enqueues.

[swissspidy]

Yes, contributing to WPCS would be best here 👍

Right now it only has 2 regexes for external scripts/styles, none for inline scripts etc.

Code in question: https://github.com/WordPress/WordPress-Coding-Standards/blob/d77b0204f49e4b4a2bbac791e3bbfbfe603edf06/WordPress/Sniffs/WP/EnqueuedResourcesSniff.php#L51-L81

Feedback is old and has been addressed

[davidperezgar]

All seems ok. We're going to make an issue to WPCS in order to have better detection of enqueues.