Phpinsights: [Code] Type hint declaration with classes that extends some class or implements some interface

[tarlepp]

Eg. if I have this

<?php
declare(strict_types = 1);

namespace App\Security\Voter;

use App\Entity\User;
use App\Entity\UserInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Voter;

class IsUserHimselfVoter extends Voter
{
    private const ATTRIBUTE = 'IS_USER_HIMSELF';

    /**
     * Determines if the attribute and subject are supported by this voter.
     *
     * @param string $attribute An attribute
     * @param mixed  $subject   The subject to secure, e.g. an object the user wants to access or any other PHP type
     *
     * @return bool True if the attribute and subject are supported, false otherwise
     */
    protected function supports($attribute, $subject): bool
    {
        return $attribute === self::ATTRIBUTE && $subject instanceof User;
    }

    /**
     * Perform a single access check operation on a given attribute, subject and token.
     * It is safe to assume that $attribute and $subject already passed the "supports()" method check.
     *
     * @SuppressWarnings("unused")
     *
     * @param string         $attribute
     * @param User|mixed     $subject
     * @param TokenInterface $token
     *
     * @return bool
     */
    protected function voteOnAttribute($attribute, $subject, TokenInterface $token): bool
    {
        /** @var UserInterface $user */
        $user = $token->getUser();

        return $token->isAuthenticated() && $user->getId() === $subject->getId();
    }
}

This tool is giving me following:

• [Code] Type hint declaration:
  src/Security/Voter/IsUserHimselfVoter.php:34: Method \App\Security\Voter\IsUserHimselfVoter::supports() does not have parameter type hint for its parameter $attribute but it should be possible to add it based on @param annotation "string".
  src/Security/Voter/IsUserHimselfVoter.php:51: Method \App\Security\Voter\IsUserHimselfVoter::voteOnAttribute() does not have parameter type hint for its parameter $attribute but it should be possible to add it based on @param annotation "string".

And really I cannot add those types there because that parent class Voter doesn't have those.

<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <[email protected]>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Symfony\Component\Security\Core\Authorization\Voter;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

/**
 * Voter is an abstract default implementation of a voter.
 *
 * @author Roman Marintšenko <[email protected]>
 * @author Grégoire Pineau <[email protected]>
 */
abstract class Voter implements VoterInterface
{
    /**
     * {@inheritdoc}
     */
    public function vote(TokenInterface $token, $subject, array $attributes)
    {
        // abstain vote by default in case none of the attributes are supported
        $vote = self::ACCESS_ABSTAIN;

        foreach ($attributes as $attribute) {
            if (!$this->supports($attribute, $subject)) {
                continue;
            }

            // as soon as at least one attribute is supported, default is to deny access
            $vote = self::ACCESS_DENIED;

            if ($this->voteOnAttribute($attribute, $subject, $token)) {
                // grant access as soon as at least one attribute returns a positive response
                return self::ACCESS_GRANTED;
            }
        }

        return $vote;
    }

    /**
     * Determines if the attribute and subject are supported by this voter.
     *
     * @param string $attribute An attribute
     * @param mixed  $subject   The subject to secure, e.g. an object the user wants to access or any other PHP type
     *
     * @return bool True if the attribute and subject are supported, false otherwise
     */
    abstract protected function supports($attribute, $subject);

    /**
     * Perform a single access check operation on a given attribute, subject and token.
     * It is safe to assume that $attribute and $subject already passed the "supports()" method check.
     *
     * @param string         $attribute
     * @param mixed          $subject
     * @param TokenInterface $token
     *
     * @return bool
     */
    abstract protected function voteOnAttribute($attribute, $subject, TokenInterface $token);
}

[tarlepp]

Also noticed that I'm also getting this:

• [Code] Unused parameter:
  src/Security/Voter/IsUserHimselfVoter.php:51: Unused parameter $attribute.

which is also related to this same.

[nunomaduro]

Alright, that is an issue from the util we use behinds the scenes : phpcs. There is no really solution for this at the moment.

I think the best would be waiting for this issue: https://github.com/nunomaduro/phpinsights/issues/78.

[tarlepp]

One solution could be to use easy-coding-standard.yaml file, I have used ECS quite while and it's configuration I have used following:

imports:
    - { resource: '%vendor_dir%/symplify/easy-coding-standard/config/psr2.yml' }
    - { resource: '%vendor_dir%/symplify/easy-coding-standard/config/php71.yml' }
    - { resource: '%vendor_dir%/symplify/easy-coding-standard/config/clean-code.yml' }
    - { resource: '%vendor_dir%/symplify/easy-coding-standard/config/common.yml' }

parameters:
    skip:
        SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingParameterTypeHint:
            - '*src/Entity/Traits/UserSerializer.php'
            - '*src/Repository/Traits/LoadUserByUserNameTrait.php'
            - '*src/Security/ApiKeyAuthenticator.php'
            - '*src/Security/ApiKeyUserProvider.php'
            - '*src/Security/IsUserHimselfVoter.php'
            - '*src/Security/UserProvider.php'
            - '*src/Security/SecurityUserFactory.php'

And with that I mean that users should be able to ignore specified Sniff for specified files, not just skip that sniff totally.

[nunomaduro]

@tarlepp I want something easier, such as adding something on the config file:

'ignore' => [
    '*comment with @inheritDoc', # Ignores every issue that ends with this expression.
],

[nunomaduro]

@tarlepp Do you know any alternative to regex that allows todo this?

[tarlepp]

@nunomaduro not sure what you mean with that. @inheritdoc annotation is not relevant within this issue at all.

[nunomaduro]

lol, my bad. I was just saying that this issue will be solved by ignoring errors. Nevermind.

[tarlepp]

not really solved just by ignoring all those errors - people usually want to ignore errors on specified files where they don't have any solutions to solve issues like this.

If people are ignoring that error on project wide - it's useless and basically just generates problems in long run - basically that means that just ignoring all errors you're good - while the real use case totally different one...

[nunomaduro]

I am not sure on that, I think it's fine sometimes just ignoring errors. And add the total of ignored errors on the console.

[tarlepp]

imho that will make spotting real errors a lot harder. Imagine that you've project which have eg. ~500+ files and in that only those seven (7) files has that "error". So most likely you will miss real errors.

Basically if all those Sniff classes has property ignoredFiles would make configuration of this quite easy and user friendly.

[dennislindsey]

I've been trying to figure out how I can ignore specific errors for specific files/directories because of the APIs I'm forced to implement by their inherited classes. I'm not sure how difficult it would be to accomplish, but I'd be happy using a syntax similar to what @tarlepp suggested, but pulling that into the existing config file. Maybe something like this?

...
'skip' => [
    \ObjectCalisthenics\Sniffs\Files\FunctionLengthSniff::class => [
        'app/Nova/*',
    ],
    \PHP_CodeSniffer\Standards\Generic\Sniffs\CodeAnalysis\UselessOverridingMethodSniff::class => [
        \App\Providers\EventServiceProvider::class,
        \App\Providers\NovaServiceProvider::class,
    ]
],
...

[olivernybroe]

This could be possible if we create a wrapper class for a sniff which exist if the file is in the array.
However I think using a ignoredFiles parameter is smarter as it follows the normal parameter syntax.

As far as I remember you can in your phpdoc add @phpcsSuppress SlevomatCodingStandard.TypeHints.TypeHintDeclaration.MissingParameterTypeHint and it will ignore the error for that method. This should work right now.

[olivernybroe]

@tarlepp I am not sure if this solves your problem, but we now have support for excluding a file on a specific sniff.

    'config' => [
        \SlevomatCodingStandard\Sniffs\Functions\UnusedParameterSniff::class => [
            'exclude' => [
                'src/Path/To/My/File.php',
                'src/Path/To/Other/File.php,
            ],
        ]
    ]

https://phpinsights.com/insights/#configure-insights-2