PHPCSStandards / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
961 stars 57 forks source link

Make sniff documentation more useful and more discoverable #317

Open przemekhernik opened 9 months ago

przemekhernik commented 9 months ago

Is your feature request related to a problem?

Searching for proper sniffs/sniffs explanations was always a complicated thing when using phpcs, at least for me, especially as a newcomer. I've had several approaches over the years to this, without success, and now I've decided to give it a try one more.

What problems I have before digging deeper in the phpcs codebase?

  1. Building own ruleset: When I wanted to build my own ruleset I couldn't find an easy way to find the sniff codes that I should include. Some sniffs include docs, that can be generated with phpcs --standard=Generic --generator=text but the codes are not available there so when I found something useful, I had to use a little bit of reverse engineering to find out the code.

  2. Getting sniffs explanations: I know that we can get a sniff explanation (when docs exist) with phpcs --standard=Generic --generator=text --sniffs=Generic.Metrics.CyclomaticComplexity but it would be also nice to search for them easier, especially for newcomers.

When I was searching for solutions I found a few discussions with similar concerns. That's why I decided to ask for this.

Describe the solution you'd like

It would be much easier for me as a beginner if the sniff code were available in the docs generated with --generate attribute. So I could just generate the docs for specific standards, select the sniffs that I'm interested in, and take the sniff codes easily. Of course, it's not impossible to find out the code, but such a simple improvement would be nice.

I've already decided to give it a try for my internal needs and It was really useful! The changes/propositions:

Thanks to this I'm able to:

I'm aware that I might miss something crucial that would similarly solve those problems, but if not, maybe it will be useful for others. What do you think?

Screenshots

HTML

HTML

Text

Text

Markdown

markdown

Additional context (optional)

jrfnl commented 9 months ago

Thanks @przemekhernik for opening this ticket.

I understand what you are saying and can understand that this would seem like a quick fix, but I think it would be prudent to look at the docs format in a more holistic manner.

What I mean by that, is that I think the docs feature should be looked at not just for how it is available and used now, but what would be a way to make this feature more useful to end-users in general.

Some ideas I have floating around about this:

The problem with any and all of these changes, would be that any change to a sniff may cause the docs to become out of date, so this would add a maintenance and review burden and this needs some thinking about if and how this could be (partially) automated.

Some automation ideas:

Aside from the information to be contained in the docs, I also think any discussion about changes to the docs format needs to be accompanied by a discussion on improving the discoverability of sniffs.

Currently there are three PHPCS native generators: Text, HTML, Markdown. I don't know if there are other custom generators in use in external standards. It would be nice to get some insight into that.

While you clearly have discovered the Docs generator feature, I have a feeling that the majority of users don't know it exists and don't use it, so what would be a way to make this wealth of information more easily discoverable by end-users ?

One idea I've been thinking about is to add a generator which would parse the docs to a format which could be used by Jekyll based GH Pages and to then publish a website with the documentation of all sniffs, including a search feature. I can even imagine, including the docs of select external standards in this search feature as well, so one could search on "import use" and see which sniffs are available related to import use statements in various PHPCS native and external standards to make an informed choice.

Aside from the initial time investments setting this up, this again would need to be accompanied by automation to automatically update the website on a new release of PHPCS itself and on releases of any external standards for which docs are included.

However, all of this is only useful if there are docs available for all sniffs, which is why I opened issue #148 to at least try to get XML docs in place for all PHPCS native sniffs. Similar tickets are open in some external standards I'm involved with, like PHPCompatibility. (not so in PHPCSExtra as that has had a docs requirement from the start, so all sniffs already have docs :tada:)

Priority wise, changing the docs format is not high on my list at this time as there are more urgent things to address in PHPCS.

First priority regarding the docs now, is getting the docs for each sniff in place and there is still enough work to do there. After that we can look at enhancing the available information.

Having said that, I do think it would be good to get a discussion going about this topic, so that when the time is right, all ideas and opinions have been gathered and we can take informed decisions to move this forward.

As part of this ticket, I think it would also be good to gather a list of tickets related to this topic from the Squizlabs repo as I know there have been a few discussions related to this in the past. One such ticket is https://github.com/squizlabs/PHP_CodeSniffer/issues/1926

Either way, that's my two pennies for now. None of this is set in stone, these are just ideas and other ideas, either for features to add to the XML docs or for ways to make the docs more useful/discoverable are very welcome!

przemekhernik commented 9 months ago

Woah, thanks for the amazing feedback! You probably spent a lot of time on this so highly appreciated πŸ™Œ

I understand what you are saying and can understand that this would seem like a quick fix, but I think it would be prudent to look at the docs format in a more holistic manner.

I agree! It's totally worth thinking about docs in a wider range. I like the ideas you've mentioned - especially documenting error codes - and they deserve to be put (and should IMO) in the main message of the issue to not miss this πŸ™Œ

What I mean by that, is that I think the docs feature should be looked at not just for how it is available and used now, but what would be a way to make this feature more useful to end-users in general.

Priority wise, changing the docs format is not high on my list at this time as there are more urgent things to address in PHPCS.

First priority regarding the docs now, is https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/148 and there is still enough work to do there. After that we can look at enhancing the available information.

I think that we can agree together, that this topic due to its complexity will require more time to be properly handled as we wish. So if you consider my initial idea about putting the sniff codes into the docs as useful too, maybe we can think about implementing this now?

I mean, If I understand the codebase related to generating docs well (please correct me if not), after this change, we gain a small improvement in the short term which seems fine compared to a huge improvement in the long, and maybe unknown term. This change seems to add some value and doesn't take anything back. Also, it works even without updating the docs, so we gain some automation too.

Of course, it won't magically solve all the problems, but I some of them for sure. At least in my case πŸ˜… Then we can think more and more about improving the docs process in the way you mentioned Let me know what do you think!

Aside from the information to be contained in the docs, I also think any discussion about changes to the docs format needs to be accompanied by a discussion on improving the discoverability of sniffs.

I agree 🫑 As a result it would be nice to have the docs that are similarly useful as the ones from other popular liters.

However, all of this is only useful if there are docs available for all sniffs, which is why I opened issue https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/148 to at least try to get XML docs in place for all PHPCS native sniffs.

Yeah I've seen this topic and I hope that I'll be able to help there too πŸ™

As part of this ticket, I think it would also be good to gather a list of tickets related to this topic from the Squizlabs repo as I know there have been a few discussions related to this in the past.

I'll try to handle this. I've been reviewing the topics there in the last days so maybe I'll be able to find all of them easier.

jrfnl commented 9 months ago

if you consider my initial idea about putting the sniff codes into the docs as useful too, maybe we can think about implementing this now?

I rather not. Why would your "random" change get priority over changes to the docs which have been requested by others ? I think any changes need a good think about the whole thing, including what changes are needed to the XSD and how to keep things readable without too much noise.

przemekhernik commented 9 months ago

Why would your "random" change get priority over changes to the docs which have been requested by others ?

Oh, I see. I just wanted to help with this implementation so I thought that if there's a possibility to improve it a little bit, pretty easily, it would be fine to go for it to make the next step to make the tool better. But If you consider this as a random change, not a needed improvement, I understand that it's not worth discussing it at least now 🫑

Let's skip this and take care of things you suggested πŸ˜€

jrfnl commented 9 months ago

@przemekhernik Don't get me wrong, I do think it is a valuable suggestion and a valid request. I just don't think that it should be prioritized over improving the docs in a more holistic way.

przemekhernik commented 9 months ago

Thanks for saying this! I'll try to think about the things you've mentioned to make the docs better 🀝

Meantime, I've created a simple bash script that iterates through the default standards and shows sniff codes with explanations in one place. It will help me or others solve the problems I mentioned earlier and focus on internal docs improvements. That's not perfect, but at least it's easier to find specific sniffs πŸ™

It is available here if someone want to check it: https://tentyp.dev/library/php/phpcs-cheatset/ πŸ‘‹

jrfnl commented 9 months ago

@przemekhernik Nice! Can I recruit you to help with getting a website for PHPCS set up ? πŸ˜‰

fredden commented 9 months ago

Scan the sniff code for all addError()/addWarning() calls, gather the error codes/error code patterns and verify against error codes mentioned in the docs.

I've had a look into this and can see how this can be achieved within the current test-suite, without too much difficulty. The main issue is that we currently have no way of knowing which error codes are covered by the documentation. When we have this information available in the XSD, we can add this functionality if we still want it.

While looking into this, I noticed that it would be trivial to add a warning if there is no XML documentation file for a given sniff, and if there are fixable errors/warnings, but no .fixed file. @jrfnl are you open to two pull requests adding each of these warnings to the existing test suite?

jrfnl commented 9 months ago

@fredden

I've had a look into this and can see how this can be achieved within the current test-suite, without too much difficulty. The main issue is that we currently have no way of knowing which error codes are covered by the documentation. When we have this information available in the XSD, we can add this functionality if we still want it.

Nice! I would prefer a check separate from the test suite though, as while it is about QA, it is not a test. I imagine it could be a tool to be added to PHPCSDevTools.

While looking into this, I noticed that it would be trivial to add a warning if there is no XML documentation file for a given sniff

That is also something which should not be part of the test suite and there is already a separate tool available for this, which will be added and turned on once the docs are complete: https://github.com/PHPCSStandards/PHPCSDevTools#checking-whether-all-sniffs-in-a-phpcs-standard-are-feature-complete

and if there are fixable errors/warnings, but no .fixed file.

See issue #299 + #300 and in particular https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/300#issuecomment-1905144488 Short version: I have a commit ready for this already, but this change will need to go into 4.0.

przemekhernik commented 9 months ago

Nice! Can I recruit you to help with getting a website for PHPCS set up ? πŸ˜‰

We need to discuss it with my wife and toddler 🀣 But sure, I can try to help in this area. But at first, I'll try to think about what can we do to have all the docs in place.

Currently, I'm working on a video material related to using phpcs in the development process. There were tons of such materials already, but I wanted to focus on the things that were less covered in them like the docs discoverability. I believe that maybe with this we'll be able to find more people to help with the docs. We just need about 56 sniffs to be documented to have them all πŸ˜… I hope this material will contribute to the project by finding more people who are willing to help πŸ™

I'll be recording probably next week, so I'll be pleased to hear your thoughts about the things that were hard - at least for me - the configuration. The general thoughts are here. Of course just only if you have a time!

jrfnl commented 9 months ago

The general thoughts are here. Of course just only if you have a time!

@przemekhernik I've had a quick read-through and would make the following suggestions:

stronk7 commented 8 months ago

Just coming from #148 (let's add all the missing docs), I think that there are various aspects that may be documented (basically all already named here):

And, maybe, we should consider not only having docs for the individual Sniffs, but also for their categories (namespace tree) and for the standard itself.

About how much of that can be automated, ideally being able to do it within code would be phenomenal (within phpdoc block or so). Keeping the docs manually maintained (into structured format, like current Sniff docs, or manually, like current property customisations wiki) is a real pain.

To dream is cheapβ„’ , great work everybody!

Ciao :-)