Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Open
c33s opened this issue Jul 16, 2021 · 8 comments

Comments

@c33s
Copy link

c33s commented Jul 16, 2021

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

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

@c33s
Copy link
Author

c33s commented Jul 21, 2021

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

c33s commented Jul 21, 2021

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

c33s commented Aug 9, 2022

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

c33s commented Dec 16, 2022

ping

@stof
Copy link

stof commented Aug 23, 2024

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.

@dotdash
Copy link

dotdash commented Dec 5, 2024

Yeah, same here with PHPStan:

FILE: /home/bsteinbrink/src/phpstan-src/issue-bot/src/Comment/BotCommentParser.php
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 11 | ERROR | Class name PHPStan\IssueBot\Comment\BotCommentParser does not match filepath /home/bsteinbrink/src/phpstan-src/issue-bot/src/Comment/BotCommentParser.php.
    |       | (SlevomatCodingStandard.Files.TypeNameMatchesFileName.NoMatchBetweenTypeNameAndFileName)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Moving it to /home/bsteinbrink/phpstan-src/ makes the errors disappear.

dotdash pushed a commit to dotdash/phpstan-src that referenced this issue Dec 5, 2024
For each root namespace, the slevomat rule considers the left-most
match of the given directory in the absolute path of the file. That is,
for /home/user/src/phpstan-src/ the root namespace PHPStan is not
assigned to /home/user/src/phpstan-src/src, but to /home/user/src, which
is obviously wrong.

The bug is known as slevomat/coding-standard#1249 for a long time, but
yet to be fixed. To avoid issues for developers of PHPStan, we can set
a basepath of "." in the PHP CodeSniffer config, which causes paths to
be evaluated relative to the current directory, avoiding
false-positives in the path leading up to the phpstan-src directory.
@dotdash
Copy link

dotdash commented Dec 5, 2024

FWIW, for PHPStan setting the basepath for phpcs to . fixes the problem for me. That way, all paths are evaluated relative to the current directory.

See phpstan/phpstan-src#3711

ondrejmirtes pushed a commit to phpstan/phpstan-src that referenced this issue Dec 5, 2024
For each root namespace, the slevomat rule considers the left-most
match of the given directory in the absolute path of the file. That is,
for /home/user/src/phpstan-src/ the root namespace PHPStan is not
assigned to /home/user/src/phpstan-src/src, but to /home/user/src, which
is obviously wrong.

The bug is known as slevomat/coding-standard#1249 for a long time, but
yet to be fixed. To avoid issues for developers of PHPStan, we can set
a basepath of "." in the PHP CodeSniffer config, which causes paths to
be evaluated relative to the current directory, avoiding
false-positives in the path leading up to the phpstan-src directory.
ondrejmirtes pushed a commit to phpstan/phpstan-src that referenced this issue Dec 5, 2024
For each root namespace, the slevomat rule considers the left-most
match of the given directory in the absolute path of the file. That is,
for /home/user/src/phpstan-src/ the root namespace PHPStan is not
assigned to /home/user/src/phpstan-src/src, but to /home/user/src, which
is obviously wrong.

The bug is known as slevomat/coding-standard#1249 for a long time, but
yet to be fixed. To avoid issues for developers of PHPStan, we can set
a basepath of "." in the PHP CodeSniffer config, which causes paths to
be evaluated relative to the current directory, avoiding
false-positives in the path leading up to the phpstan-src directory.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants