slevomat / coding-standard

Slevomat Coding Standard for PHP_CodeSniffer provides many useful sniffs
MIT License
1.39k stars 174 forks source link

TypeNameMatchesFileName Namespace calculation is broken if having double source dir (src/src) #1249

Open c33s opened 3 years ago

c33s commented 3 years ago

the MR #987 which would fix exactly this problem was not merged. i have exactly the problem described in this MR, if you structrue you code in an additional src dir the namespace calculation is broken

path: /projects/entity-loader-bundle

├── wiki
|  └── ...
├── doc
├── entity-read-notes.md
├── src
|  ├── codeception.yml
|  ├── composer.json
|  ├── composer.lock
|  ├── composer.yaml
|  ├── config
|  ├── docs
|  ├── LICENSE
|  ├── phpstan.neon
|  ├── README.md
|  ├── RoboFile.php
|  ├── src
|  ├── tests
|  └── vendor
├── tmp
|  ├── Content
|  └── services.yaml

where i use the additional level to have for e.g. an additional tmp dir or for cloning two branches of my bundle at the same time, or for cloning the gitlab wiki repo, the namespace detection is broken.

my path is

/projects/entity-loader-bundle/src/src/C33sEntityLoaderBundle.php

where the config files resides in

/projects/entity-loader-bundle/src/

having this config (exactly like in the composer autoload section)

    <rule ref="SlevomatCodingStandard.Files.TypeNameMatchesFileName">
        <properties>
            <property name="rootNamespaces" type="array">
                <element key="src" value="C33s\Bundle\EntityLoaderBundle"/>
            </property>
        </properties>
        <exclude-pattern>/Robofile.php</exclude-pattern>
    </rule>

i get (\SlevomatCodingStandard\Sniffs\Files\TypeNameMatchesFileNameSniff):

$expectedTypeName "C33s\Bundle\EntityLoaderBundle\src\C33sEntityLoaderBundle"
$typeName "C33s\Bundle\EntityLoaderBundle\C33sEntityLoaderBundle"
kukulich commented 3 years ago

@c33s key="src/src" does not work?

c33s commented 3 years ago

@c33s key="src/src" does not work?

@kukulich i am not sure if that would make sense. only because i put the project in another src dir the ci or my colleagues won't also have it in this subdir. it is a directory out of the project dir, feels really wrong if a check tool has to know about the parent directories name. so defining it as src/src sounds like the wrong approach to me (even if it might work).

c33s commented 3 years ago

my current workaround is to patch slevomat/coding-standard with vaimo/composer-patches

add to your composer.json

"require": {
    ...
    "vaimo/composer-patches": "*"
    ...
},
"extra": {
    "patches": {
        "slevomat/coding-standard": {
            "[PATCH] Namespace Extractor see https://github.com/slevomat/coding-standard/issues/1249": {
                "source": "patches/slevomat/coding-standard/namespace.patch",
                "sha1": "912d72b0c573925818388953e8c043351d715139",
                "version": [
                    ">=7.0.12 <7.0.14"
                ]
            }
        }
    }
}

put into patches/slevomat/coding-standard/namespace.patch

--- "SlevomatCodingStandard/Sniffs/Files/FilepathNamespaceExtractor.php.org"
+++ "SlevomatCodingStandard/Sniffs/Files/FilepathNamespaceExtractor.php"
@@ -53,7 +53,7 @@ class FilepathNamespaceExtractor
        }

        /** @var string[] $pathParts */
-       $pathParts = preg_split('~[/\\\]~', $path);
+       $pathParts = preg_split('~[/\\\]~', str_replace($this->getCurrentDir(), '', $path));
        $rootNamespace = null;
        while (count($pathParts) > 0) {
            array_shift($pathParts);
@@ -85,4 +85,10 @@ class FilepathNamespaceExtractor
        return substr($typeName, 0, -strlen('.' . $extension));
    }

+   protected function getCurrentDir(): string
+   {
+       $dir = getcwd();
+
+       return $dir !== false ? $dir : '';
+   }
 }
c33s commented 2 years ago

@kukulich can you please merge #987 i keep maintaining the patch for all my projects :(

edit:

key="src/src" does not work?

hardcoding my local path would make no sense as the path would only be valid for my computer and will result in a broken pipeline as the path would be different in this case.

may i ask what is the problem of merging the bugfix? apparently there is a bug and the workaround (hardcoding scr/src) won't fix the actual problem of the broken path detection.

c33s commented 1 year ago

ping

stof commented 2 months ago

I have a similar issue when checking the coding standards in https://github.com/phpstan/phpstan-doctrine which uses such config:

    <rule ref="SlevomatCodingStandard.Files.TypeNameMatchesFileName">
        <properties>
            <property name="rootNamespaces" type="array">
                <element key="src" value="PHPStan"/>
                <element key="tests" value="PHPStan"/>
            </property>

        </properties>
    </rule>

when cloning that repo in /home/stof/src/phpstan/phpstan-doctrine, I get reports like this:

FILE: /home/stof/src/phpstan/phpstan-doctrine/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 7 | ERROR | Class name PHPStan\Type\Doctrine\Collection\IsEmptyTypeSpecifyingExtensionTest does not match filepath
   |       | /home/stof/src/phpstan/phpstan-doctrine/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php.
   |       | (SlevomatCodingStandard.Files.TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName)
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

To me, it looks like it tries to match the PHPStan top-level namespace to the phpstan folder which is a parent folder of my project root.