Coding-standard: UnusedUsesSniff and case mismatch

[gsdevme]

Hi,

I can see via https://github.com/slevomat/coding-standard/commit/7795645f18aebbff58ae1d0062ec6590642d0b3f there was a change to search for case-mismatch.. however Im not sure if this is a behaviour change. In the example below its flagged because exception !== Exception however.. this is a just a documentation comment about the class.. Was this meant to target types in docblocks only?

Documentation like this tends to be written in non-programming syntax, whats the thought behind this change long term?

use Exception;
use Symfony\Component\HttpFoundation\Response;

/**
* An exception that encodes a fully-formed HTTP response.
*/
class ResponseException extends Exception

Another example, a simple documentation comment written in english.

/**
 * A custom Doctrine type for personal details in a checkout.
 */
class PersonalDetailsType extends Type

[Majkl578]

I can confirm this issue, lots of false positives. For example for Doctrine's CacheFactory:

 23 | ERROR | Case of reference name cache and use statement Cache do not match.
 58 | ERROR | Case of reference name cache and use statement Cache do not match.
 58 | ERROR | Case of reference name region and use statement Region do not match.
 61 | ERROR | Case of reference name region and use statement Region do not match.
 63 | ERROR | Case of reference name cache and use statement Cache do not match.
 88 | ERROR | Case of reference name cache and use statement Cache do not match.
 88 | ERROR | Case of reference name region and use statement Region do not match.
 90 | ERROR | Case of reference name cache and use statement Cache do not match.
 92 | ERROR | Case of reference name cache and use statement Cache do not match.
 97 | ERROR | Case of reference name cache and use statement Cache do not match.
 97 | ERROR | Case of reference name region and use statement Region do not match.
 99 | ERROR | Case of reference name region and use statement Region do not match.

[soullivaneuh]

Same thing here:

FILE: /code/src/Event/AbstractHookEvent.php
-----------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------------------------------------------------------------------
 46 | ERROR | Case of reference name event and use statement Event do not match. (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
 48 | ERROR | Case of reference name event and use statement Event do not match. (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
 49 | ERROR | Case of reference name event and use statement Event do not match. (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
-----------------------------------------------------------------------------------------------------------------------------------------------------------

The pointed code part:

/**
 * Returns the correct event name according to the payload.
 *
 * It will always return an event name.
 * According event constant must be defined to make it working with subscribers.
 */

[iammattcoleman]

I'm seeing a ton of these on my tests, too, now.

It seems like it should only be checking type-hinting annotations (@param, @return, @method, etc.) in the docblock.

It shouldn't check all annotations: for example, Symfony @Route annotations should not be checked for class names.

[gsdevme]

Yeah seems we all agree a general consensus, Id imagine this is a mistake and not expected behaviour

[kukulich]

I'm sorry I hope it's fixed via https://github.com/slevomat/coding-standard/commit/c7b06b961efc2d6ec4b813acb01d557346cdf391

Please test. I'm not sure if I test all possible use cases.

The bug was caused because of relaxed regexp to support syntax like https://github.com/slevomat/coding-standard/blob/master/tests/Sniffs/Namespaces/data/unusedUsesInAnnotation.php#L24 or https://github.com/slevomat/coding-standard/blob/master/tests/Sniffs/Namespaces/data/unusedUsesInAnnotation.php#L31

[lcobucci]

@kukulich just tested and it's almost 100%.

There's a small issue with things like @Serializer\Type("uuid") when having a Uuid class imported

[kukulich]

@lcobucci Fixed https://github.com/slevomat/coding-standard/commit/39fca81221f3ef98058b0b4b964f7158ead6cc70

[Majkl578]

MismatchingCaseSensitivity seems to be fixed (no report for ORM lib/), but the fix probably introduced #280.

[lcobucci]

@kukulich now the sniff fails on a different scenario:

namespace Testing;

use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

final class Blah extends TestCase
{
    /**
     * @var Process|MockObject
     */
    private $process;

    /**
     * @before
     */
    public function configure(): void
    {
        $this->process = $this->createMock(Process::class);
    }
}

| ERROR | [x] Type PHPUnit\Framework\MockObject\MockObject is not used in this file.

[lcobucci]

It's not the MismatchingCaseSensitivity, though 😄

[kukulich]

Fucking sniff doing too much :)

[lcobucci]

Just saw @Majkl578's comment right now... I think it's safe to assume that this issue has been fixed and move on to #280

[iammattcoleman]

I'm still seeing problems with the @Route annotation. Add the following comment to tests/Sniffs/Namespaces/data/caseInsensitiveUse.php to see it in action:

/**
 * @Route("/uuid/example")
 */

Perhaps make a whitelist with only certain annotations and allow us to add/remove annotations via a setting?

Also, the recent changes broke all of my @expectedException annotations: they're all being reported as unused (SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse) now

[lcobucci]

@iammattcoleman are you configuring these annotations as useful in SlevomatCodingStandard.TypeHints.TypeHintDeclaration?

[iammattcoleman]

@lcobucci I'm not using SlevomatCodingStandard.TypeHints.TypeHintDeclaration. Prior to c7b06b9, I didn't get any SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse errors for any of the @expectedException annotations in my test classes.

[kukulich]

Ok, one more try: https://github.com/slevomat/coding-standard/commit/bfda8d10556d31a18b3c16d2a420ac30f2a09c07

[lcobucci]

100% for me, :shipit:

[iammattcoleman]

@kukulich I'm still getting incorrect results for my @Route annotations:

FILE: /Users/matt/Projects/symfonyproject/src/AppBundle/Controller/WidgetController.php
---------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
---------------------------------------------------------------------------------------------------
  25 | ERROR | Case of reference name "widget" and use statement "Widget" do not match.
     |       | (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
  52 | ERROR | Case of reference name "widget" and use statement "Widget" do not match.
     |       | (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
  81 | ERROR | Case of reference name "widget" and use statement "Widget" do not match.
     |       | (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
 116 | ERROR | Case of reference name "widget" and use statement "Widget" do not match.
     |       | (SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity)
---------------------------------------------------------------------------------------------------

Line 25 of the file in question is:

    * @Route("/widget/list", name="widget_list")

Also, I'm still getting the SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse errors for all my @expectedException annotations that started happening in commit c7b06b9.

[kukulich]

@iammattcoleman Can you please post an example of the annotation?

[iammattcoleman]

@kukulich Here are all the @Route annotations that should not count as SlevomatCodingStandard.Namespaces.UnusedUses.MismatchingCaseSensitivity errors:

    /**
     * @Route("/widget/list", name="widget_list")
     * @Route("/widget/grid", name="widget_grid")
     * @Route("/widget/show/{uuid}", name="widget_show")
     * @Route("/widget/view/{uuid}", name="widget_view")
     */

The use statement for Widget is:

use MC\SymfonyProject\AppBundle\Entity\Widget;

Here are all the @expectedAnnotation annotations that are not being detected as uses:

    /**
     * @expectedException InvalidArgumentException
     * @expectedException LengthException
     * @expectedException RuntimeException
     */

[kukulich]

@iammattcoleman I fixed the @Route bugs but I'm not able to reproduce the @exceptionException bug: https://github.com/slevomat/coding-standard/commit/9b44d3bc939b5dc54bc859911ea5822baeb45d81

[iammattcoleman]

@kukulich 9b44d3b fixed both the @Route and @expectedException annotations for me. Unfortunately, it broke @Assert annotations.

use Symfony\Component\Validator\Constraints as Assert;

    /**
     * @Validate(fields={
     *     "widgetUuid"   = @Assert\Uuid(),
     *     "clientAuthKey" = @Assert\NotBlank()
     * })
     */

[kukulich]

Fixed in https://github.com/slevomat/coding-standard/commit/ad3b191998286caa3a915390dd6455a91f039e60 and I'm done and going to ship it :)

[xificurk]

I'm still getting errors on 4.4.5 in tests that use nette/tester. A minimalistic example:

<?php
declare(strict_types = 1);

namespace Foo;

use Tester\Assert;
use Tester\TestCase;


require_once __DIR__ . '/../bootstrap.php';


/**
 * @testCase
 */
class FooTest extends TestCase
{

    public function testTrue(): void
    {
        Assert::true(true);
    }

}


(new FooTest())->run();

The fact that there is a @testCase annotation and a class named TestCase is a pure coincidence. I really don't see a reason why an annotation name and class name should be generally linked in any way. It would be nice to at least have the option to disable this behavior.

[kukulich]

@xificurk You can disable the “searchAnnotions” option.

[kukulich]

@xificurk Or you can disable the case mismatch check

[xificurk]

You can disable the “searchAnnotions” option.

Yes, but then I would get a lot of unused uses errors e.g. in Doctrine entities.

Or you can disable the case mismatch check

Yes, but I will loose this sniff for php code as well, e.g. this error:

use Foo\Bar;
class Baz extends bar {}

--
I know that it's kind of funny that for some cases I want the annotation name linked to class name, and for others not, but that's the real-world state of things. Furthermore it just worked with previous versions of slevomat/coding-standard.
A solution might be either to have the option to disable the case mismatch check selectively for annotation names only, or have a configurable list of annotation names that should be completely ignored by this sniff.

[xificurk]

Seems like the ignore list should be pretty easy to implement - I'll prepare PR.

[dg]

Another case:

<?php

use Tester\TestCase;

/**
 * @testCase
 */
class StructureTestCase extends TestCase
{

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.