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

Detect other PSR violations where namespace is missing or not a subnamespace of configured one #8

Merged
merged 5 commits into from
May 31, 2024

Conversation

PrinsFrank
Copy link
Contributor

@PrinsFrank PrinsFrank commented Apr 30, 2024

Fixes composer/composer#11957

To get warnings about missing namespaces or non-common namespaces in Composer when running with --strict-psr, classes shouldn't be skipped before checking their namespace if they don't have a common root.

@PrinsFrank PrinsFrank force-pushed the add-missing-strict-psr-violations branch 2 times, most recently from d1fb66b to 3ee697c Compare April 30, 2024 11:45
@PrinsFrank PrinsFrank force-pushed the add-missing-strict-psr-violations branch from 3ee697c to 9956a25 Compare April 30, 2024 11:46
…ilently passing if valid classes are found, will only show up when requesting PSR violations)
@PrinsFrank PrinsFrank marked this pull request as ready for review April 30, 2024 11:59
Copy link

@szepeviktor szepeviktor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No silent ignoring 👍🏻

@Seldaek Seldaek merged commit a05a230 into composer:main May 31, 2024
9 checks passed
@Seldaek
Copy link
Member

Seldaek commented May 31, 2024

Thanks, I think it makes sense but I am a bit worried about unforeseen side-effects here.. It should only be warnings though nothing dramatic so let's merge and see if we get more feedback than expected :)

@Seldaek
Copy link
Member

Seldaek commented May 31, 2024

@PrinsFrank already got a bunch of errors on the Composer repo due to test fixtures😅

One last one I cannot fix immediately tho is in a dependency:

Class SymfonyExcludeListSimplePhpunit located in ./vendor/symfony/phpunit-bridge/bin/simple-phpunit.php does not comply with psr-4 autoloading standard (expected namespace Symfony\Bridge\PhpUnit\). Skipping.

So my guess is it will cause more pain than we think..

@PrinsFrank
Copy link
Contributor Author

@Seldaek these warnings only really make sense in the composer project itself, right? Errors in autoloaded classes can be more easily found if the strict-psr is run in the project itself, but if a dependency ships with non-psr4 compliant classes I don't think it makes sense to show these errors in dependant packages?

@Seldaek
Copy link
Member

Seldaek commented May 31, 2024

Right yes that's true. I need to see if I can ignore dependencies. It's not that easy given the API we have here.

@PrinsFrank
Copy link
Contributor Author

I can look into this this weekend as well if you want? I'd love to help out right now, but I'm ill at the moment. ;)

@Seldaek
Copy link
Member

Seldaek commented May 31, 2024

Nah don't worry I am on it. 6307dcf fixes some of the problems already. And the new method added there will let me clean up vendor warnings in Composer.

Hope you get well soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants