Downgrade Files.IncludingNonPHPFile.IncludingSVGCSSFile to a warning with a more descriptive message

[rebeccahum]

Since SVG files are submitted for review and VIPCS runs on them, I think we can downgrade Files.IncludingNonPHPFile.IncludingSVGCSSFile to a warning level rather than an error. The main reasoning why we recommend file_get_contents() over include() is because we don't want arbitrary code evaluated and it's faster — we should indicate the "why" in the messaging.

[rebeccahum]

@jrfnl Since CSS files will be slated to be removed in PHPCS (#442), should we separate it out so SVGs are a warning and CSS remain an error? This will be a breaking error code change, however.

[jrfnl]

@rebeccahum Just checking - why was this issue closed ? And why is it (still) in the 2.3.0 milestone, while - as far as I can see - the issue was not addressed ?

The main reasoning why we recommend file_get_contents() over include() is because we don't want arbitrary code evaluated and it's faster — we should indicate the "why" in the messaging.

The first part - "we don't want arbitrary code evaluated" - I can understand. The second part - "it's faster" -, however, is unlikely.

include is a language construct and because of that will generally be faster than file_get_contents(), which is a function call.

Since CSS files will be slated to be removed in PHPCS (#442), should we separate it out so SVGs are a warning and CSS remain an error? This will be a breaking error code change, however.

PHPCS 4.x will no longer have tokenizer support for CSS files. This means that you will no longer be able to pass a .css file to PHPCS for analysis.

This sniff, however, does not target CSS files, but PHP files (containing code which includes css files). So for this sniff, nothing will change with PHPCS 4.x. It will continue to function for both CSS as well as SVG files (and all other types) as before.

It would be different if we were trying to analyse a CSS file doing something like:

@import url("../parenttheme/style.css");

[rebeccahum]

@jrfnl Ah, I should have been more transparent as to why I closed this. After an audit, we've decided that we're going to keep this at the error level since we typically do not review SVG/CSS/other non-PHP files. So, it would be good to flag that they are properly being included (and not being executed).