PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

PHP 8.1 support incomplete

Open SharkMachine opened this issue 1 year ago • 15 comments

Code sample

<?php

/**
 * @param One&Two $param
 * @return void
 */
function sigh(One&Two $param): void
{
    // Do you really support PHP 8.1
}

To reproduce Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
 4 | ERROR | [x] Tag value for @param tag indented incorrectly; expected 2 spaces but found 1
   |       |     (Generic.Commenting.DocComment.TagValueIndent)

Expected behavior

No error

Versions (please complete the following information):

  • OS: Ubuntu 22.04
  • PHP: 8.1
  • PHPCS: 3.7.2
  • Standard: You can see it from above

SharkMachine avatar Apr 15 '23 03:04 SharkMachine

#3798

SharkMachine avatar Apr 15 '23 03:04 SharkMachine

If you add a blank* line between lines 4 & 5 in your sample, do you still get an error? I expect not. Do you get the same error if the type is not a union type? I expect so.

This seems like an alignment issue, not a (lack of) PHP version support.

* Not an completely blank line, as this is in a docblock, so you will need the correct indentation and an asterisk, but nothing else.

fredden avatar Apr 15 '23 17:04 fredden

If you add a blank* line between lines 4 & 5 in your sample, do you still get an error? I expect not. Do you get the same error if the type is not a union type? I expect so.

This seems like an alignment issue, not a (lack of) PHP version support.

  • Not an completely blank line, as this is in a docblock, so you will need the correct indentation and an asterisk, but nothing else.

PHPCS doesn't understand the required spacing when using intersection types.

SharkMachine avatar Apr 21 '23 20:04 SharkMachine

@fredden Instead of looking at my problem and help me to solve it, you wanted to belittle me.

SharkMachine avatar Apr 21 '23 20:04 SharkMachine

I have now been able to find time to answer my own questions/suggestions. (I was away from my machines at the time, and I wanted to get you a quick response to your support query.)

If you add a blank* line between lines 4 & 5 in your sample, do you still get an error?

The following code snippet does not show any error for me from Generic.Commenting.DocComment.TagValueIndent. This suggests that the tool (and the sniff in question) handles this code just fine.

<?php

/**
 * @param One&Two $param
 *
 * @return void
 */
function doThing(One&Two $param): void
{
}

Do you get the same error if the type is not a union type?

The following code snippet does get a complaint from Generic.Commenting.DocComment.TagValueIndent on line 4. This suggests that the (union) type is not relevant in this case.

<?php

/**
 * @param bool $param
 * @return void
 */
function doThing(bool $param): void
{
}

Instead of looking at my problem and help me to solve it, you wanted to belittle me.

This is not true. Perhaps I was influenced by the language that you used in the bug report. There was no malice in my intention. I thought I'd provided some suggestions to triage/refine the bug report.

From the information provided, I don't understand what the problem is that you're trying to report. @SharkMachine, please can you provide some more details to help me understand why this is a PHP 8.1/union type support problem? Is there another code sample that demonstrates the issue you're facing?

fredden avatar Apr 24 '23 13:04 fredden

Another case (+ PHP 8.2)

<?php

declare(strict_types=1);

namespace Test;

/**
 * Bla bla bla
 */
readonly class Example
{
    public function test() : void
    {
        echo '123';
    }
}

Show

FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [ ] The file-level docblock must follow the opening PHP tag in the file header
   |       |     (PSR12.Files.FileHeader.IncorrectOrder)
 9 | ERROR | [x] Header blocks must be separated by a single blank line (PSR12.Files.FileHeader.SpacingAfterBlock)
------------------------------------------------------------------------------------------------------------------------------------

If remove readonly OR put final - all will work.

mrVrAlex avatar May 03 '23 11:05 mrVrAlex

@mrVrAlex that sounds like https://github.com/squizlabs/PHP_CodeSniffer/issues/3727 and not what @SharkMachine was trying to highlight. Can you confirm?

fredden avatar May 03 '23 11:05 fredden

@mrVrAlex I agree with @fredden, your issue is unrelated to the one being discussed here and belongs with issue #3727. Either way, I have pulled a fix now in https://github.com/squizlabs/PHP_CodeSniffer/pull/3816

jrfnl avatar May 03 '23 12:05 jrfnl

@jrfnl Apologies, that bother you. But could you please tell us when is the next release that will include these modifications?

PS: PHP8.1 exited in November 2021, my team has been using it for more than 1 year, and we are still avoiding using read-only classes because our pipelines run code sniffer

vladimir-borduja avatar Oct 10 '23 08:10 vladimir-borduja

@jrfnl we are facing the same issue as @vladimir-borduja, any news about the next release?

Thank you!

Fahani avatar Oct 15 '23 16:10 Fahani

@jrfnl we are facing the same issue as @vladimir-borduja, any news about the next release?

@vladimir-borduja @Fahani Unfortunately no, we all (including me), will just have to wait until @gsherwood has some availability again... Also see #3861

jrfnl avatar Oct 15 '23 21:10 jrfnl

Another case (+ PHP 8.2)

<?php

declare(strict_types=1);

namespace Test;

/**
 * Bla bla bla
 */
readonly class Example
{
    public function test() : void
    {
        echo '123';
    }
}

Show

FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | [ ] The file-level docblock must follow the opening PHP tag in the file header
   |       |     (PSR12.Files.FileHeader.IncorrectOrder)
 9 | ERROR | [x] Header blocks must be separated by a single blank line (PSR12.Files.FileHeader.SpacingAfterBlock)
------------------------------------------------------------------------------------------------------------------------------------

If remove readonly OR put final - all will work.

Another slightly weird way to fix this is to add a docblock between <?php and declare(strict_types=1);

Tahiaji avatar Oct 16 '23 06:10 Tahiaji

@Tahiaji That case has already been fixed in master via #3816

jrfnl avatar Oct 16 '23 06:10 jrfnl

FYI: A fix for the issue reported by @mrVrAlex (PSR12/FileHeader sniff) is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).

jrfnl avatar Dec 08 '23 23:12 jrfnl

This still being open doesn't look good for https://wiki.php.net/rfc/dnf_types :<

SharkMachine avatar Mar 03 '24 07:03 SharkMachine