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

Ruleset::setSniffProperty(): add test for edge case / unused sniff #753

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 3, 2024

Description

I haven't been able to come up with a real situation in which this condition could result in an early return - either via reading an XML ruleset or via inline phpcs:set annotation. Having said that, the method is public and is regularly used in test frameworks for external standards, so this code should remain in place.

This commit now safeguards the behaviour via a test.

Suggested changelog entry

N/A

Related issues/external references

This PR is part of a series of PRs expanding the tests for the Ruleset class.

I haven't been able to come up with a _real_ situation in which [this condition](https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/15db7232015d4fc1e023ef1eff0e65777a906f2c/src/Ruleset.php#L1476-L1479) could result in an early return - either via reading an XML ruleset or via inline `phpcs:set` annotation. Having said that, the method is `public` and is regularly used in test frameworks for external standards, so this code should remain in place.

This commit now safeguards the behaviour via a test.
@jrfnl jrfnl added this to the 3.11.2 milestone Dec 7, 2024
@jrfnl jrfnl merged commit 32c5cd9 into master Dec 7, 2024
59 checks passed
@jrfnl jrfnl deleted the feature/ruleset-setsniff-property-edge-case-test-unused-sniff branch December 7, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants