FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Sanity check `#[RequiresPhp]` value and range by staabm · Pull Request #269 · phpstan/phpstan-phpunit · GitHub

/ phpstan-phpunit Public

Sanity check #[RequiresPhp] value and range - #269

Merged
staabm merged 20 commits into
phpstan:2.0.xfrom
staabm:sanity
Jun 19, 2026
ConversationCommits (20)ChecksFiles changed
Merged

Sanity check #[RequiresPhp] value and range#269
staabm merged 20 commits into
phpstan:2.0.xfrom
staabm:sanity

Conversation

staabm commented Dec 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • when #[RequiresPhp] is lower then the phpstan analysis version, reports a test requirement as beeing always false
  • when #[RequiresPhp] contains a version which is not parse-able, reports a error

staabm commented Dec 5, 2025
View reviewed changes
Comment thread src/Rules/PHPUnit/AttributeRequiresPhpVersionRule.php Outdated
$this->deprecationRulesInstalled = $deprecationRulesInstalled;

$parser = new VersionParser();
$this->phpstanVersionConstraint = $parser->parseConstraints($phpVersion->getVersionString());

staabm Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

maybe this should compare against the composer.json min version instead of the phpstan.neon version?

ondrejmirtes commented Dec 8, 2025

Copy link
Copy Markdown
Member

I'd rather use https://github.com/phar-io/version which is also used by PHPUnit I think https://github.com/sebastianbergmann/phpunit/blob/2d6dc600b52cd74141c422a5d0601ca26c185bb9/src/Metadata/Version/Requirement.php

Yeah, comparing against PHP_VERSION_ID would take care of all issues. We just need to properly understand the constraints in all cases.

staabm commented Dec 8, 2025

Copy link
Copy Markdown
Contributor Author

I'd rather use phar-io/version which is also used by PHPUnit I think sebastianbergmann/phpunit@2d6dc60/src/Metadata/Version/Requirement.php

thats only one part of the story. see also https://github.com/sebastianbergmann/phpunit/blob/2d6dc600b52cd74141c422a5d0601ca26c185bb9/src/Metadata/Version/Requirement.php#L41-L48 which adds a fallback path on top which is implemented in phpunit itself.

reworked it to match that

staabm mentioned this pull request Dec 15, 2025
Merged
staabm marked this pull request as draft December 17, 2025 06:59

ondrejmirtes commented Feb 9, 2026

Copy link
Copy Markdown
Member

The related PR was merged so you can resume your work on this. Thank you.

staabm mentioned this pull request Apr 16, 2026
Merged
staabm mentioned this pull request Jun 20, 2026
Open
staabm marked this pull request as ready for review June 18, 2026 15:12
staabm requested a review from VincentLanglet June 18, 2026 15:12
This was referenced Jun 18, 2026
Closed
Merged

staabm commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, comparing against PHP_VERSION_ID would take care of all issues. We just need to properly understand the constraints in all cases.

I got an idea for this one. I will push it tomorrow into this PR

staabm mentioned this pull request Jun 19, 2026
Merged

staabm commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@VincentLanglet finally, this is really ready for review :-)

VincentLanglet reviewed Jun 19, 2026
View reviewed changes
Comment thread tests/Rules/PHPUnit/AttributeRequiresPhpVersionRuleTest.php

$this->analyse([__DIR__ . '/data/requires-php-version-mismatch.php'], [
[
'Version requirement will always evaluate to false.',

VincentLanglet Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

#[RequiresPhp('<= 8.5')] will always evaluate to false ?

staabm Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

because the test assumes PHP_VERSION_ID 80500 and the constraint only has 2 digits.
it would not error for '<= 8.5.0'.

this is what #303 is about

VincentLanglet Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That's tricky.

Should we have a test with 8.6 to ensure this one is not reported ?

staabm Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done

staabm merged commit bf910b5 into phpstan:2.0.x Jun 19, 2026
95 checks passed

staabm commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

(needs to wait for the PHPStan 2.2.3 release before extension release)

staabm deleted the sanity branch June 20, 2026 17:42
This was referenced Jun 25, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

VincentLanglet VincentLanglet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL