search

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
880 stars 54 forks source link

Docs: add missing XML documentation for sniffs #148

Open jrfnl opened 9 months ago

jrfnl commented 9 months ago

Sniffs in PHP_CodeSniffer should preferably be accompanied by documentation. There are currently still a number of sniffs which don't have documentation.

Sniff documentation is provided via XML files in the standard's Docs directory and is available to end-users via the command-line and/or can be compiled into an HTML or Markdown file.

To see an example of some of some available documentation, run:

phpcs --standard=PSR12 --generator=Text

Getting started

The CONTRIBUTING doc contains information on writing sniff documentation and guidelines which should be followed.

Action list

Blocked

To Do

Note: a number of sniffs will be removed in v 4.0. Those have been deliberately excluded from the above action list.

Claimed/Work in Progress

Done

Want to contribute ?

Leave a comment below to claim a sniff you'll be working on.

PRs related to this task should preferably only contain the docs for one sniff each.

rodrigoprimo commented 9 months ago

Leave a comment below to claim a sniff you'll be working on.

@jrfnl, I can help with this issue. I will start with the Generic.Formatting.SpaceBeforeCast sniff and, if all goes well, I can create PRs for more sniffs. I will make sure to create one PR per sniff.

Should we document the options that a given sniff supports? If so, how? For example, the Generic.Arrays.ArrayIndent supports indent, but I'm not sure how to document it by checking the instructions in CONTRIBUTING.md, and I was also not able to find any examples checking a few of the sniffs that are already documented.

jrfnl commented 9 months ago

Should we document the options that a given sniff supports? If so, how? For example, the Generic.Arrays.ArrayIndent supports indent, but I'm not sure how to document it by checking the instructions in CONTRIBUTING.md, and I was also not able to find any examples checking a few of the sniffs that are already documented.

Based on precedent in this repo, the documentation should outline the default behaviour of the sniff.

In docs for some external sniffs, I've sometimes mentioned that the behaviour is adjustable via properties, but I don't think that is done anywhere in this repo, so for now, I would leave it out. The wiki contains a page with details on all customisable properties anyhow.

This can be revisited at a later point in time, but for now, getting the initial version of the docs in is more important.

rodrigoprimo commented 9 months ago

Sounds good to me. Thanks for the additional details.

rodrigoprimo commented 9 months ago

@jrfnl, in the next few days, I can create PRs to add documentation to the Generic sniffs that are missing docs.

(please let me know if you prefer that I claim one sniff at a time instead of claiming a group of sniffs as I'm doing here)

jrfnl commented 9 months ago

@jrfnl, in the next few days, I can create PRs to add documentation to the Generic sniffs that are missing docs.

(please let me know if you prefer that I claim one sniff at a time instead of claiming a group of sniffs as I'm doing here)

Great! Moved those to "claimed" now. Just make sure you submit each doc as a separate PR (but you knew that already).

dingo-d commented 9 months ago

I could take the PSR12 ones 👍🏼

jrfnl commented 9 months ago

I could take the PSR12 ones 👍🏼

Moved to "Claimed" 👍🏻

przemekhernik commented 7 months ago

@jrfnl I want to learn how to create a proper sniff doc, so I can take care of Squiz.WhiteSpace.FunctionClosingBraceSpace as a first.

jrfnl commented 7 months ago

@przemekhernik 👍🏻 Moved to "Claimed"

przemekhernik commented 7 months ago

@jrfnl I have a small question about docs. Generally, I'm trying to use reverse engineering to check what specific sniff does to write docs based on the sniff code. Sometimes I'm wondering if there some better way. What is your/recommended approach here?

For example, I'm not sure how to check this part of the sniff. Also, I've get through opened issues and find out that there's a issue for removing JS/CSS tokenizers. This one seems to be JS-one so should I bother myself about this in the docs?

jrfnl commented 7 months ago

@jrfnl I have a small question about docs. Generally, I'm trying to use reverse engineering to check what specific sniff does to write docs based on the sniff code. Sometimes I'm wondering if there some better way. What is your/recommended approach here?

@przemekhernik Well, best is obviously if the docs would (have been) written when the sniff was, but for this exercise, reverse engineering is unfortunately needed.

I can imagine different people having different approaches, but I would normally run the sniff over the test case file using the code report to see what is being flagged with which error code and combine that with reading the test case file taking note of what is not being flagged too + reading the sniff code.

For this particular sniff, the command I'd use for the test case file would be like so:

phpcs -ps ./src/Standards/Squiz/Tests/WhiteSpace/FunctionClosingBraceSpaceUnitTest.inc --standard=Squiz --report=code,source --sniffs=Squiz.WhiteSpace.FunctionClosingBraceSpace

For example, I'm not sure how to check this part of the sniff. Also, I've get through opened issues and find out that there's a issue for removing JS/CSS tokenizers. This one seems to be JS-one so should I bother myself about this in the docs?

Feel free to ignore anything JS/CSS specific while writing the docs as, as you already saw, support for JS/CSS will be dropped soon enough, so just focusing on what the sniff does for PHP code should be fine.

jrfnl commented 7 months ago

@przemekhernik Just looked at the code and the tests for that sniff and it looks like you unfortunately selected a sniff which has a huge chunk of untested code..., so the tests vs code approach will not be sufficient in this case. 😞

jaymcp commented 6 months ago

I'd be happy to claim the remaining Squiz.WhiteSpace ones, apart from Squiz.WhiteSpace.PropertyLabelSpacing, as that one seems to only target JS

jrfnl commented 6 months ago

@jonmcp Thank you. Added them to the "Claimed" list (and removed the PropertyLabel one - you are 100% correct, that one shouldn't have been on the list)

dingo-d commented 6 months ago

Unfortunately I don't think I'll have much free time to finish the rest of the PSR12 documentation :(

If anybody wants to pick up what I've been working on that would be great.

jrfnl commented 6 months ago

@dingo-d Thank you for the heads-up and thank you for the work already done on this. Docs for 8 out of 12 sniffs were merged so far, so that's great progress already!

I will move the remaining four back to "Unclaimed" and will close the PRs. If/when someone claims them and wants to continue on the work you've done for these already, the PR(s) can be reopened.

stronk7 commented 5 months ago

In docs for some external sniffs, I've sometimes mentioned that the behaviour is adjustable via properties, but I don't think that is done anywhere in this repo, so for now, I would leave it out. The wiki contains a page with details on all customisable properties anyhow.

Coincidentally, today I was discussing about how to document our standard properties and config options.

It really would be nice to be able to add some "official" way to add the configurable properties to the Sniffs docs (and, in parallel, maybe, be also able to have a "main" documentation file for the standard itself, allowing custom config options (and maybe also properties, if they are standard-wide).

The above is both a declaration and a question, heh, because I've not found any way to, officially, document them (other than manual docs). So I assume it doesn't exist (neither standard or sniff level).

Ciao :-)

jrfnl commented 5 months ago

@stronk7 You may be interested in having a read through #317...