Language injections sniff

[kagg-design]

Description

Some sniffs are triggered in specific cases that should be excluded. I'll update the list during further testing.

• PhpStorm's language injection comment /** @lang <Language> */ triggers Generic.Commenting.DocComment.MissingShort
• PhpStorm's language injection comment // language=<Language> triggers Squiz.Commenting.InlineComment.InvalidEndChar

I have added a new sniff that checks the two language injections mentioned above and blocks the raising relevant errors by another sniff. It required a lot of debugging, however. No examples on the Internet, no such cases in our code.

I have added a simple test for it, which also required a lot of debugging, as we need to use not only the sniff tested but also standard sniffs (in which we have to block raising the error).

Motivation and Context

Fixes #13.

Testing procedure

PHPUnit tests.

[ihorvorotnov]

@kagg-design FYI as a temporary workaround we ended up using // language=<Language> PhpStorm. format, which satisfies both PhpStorm and PHPCS. But it'll be great to avoid putting PhpStorm. in the end or to use the @lang annotation.

[kagg-design]

@ihorvorotnov Right now nor @lang, nor language= injections do not pass the PHPCS validation in the WPForms plugin.

The current PR solves both issues.

Also, we do not need to add PhpStorm. to the end of the // language=<lang> annotation. The existing annotation like // language=HTML PhpStorm. are also OK after the fix.

[ihorvorotnov]

@kagg-design check \WPForms_Frontend::get_captcha_inline_script for example. This is how we currently use it in the plugin:

Please note the ... PhpStorm. part. Here's the issue in the wpforms-plugin repo itself (not here) and a doc. Hope it makes sense. Not ideal, but works for now.

[kagg-design]

@ihorvorotnov I saw usages of // language=JavaScript PhpStorm. in the WPForms code.

I have mentioned above that with the current fix:

1. Existing usages are OK.
2. We can add new // language=JavaScript annotations as it is written in the current line, without annoying PhpStorm and without the dot at the end, i.e. exactly as it is recommended in the manual.

[ihorvorotnov]

@wppunk

Firstly, all our comments have a stop symbol. I would not say I like the that languages comments don't.

Those are annotations for the IDE, not comments. I don't see any reason why we should limit ourselves in this case.

Secondly, PhpStorm can't recognize language that ends with a dot. This is why we always decided to add the PhpStorm word after the language keyword/annotation. And I think we should continue work in this way.

If we can (I'm assuming we can - and that's what @kagg-design did) use annotations the way PhpStorm defines them - without a period, without useless and unnecessary PhpStorm. after the annotation, let's do them this way.

[wppunk]

Those are annotations for the IDE, not comments. I don't see any reason why we should limit ourselves in this case.

I'm about consistency. If all our comments and PHPDocs have a dot at the end of a message, why should I, as a developer, think about the difference between simple comments and comments for PhpStorm? 🤔

[kagg-design]

Those are annotations for the IDE, not comments. I don't see any reason why we should limit ourselves in this case.

I'm about consistency. If all our comments and PHPDocs have a dot at the end of a message, why should I, as a developer, think about the difference between simple comments and comments for PhpStorm? 🤔

As mentioned by @ihorvorotnov above, language injections are not comments, but annotations. Here is specified how the language injection annotation should look like.

[ihorvorotnov]

@wppunk This feels like an unnecessary bureaucracy though. There's nothing wrong with having separate standards for separate, non-related things (even though they look similar). That's why we build this PHPCS sniffs suite - to help doing the right thing where it's needed and enforce whatever standards we have by reducing the cognitive load on our brains. Let's teach PHPCS to treat annotations as annotations instead of comments and problem solved 😉

[kagg-design]

1. For the one-line comment validate:

• space before the language keyword
• the = sign after the language keyword
• PhpStorm suffix
• a dot at the end.
• etc.

We have space before language keyword, line 6 in the test fileWPForms/Tests/TestedFiles/Comments/LanguageInjection.php. We have the=sign after thelanguage` keyword. Lines 3 and 6. We won't implement the PhpStorm suffix, as it is against the specification of language annotations. We do not need a dot at the end, according to the discussion below.

[kagg-design]

2. Fro the multi-line comment

Language annotations cannot be multiline.

[ihorvorotnov]

@kagg-design

Language annotations cannot be multiline.

Does this mean we can't use something like this:

/**
 * IE11 polyfills for native `matches()` and `closest()` methods.
 *
 * @lang JavaScript
 */
$polyfills = 'if (!Element.prototype.matches) {
    // ...
';

[kagg-design]

@ihorvorotnov I did not find any other specification of the @lang xxx and // language=xxx injections except in PhpStorm docs. There is no other specification of the injections except as a single line.

[ihorvorotnov]

@kagg-design You're right, annotations in multiline comments do not work out of the box in PhpStorm. Just trying to clarify all possible use cases / preferred formats so we use them properly. Thanks for clarification! 👍🏼