Provide rule info for warnings/errors

[timelsass]

I think a good feature to add would be when there are warnings/errors we could have some sort of indication as to what rule was actually causing the warning/error. The main reason is it's hard to reference the actual PHPCS rules while fixing issues and marking the ones that are deemed safe with the correct annotations.

[dingo-d]

That means that we would just use more verbosity? I was trying to add this but the runner settings were not working for me. But I think that it should be easy to add. We would need to refactor the output I think (based on the results we get from the JS).

[timelsass]

Ooo. yeah I haven't looked into it to be honest - it would make sense that more verbosity would output this though! Yeah we'll have to tinker around with the runner, and see what we can get out of it, and it would make sense to change up the output. I know some of the rulesets can be kinda long name-wise, so I don't know if you have given any thoughts on how we might want to present it. I was thinking the easy way would just be maybe italics and smaller font below the message so it's not too invasive, but easy to copy and paste. The other option might be adding like a copy button off to the side which on hover might say "copy annotation" - that would be kinda handy and keep the output not as cluttered. I don't know if anyone else has thoughts on something like this design wise

[dingo-d]

Yeah, when I do phpcs from the cmd I get something like this:

 106 | ERROR   | [ ] Inline comments must end in full-stops,
     |         |     exclamation marks, or question marks
     |         |     (Squiz.Commenting.InlineComment.InvalidEndChar)

The last part is something that we could output underneath. That way we know where it comes from.

Asides from that, we should do the same for every 'manual' sniff (readme, headers, tags etc.) so that we have consistency.

[joyously]

You could use the class name as an index into an array of translated messages.

[timelsass]

@joyously good call, I like that idea, @dingo-d and I had talked about some refactoring of some of the manual sniffs, which is on our radar and will in turn help keep aligned with this too. I think it will encourage us to make some of the "manual" sniffs follow a good classname/naming convention, so the output is more logical.

Right now for example we could do something like translate snake case to pascal case for the FQCN + function name and end up with: ThemeSniffer.Callback.RunSnifferCallback.ScreenshotCheck, which wouldn't fit in quite as well as: ThemeSniffer.Sniffs.Screenshot.Missing and ThemeSniffer.Sniffs.Screenshot.InvalidDimensions.

[dingo-d]

You can reffer to the https://github.com/WPTRT/WPThemeReview/issues/166 where we discussed categorization.

Ideally it would be great if we could follow the handbook. The idea is to align coding standards/sniffs with the handbook, like WPCS is doing.

[timelsass]

that makes sense do you think then instead of using ThemeSniffer\Sniffs that we move all of those up a level?

[dingo-d]

I'm fine with the messages that are coming from the Theme Sniffer have ThemeSniffer as a parent namespace for the errors coming from the plugin and not from the coding standards 🙂