Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sniff documentation more useful and more discoverable #317

Open
1 task done
przemekhernik opened this issue Jan 29, 2024 · 12 comments
Open
1 task done

Make sniff documentation more useful and more discoverable #317

przemekhernik opened this issue Jan 29, 2024 · 12 comments

Comments

@przemekhernik
Copy link
Contributor

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:

  • Review the generated docs and find the sniff codes to include/exclude them in the ruleset much faster.
  • Find more information about errors much easier by searching info by sniff code.

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)

  • I intend to create a pull request to implement this feature.
@jrfnl
Copy link
Member

jrfnl commented Jan 29, 2024

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:

  • Document the error code(s) each <standard> block covers.
    The ticket description above talks about the three part sniff codes - Stdn.Category.SniffName, but every error/warning also has a fourth part - Stnd.Category.SniffName.ErrorCode and generally speaking each <standard> block + code samples covers one or more error codes.
    As error codes can be included/excluded individually from a custom ruleset, I believe it could be useful make the error codes which are used by a sniff available via the XML Docs.
  • Document public properties which can be set on a sniff and what they do.
    These are currently documented in the wiki and that list is maintained manually and does not have any direct link with the XML docs.
  • Allow for reference URLs to be added to XML docs, either for the whole sniff and/or for individual "standards" within the docs.
    Think: links to the FIG PSR2/PSR12 specifications for the PSR2/PSR12 sniffs.
    But also, links to the PHP manual or to a PHP RFC for the PHPCompatibility sniffs, or to a company coding standards manual for company-specific sniffs, or to a CMS/Framework manual for CMS/Framework specific sniffs.
    Loosely related: Feature request: Establish a pattern of providing a link to explain a lint failure squizlabs/PHP_CodeSniffer#1603

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:

  • Automatically run the sniff over the valid/invalid code samples in the associated XML docs to verify that these still line up.
  • 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.
  • Use reflection on the sniff class to verify that all public properties in the sniff are mentioned in the docs and that no public properties are mentioned in the docs which no longer exist on a sniff.
  • Check URLs mentioned in the docs do not 404.
  • Possibly automate generating/updating the wiki page based on the XML docs.
  • For all these automation ideas, it should be considered whether the automation could also be useful for external standards with XML docs for their sniffs and if so, it should be set up to allow for them to use it too.
    As an example, PHPCSDevTools currently already contains two tools related to the XML docs - an XSD file to validate XML docs against and a "check complete" tool which can check that all sniffs are accompanied by tests and documentation.

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 🎉)

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 squizlabs/PHP_CodeSniffer#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!

@jrfnl jrfnl changed the title Include sniff codes in the docs when using --generator argument Make sniff documentation more useful and more discoverable Jan 29, 2024
@przemekhernik
Copy link
Contributor Author

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 #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 #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
Copy link
Member

jrfnl commented Jan 30, 2024

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
Copy link
Contributor Author

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
Copy link
Member

jrfnl commented Jan 30, 2024

@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
Copy link
Contributor Author

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
Copy link
Member

jrfnl commented Jan 31, 2024

@przemekhernik Nice! Can I recruit you to help with getting a website for PHPCS set up ? 😉

@fredden
Copy link
Member

fredden commented Feb 2, 2024

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
Copy link
Member

jrfnl commented Feb 2, 2024

@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 #300 (comment)
Short version: I have a commit ready for this already, but this change will need to go into 4.0.

@przemekhernik
Copy link
Contributor Author

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
Copy link
Member

jrfnl commented Feb 2, 2024

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:

  • Example command contains a typo: --colored should be --colors.
  • I'd recommend not using -n in the examples as that's hiding all warnings and new users will follow advise like that blindly and not realize they are silencing half the information.
  • I'd recommend calling the config file phpcs.xml.dist or .phpcs.xml.dist, not phpcs.xml (That way, devs can overload the project ruleset with their own version in a phpcs.xml file, which includes the dist file).
  • I'd recommend using selective ignores in the // phpcs:ignore example as a plain // phpcs:ignore will ignore the complete line, while a selective ignore // phpcs:ignore Stnd.Cat.Sniff.Errorcode will ignore only that which should be ignored and will still show other errors when the line is updated.
    Or at the very least, mention that ignores/disables etc can be made selective.
  • Also please fix the typo in my GitHub handle (in multiple places).

@stronk7
Copy link
Contributor

stronk7 commented Mar 19, 2024

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):

  • error codes.
  • configurable properties.
  • custom config settings.

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 :-)

@jrfnl jrfnl mentioned this issue Apr 6, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants