Skip to content

Fixes for PHPStan level 5#282

Merged
acoulton merged 5 commits intoBehat:masterfrom
uuf6429:chore/phpstan-level-5
Feb 19, 2025
Merged

Fixes for PHPStan level 5#282
acoulton merged 5 commits intoBehat:masterfrom
uuf6429:chore/phpstan-level-5

Conversation

@uuf6429
Copy link
Contributor

@uuf6429 uuf6429 commented Feb 6, 2025

No description provided.

"autoload-dev": {
"psr-4": {
"Tests\\Behat\\": "tests/Behat/"
"Tests\\": "tests/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This helps:

  1. IDEs (e.g. PhpStorm or IDEA) correctly detect the tests directory as the "tests root"
  2. It decrease the wtf factor if someone adds test classes elsewhere which wouldn't load otherwise
  3. It may not be the best, performance-wise, but I think it's not so critical seeing as it only impacts tests (i.e. composer --dev).

Copy link
Member

Choose a reason for hiding this comment

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

My proposal would be to flatten the tests folder (thanks to PSR-4 possibilities) but keeping a prefix that enforce using a proper namespace of that library (I would even use Tests\Behat\Gherkin as prefix)

Copy link
Contributor Author

@uuf6429 uuf6429 Feb 19, 2025

Choose a reason for hiding this comment

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

I wouldn't be against it, but then it does become a bit inconsistent with src:

(although I suppose we can do it for src too - is a change in filesystem structure considered a BC break?)

Copy link
Member

Choose a reason for hiding this comment

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

it is not a BC break (assuming you update the autoload config to be valid for the new path), unless your project requires users to hardcode a path to a file inside your package (which is not the case for Gherkin)

@uuf6429 uuf6429 requested a review from stof February 11, 2025 20:01
@uuf6429
Copy link
Contributor Author

uuf6429 commented Feb 18, 2025

@stof all points that have been raised are solved, anything else to move forward on this PR?

Roll back "yield from" to "return".
@acoulton acoulton merged commit 482a58e into Behat:master Feb 19, 2025
8 checks passed
@uuf6429 uuf6429 deleted the chore/phpstan-level-5 branch February 20, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants