Generic/LanguageConstructSpacing: add XML documentation

[rodrigoprimo]

Description

This PR adds documentation for the Generic.WhiteSpace.LanguageConstructSpacing sniff.

Despite the sniff using multiple error messages, I used a single <code_comparison> block as the valid code examples are essentially the same (per the conversation here #159 (comment)).

I opted to add the list of language constructs that this sniff supports to the description as I was worried some users might get the impression that this sniff works for all language constructs, which is not the case.

On a side note (I'm happy to move this to its own issue if that is preferable), I wonder if this sniff should remove spaces after language constructs without content? So, for example, should this sniff be triggered when it finds return ; and fix it to return;?

Suggested changelog entry

Add XML documentation for the Generic.WhiteSpace.LanguageConstructSpacing sniff

Related issues/external references

Part of #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]

@rodrigoprimo Thanks for this PR. The basic formatting and the code comparison all looks good.

Despite the sniff using multiple error messages, I used a single <code_comparison> block as the valid code examples are essentially the same (per the conversation here #159 (comment)).

Agreed on the last two messages, but the sniff also checks the spacing between the yield and from keywords for a yield from and that is not addressed in the docs now.

I opted to add the list of language constructs that this sniff supports to the description as I was worried some users might get the impression that this sniff works for all language constructs, which is not the case.

Keeping the list in the docs in sync with the token list in the sniff feels very error prone to me. It might be better to update the description to make the same distinction as the sniff does: only language constructs which can be used without parentheses.

I'm fine with mentioning a few examples in the description, though I'm not sure if that's really needed what with the code samples basically already showing examples anyway.

On a side note (I'm happy to move this to its own issue if that is preferable), I wonder if this sniff should remove spaces after language constructs without content? So, for example, should this sniff be triggered when it finds return ; and fix it to return;?

Good observation and the answer is: no, it should not. There are other sniffs to deal with the spacing before a semicolon and the chance of introducing fixer conflicts by adding such a check to this sniff too is not worth it.

[rodrigoprimo]

Thanks for the detailed review, @jrfnl!

Agreed on the last two messages, but the sniff also checks the spacing between the yield and from keywords for a yield from and that is not addressed in the docs now.

I pushed a new commit that adds a <standard> and a <code_comparison> block for the error that checks for a space between yield and from. I had missed it.

Keeping the list in the docs in sync with the token list in the sniff feels very error prone to me. It might be better to update the description to make the same distinction as the sniff does: only language constructs which can be used without parentheses.

I went with a list of the supported language constructs because I was missing a way to describe them. I agree that your suggestion is much better, and I updated the description.

Good observation and the answer is: no, it should not. There are other sniffs to deal with the spacing before a semicolon and the chance of introducing fixer conflicts by adding such a check to this sniff too is not worth it.

👍

This PR is ready for another review.

[jrfnl]

Thanks for updating this PR @rodrigoprimo! Nearly there, just a grammatical fix needed and then it can be merged.

[rodrigoprimo]

I just added a new commit with the change that you suggested. Thanks!

[jrfnl]

Thanks @rodrigoprimo ! LGTM.