squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

PHP 8.1 support incomplete #3799

Open SharkMachine opened 1 year ago

SharkMachine commented 1 year ago

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 commented 1 year ago

3798

fredden commented 1 year ago

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.

SharkMachine commented 1 year ago

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 commented 1 year ago

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

fredden commented 1 year ago

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?

mrVrAlex commented 1 year ago

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.

fredden commented 1 year ago

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

jrfnl commented 1 year ago

@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

vladimir-borduja commented 11 months ago

@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

Fahani commented 11 months ago

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

Thank you!

jrfnl commented 11 months ago

@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

Tahiaji commented 11 months ago

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);

jrfnl commented 11 months ago

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

jrfnl commented 9 months ago

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).

SharkMachine commented 6 months ago

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