Make PHPStorm tag @noinspection valid

[daniil-berg]

I know it may be a long shot, but I thought I'd give it a try since the necessary change is minimal and the potential payoff (at least for me) is great...

As I am sure you know, modern IDEs have built-in inspections for all sorts of things including code style. While it is usually possible to disable specific inspections in the IDE setting globally or on a per-project basis, this is a very crude approach and not usually what you want. Most of the inspections are sensible in most situations, but every once in a while, you want to make very confined/specific exceptions. To accomplish this, IDEs sometimes provide ways to place special comments to disable inspections in a certain scope.

PHPStorm for example considers the @noinspection tag in PHPDoc blocks and has its own set of error rules/names that can be specified to make the IDE ignore specific inspections for a line, function, class, or file.

I would like to use this feature to selectively hide some "problems" from the IDE. The problem right now is that the Moodle Code Style sniffing rules do not consider @noinspection a valid tag, causing moodle-plugin-ci to fail. Therefore I humbly suggest that we add it to the array of valid tags.

Again, I know that I could just disable certain inspection for an entire project, but I think this is a bad idea because they are generally good and useful. Also just ignoring warning markers in the IDE is a bad idea because it is distracting and numbs you or reduces their perceived significance.

One very simple use case for that tag is the custom Behat steps definition file located in a plugin's tests/behat/behat_plugintype_plugingname.php. That file, the class and its methods look "unused" to the IDE because their calls are dynamically hidden behind layers of Behat magic. I also does not conform to the PSR project structure. I use the annotation @noinspection PhpIllegalPsrClassPathInspection, PhpUnused to suppress the corresponding IDE warnings for that entire class.

I realize that an argument against this might be that this is a "non-standard" tag, i.e. one that is not specified in the PHPDoc manual. But allow me to counter this:

1. PHPStorm is widely used (especially in professional contexts), which means this affects a lot of people.
2. The Moodle core already caters to PHPStorm users specifically via the .phpstorm.meta.php directory, which means there is a certain consistency to this.

Of course there are other IDEs that have their own approaches. Apparently in VSCode you can use @disregard in a similar manner. There is an argument to be made that only accepting special tags from one IDE but not another is inconsistent. I suppose that is true, but I honestly do not see an issue with including that @disregard tag as well.

In the end, it all comes down to maintaining a pleasant/productive coding experience for devs, which is what the Code Style sniffs are ultimately for. But I realize this may be a contentious proposal. I hope my arguments make sense and I would like to read your thoughts on this. For now I will make this a draft PR, in case changes or new tests will be required.

---

PS: I just noticed something by accident and I don't know, if this is intentional. If you make this an inline tag, the code sniffs don't pick it up anymore. For example this already passes the Moodle CI check:

/**
 * Foo bar.
 *
 * @package      foo_bar
 * @license      http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
 * {@noinspection PhpUnhandledExceptionInspection}
 */

Whereas using the @noinspection tag without the braces causes an error to be displayed:
ERROR | Invalid docblock tag "@noinspection". (moodle.Commenting.ValidTags.Invalid)

PHPStorm seems to be fine with this being an inline tag as well and suppresses the inspection, which means this would be an acceptable workaround for now. I hope I am not kicking the proverbial hornet's nest here with the inline tags though... 😄 If inline tags were merely an oversight, this request still stands. Curiously awaiting feedback.