DisallowShortOpenTag: throw warning for potential short open tags in inline HTML #1400
Conversation
…inline HTML When the ini setting `short_open_tag` is `off` on the machine running PHPCS, short open tags would not be recognized in `T_INLINE_HTML` tokens. This could lead to undesirable consequences if the code would later be deployed on a machine where `short_open_tag` is set to `on`. This PR fixes that and will throw a warning if a use of short open tags is detected in inline HTML. Note: the `getSnippet()` method is a 1-on-1 copy of the one used in the `DisallowAlternativePHPTags` sniff. It would be better if that method would be placed in another file and just called by both sniff, but I wasn't sure which class would be most suited to place the method in.
jrfnl
force-pushed
the
feature/short-open-tags-in-inline-html
branch
from
4696465 to
9f77036
Compare
|
The tests are failing on the .3 file because the error lists shows lines 13, 16, 21 with warnings and there aren't even 21 lines in the file. I'm going to change that to what I get, which is the I'm also changing the message just a little, and changing the DisallowAlternativeTags messages to match that change. I hadn't noticed it before - obviously very old code :) Edit: the .1 file is also failing when short_open_tag is off, but that's because your new warnings are being generated instead of the old errors. So I'm also flipping those around based on the ini setting. |
Oh darn, sorry about that, I've moved the unit tests around a couple of times and clearly forgot to change the line numbers the last time.
grin please change it to your liking, the other one came from me as well. Just happy to be able to contribute so these are available to the wider public 😍 |
Oh it's no problem. Makes me understand the change better as well. Just wanted to document why I changed all this stuff so it's not confusing. I've committed the test changes now and they appear to be working ok. Thanks for another great contribution. |
|
👍 |
* Document the setting of the PHP ini value in the travis build script. * Remove the `short_open_tag_enabled` property as the value of this is only used once, so the check can just as efficiently be done in the appropriate place. * Adjust the regex delimiter to be backticks. * Pass `$data` as an array as that's expected by PHPCS. * Remove the throwing of a warning when short open echo tags _"might"_ be found. This warning was also added to the upstream `DisallowShortOpenTag` sniff in PHPCS 2.9.0 and would therefore cause duplicate warnings. See squizlabs/PHP_CodeSniffer/pull/1400. * Adjust the unit test line numbers (merge conflict artefact) Also: adds one missing `@since` tag which is unrelated to this PR, but in the same file and make one very long line a little more readable.
* Document the setting of the PHP ini value in the travis build script. * Set the branch this is tested with to `2.9` as PHPCS 3.x has come out since the original PR was pulled and is now `master`. * Remove the `short_open_tag_enabled` property as the value of this is only used once, so the check can just as efficiently be done in the appropriate place. * Adjust the regex delimiter to be backticks. * Pass `$data` as an array as that's expected by PHPCS. * Remove the throwing of a warning when short open echo tags _"might"_ be found. This warning was also added to the upstream `DisallowShortOpenTag` sniff in PHPCS 2.9.0 and would therefore cause duplicate warnings. See squizlabs/PHP_CodeSniffer/pull/1400. * Adjust the unit test line numbers (merge conflict artefact) Also: adds one missing `@since` tag which is unrelated to this PR, but in the same file and make one very long line a little more readable.
When the ini setting
short_open_tagisoffon the machine running PHPCS, short open tags would not be recognized inT_INLINE_HTMLtokens.This could lead to undesirable consequences if the code would later be deployed on a machine where
short_open_tagis set toon.This PR fixes that and will throw a warning if a use of short open tags is detected in inline HTML.
Fixes #1398
Note: the
getSnippet()method is a 1-on-1 copy of the one used in theDisallowAlternativePHPTagssniff.It would be better if that method would be placed in another file and just called by both sniffs, but I wasn't sure which class would be most suited to place the method in.