[Docs] Add XML doc for Squiz.Classes.ValidClassName sniff

[braindawg]

Description

This PR adds documentation for the Squiz\Classes\ValidClassName sniff.

Suggested changelog entry

Add documentation for the Squiz.Classes.ValidClassName sniff

Related issues/external references

Partial fix for #148

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.

[jrfnl]

Hi @braindawg and welcome to the PHP_CodeSniffer project! Thank you for taking on these documentation tasks.

Generally speaking, this is looking good.
Aside from the one indentation issue, the docs comply with all indent, punctuation and format requirements 👍🏻

The only thing I'm wondering about is whether saying "must be written in Pascal case" is clear enough.
Will all users understand what "Pascal case" is ? Or should the standard description be updated to include the rules which comprise Pascal case ?

Combined with the code samples, the docs now give the impression that the only requirement is that the class name starts with a capital letter, while there are more requirements.

[braindawg]

Combined with the code samples, the docs now give the impression that the only requirement is that the class name starts with a capital letter, while there are more requirements.

Fair point! I can add a Pascal_Snake_Case example to help. Not sure how far to go with bad examples, but I don't want to overdo it and add too much noise.

The only thing I'm wondering about is whether saying "must be written in Pascal case" is clear enough.
Will all users understand what "Pascal case" is ? Or should the standard description be updated to include the rules which comprise Pascal case?

Yeah, I debated how much to write out here. I wanted to be clear & concise, but I think it's safe to assume more people are familiar with the terms snake_case and camelCase than with PascalCase, and the sniff author actually has "Ensures classes are in camel caps, and the first letter is capitalised" in the file docblock.

The rules which comprise Pascal case from a human language standpoint (each word or lexical block is capitalized and all are concatenated) aren't strictly checked (or checkable) by this sniff, so I'd rather describe what will pass the sniff test. Maybe:

Class names must begin with a capital letter and contain only letters and numbers (see: PascalCase).

Thoughts?

[braindawg]

I updated the standard description to clarify what the sniff is looking for and added a new example for illegal underscores.

I didn't add any examples of actual invalid syntax, like other special characters in the filename or starting with a number. If anyone thinks that's necessary, I can create a couple, but I figured they'd be overkill.

[jrfnl]

I updated the standard description to clarify what the sniff is looking for and added a new example for illegal underscores.

@braindawg Thank you for making that update and sorry I hadn't gotten round to responding to your question before. Looks good to me.

I didn't add any examples of actual invalid syntax, like other special characters in the filename or starting with a number. If anyone thinks that's necessary, I can create a couple, but I figured they'd be overkill.

I totally agree. Examples with invalid syntax are needed in the sniff tests, but have no place in the documentation.

[jrfnl]

Approved.

FYI - the failing test is not directly related to this PR, but is to do with a test which was not stable enough. I've fixed this now in PR #853.
Once CI for that PR has finished running and that PR has been merged, I will rebase this PR to get a passing build on this one before merging.