Invalid CS failure for `PSR12.Functions.NullableTypeDeclaration.UnexpectedCharactersFound`

ScopeyNZ commented 5 years ago

Affected version

3.x-dev (dev-master)

Description

With a valid piece of code I'm managing to get a PSR12.Functions.NullableTypeDeclaration.UnexpectedCharactersFound error, on a line that's not even a function declaration.

PHP File:

<?php
namespace {
    class Test
    {
        public function test()
        {
            $test = Something::one(static::CONSTANT) ?: '';
        }
    }
}

PHPCS config:

<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="test">
    <description>Test standards</description>

    <!-- base rules are PSR-12 -->
    <rule ref="PSR12" >
    </rule>
</ruleset>

Expected result: Nothing (valid) 🙂

Actual result:

gmarriott@SSguyMBP:[~/tmp/phpcs-sandbox]: phpcs test.php -s

FILE: /Users/gmarriott/tmp/phpcs-sandbox/test.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | There must not be a space between the question mark and the type in nullable type declarations
   |       | (PSR12.Functions.NullableTypeDeclaration.UnexpectedCharactersFound)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 53ms; Memory: 6MB

Notes:

It's related to the constant call. Switching to self::CONSTANT actually fixes the issue - despite the fact the code probably won't work then as the constant isn't defined. It doesn't matter if the constant is defined or not, it will fail in either case (with static).

gsherwood commented 5 years ago

Issue is with the tokenizer, which thinks the ? is a T_NULLABLE. Needs to be fixed there.

Scope map for reference:

*** START SCOPE MAP ***
Start scope map at 1:T_NAMESPACE => namespace
=> Begin scope map recursion at token 1 with depth 1
Process token 2 on line 2 []: T_WHITESPACE => ·
Process token 3 on line 2 []: T_OPEN_CURLY_BRACKET => {
=> Found scope opener for 1:T_NAMESPACE
Process token 4 on line 2 [opener:3;]: T_WHITESPACE => \n
Process token 5 on line 3 [opener:3;]: T_WHITESPACE => ····
Process token 6 on line 3 [opener:3;]: T_CLASS => class
* token is an opening condition *
* searching for opener *
    => Begin scope map recursion at token 6 with depth 2
    Process token 7 on line 3 []: T_WHITESPACE => ·
    Process token 8 on line 3 []: T_STRING => Test
    Process token 9 on line 3 []: T_WHITESPACE => \n
    Process token 10 on line 4 []: T_WHITESPACE => ····
    Process token 11 on line 4 []: T_OPEN_CURLY_BRACKET => {
    => Found scope opener for 6:T_CLASS
    Process token 12 on line 4 [opener:11;]: T_WHITESPACE => \n
    Process token 13 on line 5 [opener:11;]: T_WHITESPACE => ········
    Process token 14 on line 5 [opener:11;]: T_PUBLIC => public
    Process token 15 on line 5 [opener:11;]: T_WHITESPACE => ·
    Process token 16 on line 5 [opener:11;]: T_FUNCTION => function
    * token is an opening condition *
    * searching for opener *
        => Begin scope map recursion at token 16 with depth 3
        Process token 17 on line 5 []: T_WHITESPACE => ·
        Process token 18 on line 5 []: T_STRING => test
        Process token 19 on line 5 []: T_OPEN_PARENTHESIS => (
        Process token 20 on line 5 []: T_CLOSE_PARENTHESIS => )
        Process token 21 on line 5 []: T_WHITESPACE => \n
        Process token 22 on line 6 []: T_WHITESPACE => ········
        Process token 23 on line 6 []: T_OPEN_CURLY_BRACKET => {
        => Found scope opener for 16:T_FUNCTION
        Process token 24 on line 6 [opener:23;]: T_WHITESPACE => \n
        Process token 25 on line 7 [opener:23;]: T_WHITESPACE => ············
        Process token 26 on line 7 [opener:23;]: T_VARIABLE => $test
        Process token 27 on line 7 [opener:23;]: T_WHITESPACE => ·
        Process token 28 on line 7 [opener:23;]: T_EQUAL => =
        Process token 29 on line 7 [opener:23;]: T_WHITESPACE => ·
        Process token 30 on line 7 [opener:23;]: T_STRING => Something
        Process token 31 on line 7 [opener:23;]: T_DOUBLE_COLON => ::
        Process token 32 on line 7 [opener:23;]: T_STRING => one
        Process token 33 on line 7 [opener:23;]: T_OPEN_PARENTHESIS => (
        Process token 34 on line 7 [opener:23;]: T_STATIC => static
        Process token 35 on line 7 [opener:23;]: T_DOUBLE_COLON => ::
        Process token 36 on line 7 [opener:23;]: T_STRING => CONSTANT
        Process token 37 on line 7 [opener:23;]: T_CLOSE_PARENTHESIS => )
        Process token 38 on line 7 [opener:23;]: T_WHITESPACE => ·
        Process token 39 on line 7 [opener:23;]: T_NULLABLE => ?
        Process token 40 on line 7 [opener:23;]: T_COLON => :
        Process token 41 on line 7 [opener:23;]: T_WHITESPACE => ·
        Process token 42 on line 7 [opener:23;]: T_CONSTANT_ENCAPSED_STRING => ''
        Process token 43 on line 7 [opener:23;]: T_SEMICOLON => ;
        Process token 44 on line 7 [opener:23;]: T_WHITESPACE => \n
        Process token 45 on line 8 [opener:23;]: T_WHITESPACE => ········
        Process token 46 on line 8 [opener:23;]: T_CLOSE_CURLY_BRACKET => }
        => Found scope closer (46:T_CLOSE_CURLY_BRACKET) for 16:T_FUNCTION
    Process token 47 on line 8 [opener:11;]: T_WHITESPACE => \n
    Process token 48 on line 9 [opener:11;]: T_WHITESPACE => ····
    Process token 49 on line 9 [opener:11;]: T_CLOSE_CURLY_BRACKET => }
    => Found scope closer (49:T_CLOSE_CURLY_BRACKET) for 6:T_CLASS
Process token 50 on line 9 [opener:3;]: T_WHITESPACE => \n
Process token 51 on line 10 [opener:3;]: T_CLOSE_CURLY_BRACKET => }
=> Found scope closer (51:T_CLOSE_CURLY_BRACKET) for 1:T_NAMESPACE
*** END SCOPE MAP ***

jrfnl commented 5 years ago

@gsherwood I've got a fix ready for this, see #2553.

gsherwood commented 5 years ago

@jrfnl Thanks for fixing this. I couldn't replicate on the current stable version so I haven't added a changelog entry for it. If this is a mistake, please let me know and I'll adjust accordingly.

jrfnl commented 5 years ago

I couldn't replicate on the current stable version

Just checked, but without this fix the issue was still reproducible.

Just check out the commit before the merge, cherrypick the unit test file change and run PHPCS with -vv to see that the ? on line 65 would be incorrectly changed to T_NULLABLE. With the fix in, it no longer is.

gsherwood commented 5 years ago

Just check out the commit before the merge, cherrypick the unit test file change and run PHPCS with -vv to see that the ? on line 65 would be incorrectly changed to T_NULLABLE. With the fix in, it no longer is.

Sorry, I meant I couldn't replicate on 3.4.2. The ? tokenizes as T_INLINE_THEN for me on 3.4.2, but it was easily reproducible on master before your fix.

jrfnl commented 5 years ago

Ah, I see what you mean. Looks like the issue was introduced by 100a81ca6236dfca27f5d18628ce607dd673c0cc in that case.

gsherwood commented 5 years ago

Looks like the issue was introduced by 100a81c in that case.

That would be the one!

tehbeard commented 5 years ago

I have ran into this bug while switching to PSR12 (running 3.5.0). It seems to be related to the use of new static(...)

Console output:

PS G:\phpcs-bug> phpcs --version
PHP_CodeSniffer version 3.5.0 (stable) by Squiz (http://www.squiz.net)

PS G:\phpcs-bug> phpcs -s --standard=psr12 --report=code --report-width=120 .\Attachment.php -v
Registering sniffs in the PSR12 standard... DONE (59 sniffs registered)
Creating file list... DONE (1 files in queue)
Changing into directory G:\phpcs-bug
Processing Attachment.php [PHP => 282 tokens in 42 lines]... DONE in 10ms (2 errors, 0 warnings)

FILE: G:\phpcs-bug\Attachment.php
------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------
LINE 34: ERROR [x] There must not be a space between the question mark and the type in nullable type declarations
                   (PSR12.Functions.NullableTypeDeclaration.WhitespaceFound)
------------------------------------------------------------------------------------------------------------------------
   32:          $finfo = new \finfo(FILEINFO_MIME_TYPE);
   33:          return new static(
>> 34:              (is_null($name) ? basename($file) : $name),
   35:              is_null($type) ? $finfo->file($file) : $type,
   36:              null,
------------------------------------------------------------------------------------------------------------------------
LINE 35: ERROR [ ] There must not be a space between the question mark and the type in nullable type declarations
                   (PSR12.Functions.NullableTypeDeclaration.UnexpectedCharactersFound)
------------------------------------------------------------------------------------------------------------------------
   33:          return new static(
   34:              (is_null($name) ? basename($file) : $name),
>> 35:              is_null($type) ? $finfo->file($file) : $type,
   36:              null,
   37:              $file
------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------
Time: 278ms; Memory: 4MB

PHP file:

<?php

namespace rebar\mail\letter;

class Attachment
{
    private $name;
    private $type;
    private $contents;
    private $file;

    /**
     * Attachment constructor.
     *
     * @param $name
     * @param $type
     * @param $contents
     * @param $file
     */
    private function __construct($name, $type, $contents, $file)
    {
        $this->name = $name;
        $this->type = $type;
        $this->contents = $contents;
        $this->file = $file;
    }

    public static function fromFile($file, $name = null, $type = null)
    {
        $test = Something::one(static::CONSTANT) ?: ''; //Added to test if regression of previous test case

        $finfo = new \finfo(FILEINFO_MIME_TYPE);
        return new static(
            (is_null($name) ? basename($file) : $name),
            is_null($type) ? $finfo->file($file) : $type,
            null,
            $file
        );
    }

    public static function fromFileUsingSelf($file, $name = null, $type = null)
    {
        //Using self instead of static
        $test = Something::one(static::CONSTANT) ?: ''; //Added to test if regression of previous test case

        $finfo = new \finfo(FILEINFO_MIME_TYPE);
        return new self(
            (is_null($name) ? basename($file) : $name),
            is_null($type) ? $finfo->file($file) : $type,
            null,
            $file
        );
    }

    // rest of methods removed for test
}

squizlabs / PHP_CodeSniffer