Empty line is not required before block comment

[widoz]

Hi,

Don't know if this is an issue or not. I'm new on this.
I encountered the following problem with this comment block:

phpcs: Empty line is not required before block comment

/*
 * Set the post slug
 *
 * Other comments lines
 */

So I changed it to :

/* Set the post slug
 *
 * Other comments lines
 */

And received the following message

phpcs: Block comment text must start on a new line

[westonruter]

Humm. Can you share your full source file? But it sounds like this is an issue being reported by Squiz.Commenting.BlockComment, and so if it is a bug, it would be a upstream problem in PHP_CodeSniffer itself.

[widoz]

The problem doesn't come every time I write a comment block, right know on my other projects doesn't appear.

May be a latency of the editor to parse the code?

[jrfnl]

I've ran into this one a number of times before as well. What about if we change that one from an error to a warning ?
Would that be considered a solution ?

[jrfnl]

Anyone still here ? Please respond to my question above.

[Cais]

I would have started the comment block with /** versus /*.

Only a minor difference but even as is, I would not consider block comments in general to be too significant and setting this to a warning over an error would seem sensible.

PS: I've been following the repo for a while; and, decided enough being a lurker (grin); but, I still feel I need to learn a lot more before I can really start contributing further.

[GaryJones]

I would have started the comment block with /** versus /*.

Then it would become a DocBlock, not a multi-line comment block. They are different things.

https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#5-2-multi-line-comments clearly shows the clear line on the first line of the comment block, so while there might be a bug in a sniff that is un-necessarily highlighting this as an issue, I think it should stay as an error.

[Cais]

Then it would become a DocBlock, not a multi-line comment block. They are different things.

I was not aware of that difference, I use a DocBlock "format" for all multiline comments in my code and they are not triggering any sniffs that I am aware of, should they?

[GaryJones]

There's probably no direct harm in using /**, though PHPCS does treat them as having a different token. Do note though, that even the WP handbook says /** shouldn't be used for multi-line comments.

What would cause more harm in terms of PHPCS, is doing the reverse; using /* to try and formally document functions, classes, files etc.

[Cais]

Do note though, that even the WP handbook says /** shouldn't be used for multi-line comments.

I missed that ... and it took three different documents before I found the reference. Thanks for the heads-up! If anyone else needs the reference specifically, here it is: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#5-2-multi-line-comments

[GaryJones]

I linked to it in my first reply to you ;-)

[Cais]

D'oh ... well, then I missed it twice :D

[jrfnl]

OK, so I've had another look at this.

The problem with the original code sample:

/*
 * Set the post slug
 *
 * Other comments lines
 */

... is not actually shown in the code sample as it misses the wider context.

Basically, when using a block comment, the sniff expects a blank above it, unless it is the first thing in the scope.

So:

$a = 'foo';
/*
 * Block comment.
 */

... will trigger a Empty line required before block comment error, while...

if ( $foo ) {

    /*
     * Block comment.
     */
}

... will trigger a Empty line not required before block comment error.

The problem with the second code sample is something completely different:

/* Set the post slug
 *
 * Other comments lines
 */

For multi-line block comments, the sniff expects the block comment opener /* and closer */ to each be on a line by themselves, as per the original first code sample.

Those message texts could possibly do with some clarification (upstream issue), but other than that, I don't think there is anything that needs doing here.

@widoz Does that help ? Can I close this issue ?

[widoz]

@jrfnl Thanks for the explanation.