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

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

Closed
bendavies opened this issue Apr 16, 2020 · 8 comments
Milestone

Comments

@bendavies
Copy link

bendavies commented Apr 16, 2020

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
Copy link

Same bug here with similar code:

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

@VasekPurchart
Copy link
Contributor

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
Copy link

                [
                    12345,
                    [0],
                    (object) [
                        'type' => 'int',
                    ],
                ],

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

williamdes added a commit to phpmyadmin/phpmyadmin that referenced this issue Apr 18, 2020
@jrfnl
Copy link
Contributor

jrfnl commented Apr 19, 2020

Related #582.

@fezfez
Copy link

fezfez commented Apr 20, 2020

same with

        yield 'testActiveOnly' => [
            static function () {
                return ['inactive' => '2'];
            },
        ];

@morozov
Copy link
Contributor

morozov commented Aug 29, 2020

This should be related to #3059 and #3060 (not a Slevomat issue per se).

@dianaarnos
Copy link

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 jrfnl added this to the 3.5.7 milestone Jul 5, 2024
@jrfnl
Copy link
Contributor

jrfnl commented Jul 5, 2024

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.

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

No branches or pull requests

8 participants