Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.2 - DNF Support #3731

Closed
auroraeosrose opened this issue Dec 14, 2022 · 6 comments · Fixed by PHPCSStandards/PHP_CodeSniffer#461
Closed

PHP 8.2 - DNF Support #3731

auroraeosrose opened this issue Dec 14, 2022 · 6 comments · Fixed by PHPCSStandards/PHP_CodeSniffer#461

Comments

@auroraeosrose
Copy link

Describe the bug
DNF in properties and as return types creates spurious formatting errors

Code sample

<?php

declare(strict_types=1);

namespace Test;

interface A
{
    public function foo(): void;
}
interface B
{
    public function bar(): void;
}

class C implements A, B
{
    public function foo(): void
    {
        echo 'foo';
    }
    public function bar(): void
    {
        echo 'bar';
    }
}

class Dnf
{
    protected (A&B)|null $property;

    public function getProperty(): (A&B)|null
    {
        return $this->property;
    }
}

To reproduce
Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
  3. See error message displayed
FILE: test.php
-------------------------------------------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 5 LINES
-------------------------------------------------------------------------------------------------------
 11 | ERROR | [ ] Each interface must be in a file by itself
 16 | ERROR | [ ] Each interface must be in a file by itself
 28 | ERROR | [ ] Each class must be in a file by itself
 30 | ERROR | [x] Expected at least 1 space before "&"; 0 found
 30 | ERROR | [x] Expected at least 1 space after "&"; 0 found
 30 | ERROR | [x] Expected at least 1 space before "|"; 0 found
 30 | ERROR | [x] Expected at least 1 space after "|"; 0 found
 32 | ERROR | [ ] There must be a single space between the colon and type in a return type declaration
 32 | ERROR | [x] Expected at least 1 space before "&"; 0 found
 32 | ERROR | [x] Expected at least 1 space after "&"; 0 found
 32 | ERROR | [x] Expected at least 1 space before "|"; 0 found
 32 | ERROR | [x] Expected at least 1 space after "|"; 0 found
-------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------

Expected behavior
Ignoring the "each interface/class must be in a file by itself" all the rest of the errors marked are NOT errors
and definitely should not be autocorrecting

The single space between colon and type declaration doesn't apparently recognize the parentheses () as part of the type declaration

Versions (please complete the following information):

  • OS: all
  • PHP: 8.2
  • PHPCS: version 4.0.0 (alpha)
  • Standard: PSR12
@jrfnl
Copy link
Contributor

jrfnl commented Dec 14, 2022

PHPCS does not support DNF (at all) yet. PHP 8.2 support has yet to be added. Also see a number of open PRs covering bits and pieces of PHP 8.2 support (but not DNF yet).

@auroraeosrose
Copy link
Author

Well at least I'm not crazy :) Is there an 8.2 checklist issue open yet like there was for 8.1? Definitely have to change the tokenizing, probably similarly to what was done for union types then..

@auroraeosrose
Copy link
Author

//phpcs:disable PSR12.Operators.OperatorSpacing
//phpcs:disable PSR12.Functions.ReturnTypeDeclaration.SpaceBeforeReturnType

disables for anyone who finds this ticket and needs them

@jrfnl I'm going to retitle this ticket to make it easier to find

@auroraeosrose auroraeosrose changed the title DNF typehints on properties and return types confuses sniffer PHP 8.2 - DNF Support Dec 14, 2022
@jrfnl
Copy link
Contributor

jrfnl commented Dec 14, 2022

Well at least I'm not crazy :) Is there an 8.2 checklist issue open yet like there was for 8.1? Definitely have to change the tokenizing, probably similarly to what was done for union types then..

No, there's no PHP 8.2 checklist (yet), then again, for the Tokenizer, PHP 8.2 is finally a quite manageble release after three years of onslaught with new syntaxes, so I don't think a checklist is needed.

AFAICS, DNF is the only thing which necessitates changes to the Tokenizer\PHP class. Everything else just needs some tweaks to utility functions and sniffs.

A preliminary discussion about how to handle DNF for the tokenizer is in this (quite unrelated) ticket: #3667 (comment) (as that's when the build started failing).

@simPod
Copy link

simPod commented Feb 24, 2023

I'm doing this right now:

// phpcs:ignore Squiz.WhiteSpace.OperatorSpacing.NoSpaceAfter, Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore
public function foo(): (A&B)|null
{
...

@jrfnl
Copy link
Contributor

jrfnl commented Apr 29, 2024

FYI: DNF support will be included in the next PHPCS release (3.10.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants