Docs: add missing XML documentation for sniffs

Question

Docs: add missing XML documentation for sniffs

Opened this issue a year ago · 18 comments

Answer 1 · 2023-12-12T14:48:59.000Z

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.

Answer 2 · 2023-12-12T15:14:52.000Z

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.

Answer 3 · 2023-12-12T15:16:56.000Z

Sounds good to me. Thanks for the additional details.

Answer 4 · 2023-12-14T15:00:31.000Z

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

Answer 5 · 2023-12-14T15:16:06.000Z

@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).

Answer 6 · 2023-12-15T07:57:06.000Z

I could take the PSR12 ones 👍🏼

Answer 7 · 2023-12-15T12:23:32.000Z

I could take the PSR12 ones 👍🏼

Moved to "Claimed" 👍🏻

Answer 8 · 2024-02-01T20:51:44.000Z

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

Answer 9 · 2024-02-02T02:25:16.000Z

@przemekhernik 👍🏻 Moved to "Claimed"

Answer 10 · 2024-02-05T14:58:25.000Z

@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?

Answer 11 · 2024-02-05T15:27:42.000Z

@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.

Answer 12 · 2024-02-05T15:32:45.000Z

@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. 😞

Answer 13 · 2024-02-19T16:07:09.000Z

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

Answer 14 · 2024-02-19T22:50:40.000Z

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

Answer 15 · 2024-03-11T09:24:48.000Z

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.

Answer 16 · 2024-03-11T18:42:28.000Z

@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.

Answer 17 · 2024-03-19T15:17:51.000Z

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

Answer 18 · 2024-03-19T15:23:56.000Z

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

Docs: add missing XML documentation for sniffs

Getting started

Action list

Blocked

To Do

Claimed/Work in Progress

Done

Want to contribute ?