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

PSR12.Files.FileHeader: false positive when there is no header #2997

Closed
jrfnl opened this issue Jun 22, 2020 · 6 comments
Closed

PSR12.Files.FileHeader: false positive when there is no header #2997

jrfnl opened this issue Jun 22, 2020 · 6 comments

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Jun 22, 2020

Given the following code run against the PSR12 standard:

<?php

/*
 * Not a file docblock.
 */

/**
 * Docblock for an include statement.
 */
include 'foo.php';

I get the following error:

--------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------
 9 | ERROR | [x] Header blocks must be separated by a single blank line
   |       |     (PSR12.Files.FileHeader.SpacingAfterBlock)

... while I expected none.

@gsherwood
Copy link
Member

This is because the docblock is ambiguous. The sniff can't tell the difference between:

<?php

/**
 * Docblock for an include statement.
 */
include 'foo.php';

and

<?php

/**
 * Docblock for the file.
 */
include 'foo.php';

So it sees the docblock and reports an error for no blank line after it. If it looked at the next token and tried to guess that this is a docblock for an include statement, it wouldn't error for the second code sample above.

I don't have a solution for this.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jun 23, 2020

@gsherwood Even thought still in draft status, PSR-5 states that an include/require statement is a structural element which can get a docblock, so for your second example, the docblock should not be regarded as the file docblock either, just like it isn't for:

<?php

/**
 * File docblock.
 */
class Foo {}

Also: why doesn't having the non-Docblock multi-line comment above the Docblock in the original example make a difference ? Shouldn't that signal to the sniff already that everything aside from the PHP open tag is missing ?

@gsherwood
Copy link
Member

PSR-5 states that an include/require statement is a structural element which can get a docblock

A lot of things can have a docblock, but it's impossible to know if they actually relate to that element. It's just a hard problem where you can post code that should obviously be ignored and I can post code that obviously shouldn't be. It's the same with detecting file comments vs OO structure comments. It's a real pain and whatever I do I'm going to generate false positives.

Also: why doesn't having the non-Docblock multi-line comment above the Docblock in the original example make a difference ? Shouldn't that signal to the sniff already that everything aside from the PHP open tag is missing ?

Because PSR12 specifically says nothing about standard comments in file headers, I have to ignore them. Any time I've not done so for PSR2, I've had developers complaining that the standard doesn't define commenting standards and they can do whatever they want. Which is true, but annoying.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jun 24, 2020

@gsherwood I hear you. Let me think on this for a while.

@Xerkus
Copy link

Xerkus commented Nov 30, 2020

Looks like I am hitting same issue, not as ambiguous however: laminas/laminas-coding-standard#56

With code sniffer 3.5.8 I get

The file-level docblock must follow the opening PHP tag in the file header (PSR12.Files.FileHeader.IncorrectOrder)
Header blocks must be separated by a single blank line (PSR12.Files.FileHeader.SpacingAfterBlock)

When run on files like:
https://github.com/mezzio/mezzio-skeleton/blob/c268aac1a272ee20b9bceaa5715aac4174d388ae/src/MezzioInstaller/Resources/config/routes-fastroute-minimal.php#L9-L26

<?php

declare(strict_types=1);

use Mezzio\Application;
use Mezzio\MiddlewareFactory;
use Psr\Container\ContainerInterface;

/**
 * FastRoute route configuration
 *
 * @see https://github.com/nikic/FastRoute
 *
 * Setup routes with a single request method:
 *
 * $app->get('/', App\Handler\HomePageHandler::class, 'home');
 * $app->post('/album', App\Handler\AlbumCreateHandler::class, 'album.create');
 * $app->put('/album/{id:\d+}', App\Handler\AlbumUpdateHandler::class, 'album.put');
 * $app->patch('/album/{id:\d+}', App\Handler\AlbumUpdateHandler::class, 'album.patch');
 * $app->delete('/album/{id:\d+}', App\Handler\AlbumDeleteHandler::class, 'album.delete');
 *
 * Or with multiple request methods:
 *
 * $app->route('/contact', App\Handler\ContactHandler::class, ['GET', 'POST', ...], 'contact');
 */
return static function (Application $app, MiddlewareFactory $factory, ContainerInterface $container): void {
};

Docblock belongs to closure here and not file level

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 2, 2023

Closing as replaced by PHPCSStandards/PHP_CodeSniffer#21

@jrfnl jrfnl closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2023
@github-project-automation github-project-automation bot moved this to Ready for Release in PHPCS v3 Development Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Release
Development

No branches or pull requests

3 participants