Add documentation for the SpaceBeforeCast sniff

[rodrigoprimo]

Description

This PR adds documentation for the SpaceBeforeCast sniff.

I opted to use a single <standard> block as this sniff generates a single warning message but with different error codes. Let me know if I should use one <standard> block per error code.

Suggested changelog entry

Add documentation for the SpaceBeforeCast 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.

[dingo-d]

@jrfnl Should the docs be validated against the phpcsdocs.xsd?

[jrfnl]

@jrfnl Should the docs be validated against the phpcsdocs.xsd?

@dingo-d That's on my to-do list, but there is also a reason why I haven't enabled that yet.

As the phpcodesniffer.com domain is now in use, it would make a lot of sense to publish the docs XSD on a phpcodesniffer.com subdomain, while it currently is on a github,io subdomain.
That would also make the publication domain future-proof.

As you cannot set up redirects from a github.io domain when it changes to a custom domain, this means that change would be breaking and would require a 2.0 release of DevTools and will mean that all external standards already using the XSD will need to update the header in their XML docs (and possibly their workflows).

There are a couple of things already slated for the DevTools 2.0 release, but it's not ready yet, so either those have to move to 3.0, or I need to finish them first. I haven't decided yet.

Either way, with the above in mind, it makes more sense to me to only add the headers to the XML docs in PHPCS itself (+ add a check in CI) once that change in DevTools has been made and released.

Makes sense ?

[jrfnl]

Thanks for this PR @rodrigoprimo. Generally speaking, looking good and the file complies with the formatting guidelines for these XML docs.

I've left some comments inline for you to have a look at.

I opted to use a single block as this sniff generates a single warning message but with different error codes. Let me know if I should use one block per error code.

That seems like the right choice to me too.

[jrfnl]

As the valid code is essentially the same for both cases, I think it might be better to use a single <code_comparison> block, where the left side would have either just the one or both of the "valid" examples and the right side the two "invalid" examples.

Might also be good to vary the type casts used in the code samples a little as people could get the impression this is only about (int) type casts based on the current code samples.

[rodrigoprimo]

Even though the valid code is essentially the same for both cases, I opted for this approach as it was the only way that I could find to have titles for the two different invalid codes to highlight that this sniff acts when there is no space and also when there are multiple spaces.

I think it might be better to use a single <code_comparison> block, where the left side would have either just the one or both of the "valid" examples and the right side the two "invalid" examples.

I might be missing something, but I was not able to find a way to have more than two <code> blocks inside a <code_comparison> block. Or is your suggestion that I combine both invalid examples in a single <code> block?

Might also be good to vary the type casts used in the code samples a little as people could get the impression this is only about (int) type casts based on the current code samples.

👍

Maybe the second example could be:

        <code title="Valid: Single space before a cast operator.">
        <![CDATA[
$c = $a . <em> </em>(string) $b;
        ]]>
        </code>
        <code title="Invalid: Multiple spaces before a cast operator.">
        <![CDATA[
$c = $a .<em>   </em>(string) $b;
        ]]>
        </code>

(I'm happy to adjust the example above and include it in the first <code_comparison> block depending on what you suggest as the best approach for the other topic that we are discussing in this thread)

[jrfnl]

There should only be two code blocks per code comparison, but a code block can contain multiple code samples.

If it clarifies things, you can use inline comments in the code sample to add some additional info, like so:

        <code title="Invalid: Multiple spaces before a cast operator.">
        <![CDATA[
// Too much space.
$c = $a +<em>   </em>(int) $b;
        ]]>
        </code>

Having said that, I don't really think those comments are needed in this case as "no space" and "too much space" both translate to "not using a single space", so a good description for the "invalid" case can cover both.

what you suggest as the best approach for the other topic that we are discussing in this thread

Which other topic ?

[rodrigoprimo]

If it clarifies things, you can use inline comments in the code sample to add some additional info, like so:

It's good to know that this is an option. Thanks.

Having said that, I don't really think those comments are needed in this case as "no space" and "too much space" both translate to "not using a single space", so a good description for the "invalid" case can cover both.

I just pushed 1f90b17 in which I combined both code examples into a single <code_comparison>, changed the second code example to use (string) instead of (int) and updated the title of the invalid case to mention both no space and multiple spaces.

what you suggest as the best approach for the other topic that we are discussing in this thread

Which other topic ?

Our conversation on whether to use one or two <code_comparison> blocks. I just wanted to make it explicit that with the example that I provided, I was not implying that we should continue to use two <code_comparison> blocks.

[rodrigoprimo]

Thanks for the review, @jrfnl! I believe I replied to all the points you raised. Could you please take a second look when you get a chance?

[jrfnl]

LGTM! Thanks @rodrigoprimo !