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

Docs: add missing XML documentation for sniffs #148

Open
21 of 78 tasks
jrfnl opened this issue Dec 9, 2023 · 18 comments
Open
21 of 78 tasks

Docs: add missing XML documentation for sniffs #148

jrfnl opened this issue Dec 9, 2023 · 18 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Dec 9, 2023

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

  • PSR12.Classes.AnonClassDeclaration (There is a first draft available created by @dingo-d, which can be iterated on [Documentation] PSR12 - Anonymous Class Declaration #167)
  • PSR12.Files.FileHeader (There is a first draft available created by @dingo-d, which can be iterated on [Documentation] PSR12 - File Header #231)
  • PSR12.Traits.UseDeclaration (There is a first draft available created by @dingo-d, which can be iterated on [Documentation] PSR12 - Use Declaration #239)
  • Squiz.Classes.ClassDeclaration
  • Squiz.Classes.ClassFileName
  • Squiz.Classes.ValidClassName
  • Squiz.Commenting.BlockComment
  • Squiz.Commenting.ClassComment
  • Squiz.Commenting.ClosingDeclarationComment
  • Squiz.Commenting.EmptyCatchComment
  • Squiz.Commenting.FileComment
  • Squiz.Commenting.FunctionComment
  • Squiz.Commenting.InlineComment
  • Squiz.Commenting.LongConditionClosingComment
  • Squiz.Commenting.PostStatementComment
  • Squiz.Commenting.VariableComment
  • Squiz.ControlStructures.ControlSignature
  • Squiz.ControlStructures.ElseIfDeclaration
  • Squiz.ControlStructures.InlineIfDeclaration
  • Squiz.ControlStructures.SwitchDeclaration
  • Squiz.Files.FileExtension
  • Squiz.Formatting.OperatorBracket
  • Squiz.Functions.FunctionDeclarationArgumentSpacing
  • Squiz.Functions.FunctionDuplicateArgument
  • Squiz.Functions.FunctionDeclaration
  • Squiz.Functions.GlobalFunction
  • Squiz.Functions.MultiLineFunctionDeclaration
  • Squiz.NamingConventions.ValidFunctionName
  • Squiz.NamingConventions.ValidVariableName
  • Squiz.Objects.ObjectInstantiation
  • Squiz.Operators.ComparisonOperatorUsage
  • Squiz.Operators.IncrementDecrementUsage
  • Squiz.Operators.ValidLogicalOperators
  • Squiz.PHP.CommentedOutCode
  • Squiz.PHP.DisallowBooleanStatement
  • Squiz.PHP.DisallowComparisonAssignment
  • Squiz.PHP.DisallowInlineIf
  • Squiz.PHP.DisallowMultipleAssignments
  • Squiz.PHP.DisallowSizeFunctionsInLoops
  • Squiz.PHP.DiscouragedFunctions
  • Squiz.PHP.EmbeddedPhp
  • Squiz.PHP.Eval
  • Squiz.PHP.GlobalKeyword
  • Squiz.PHP.InnerFunctions
  • Squiz.PHP.LowercasePHPFunctions
  • Squiz.PHP.NonExecutableCode
  • Squiz.Scope.MemberVarScope
  • Squiz.Scope.MethodScope
  • Squiz.Strings.ConcatenationSpacing
  • Squiz.Strings.DoubleQuoteUsage

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

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

jrfnl commented Dec 12, 2023

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

Sounds good to me. Thanks for the additional details.

@rodrigoprimo
Copy link
Contributor

rodrigoprimo commented Dec 14, 2023

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

jrfnl commented Dec 14, 2023

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

dingo-d commented Dec 15, 2023

I could take the PSR12 ones 👍🏼

@jrfnl
Copy link
Member Author

jrfnl commented Dec 15, 2023

I could take the PSR12 ones 👍🏼

Moved to "Claimed" 👍🏻

@przemekhernik
Copy link
Contributor

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

jrfnl commented Feb 2, 2024

@przemekhernik 👍🏻 Moved to "Claimed"

@przemekhernik
Copy link
Contributor

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

jrfnl commented Feb 5, 2024

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

jrfnl commented Feb 5, 2024

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

jaymcp commented Feb 19, 2024

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

jrfnl commented Feb 19, 2024

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

dingo-d commented Mar 11, 2024

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

jrfnl commented Mar 11, 2024

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

stronk7 commented Mar 19, 2024

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

jrfnl commented Mar 19, 2024

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

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

6 participants