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.67k stars 1.48k forks source link

False Positive: Squiz.Arrays.ArrayDeclaration.ValueNoNewline: The first value in a multi-value array must be on a new line #2937

Closed bendavies closed 4 months ago

bendavies commented 4 years ago
vendor/bin/phpcs --version
PHP_CodeSniffer version 3.5.5 (stable) by Squiz (http://www.squiz.net)

Given

<?php

declare(strict_types=1);

function foo(): Generator
{
    yield 'foo' => [
        static function (): array {
            return [];
        },
    ];
}
vendor/bin/phpcs test.php --standard=Squiz --sniffs=Squiz.Arrays.ArrayDeclaration -s

FILE: test.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------
 8 | ERROR | [x] The first value in a multi-value array must be on a new line
   |       |     (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------

if static is removed from the function, there is no violation.

ondrejmirtes commented 4 years ago

Same bug here with similar code:

        $server = new Server([
            static function (ServerRequestInterface $request, callable $next) {
                return $next($request);
            },
        ]);
VasekPurchart commented 4 years ago

Also nested arrays are affected by this, I don't know if you want this in a separate issue?

<?php

// 1)
$foo = [[1,
  2,
  3,
]];

// 2)
$foo = [[
  1,
  2,
  3,
]];

// 3)
$foo = [[1 => 1,
  2 => 2,
  3 => 3,
]];

// 4)
$foo = [[
  1 => 1,
  2 => 2,
  3 => 3,
]];

// 5)
$foo = [lorem(
  1,
  2,
  3
)];

Cases 1) and 3) report errors as before 3.5.5, cases 2) and 4) were ok before (and I consider them corrected versions of 1) and 3), but now are throwing errors.

I think this is consistent with 5) which was ok both before and now.

  4 | ERROR | [x] The first value in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
  4 | ERROR | [x] The first value in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
 10 | ERROR | [x] The first value in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
 17 | ERROR | [x] The first value in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)
 17 | ERROR | [x] The first index in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.IndexNoNewline)
 23 | ERROR | [x] The first value in a multi-value array must be on a new line (Squiz.Arrays.ArrayDeclaration.ValueNoNewline)

So this affects also Squiz.Arrays.ArrayDeclaration.IndexNoNewline (not only ValueNoNewline).

williamdes commented 4 years ago
                [
                    12345,
                    [0],
                    (object) [
                        'type' => 'int',
                    ],
                ],

This is also a false positive, object is in the right place

jrfnl commented 4 years ago

Related #582.

fezfez commented 4 years ago

same with

        yield 'testActiveOnly' => [
            static function () {
                return ['inactive' => '2'];
            },
        ];
morozov commented 4 years ago

This should be related to https://github.com/squizlabs/PHP_CodeSniffer/issues/3059 and https://github.com/squizlabs/PHP_CodeSniffer/issues/3060 (not a Slevomat issue per se).

dianaarnos commented 4 years ago

Hi, I'm using phpcs version 3.5.8 and still have this problem. Code:

public function getUserInput(): array
{
    return [];
}

Result:

FOUND 2 ERRORS AFFECTING 1 LINE
-------------------------------------------------------------------------------
 97 | ERROR | [x] The first value in a multi-value array must be on a new line
 97 | ERROR | [x] Each value in a multi-line array must be on a new line
-------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY

But, if I try to use PHPCBF:

-----------------------------------------------------------------------
FILE                                                   FIXED  REMAINING
-----------------------------------------------------------------------
tests/UserExampleTest.php     FAILED TO FIX
-----------------------------------------------------------------------
A TOTAL OF -3 ERRORS WERE FIXED IN 1 FILE
-----------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
-----------------------------------------------------------------------

2 errors became -3... 🤔

jrfnl commented 4 months ago

I've just double-checked, but the original issue as reported by @bendavies and confirmed by @ondrejmirtes and @fezfez was fixed in PHPCS 3.5.7 per ticket #3060.

The issue reported by @VasekPurchart is unrelated and is more about the interpretation of what the sniff expects and not necessarily a bug.

The issue reported by @williamdes was fixed in PHPCS 3.5.7 per ticket #3059.

The issue reported by @dianaarnos is unrelated and not reproducable with the given code sample on any PHPCS version between version 3.5.5 and current master.