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

Squiz/ScopeKeywordSpacing: add more defensive coding + extra tests #150

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 10, 2023

Description

Squiz/ScopeKeywordSpacing: rename the test case file

... to allow for adding additional test case files for this sniff.

Squiz/ScopeKeywordSpacing: add more defensive coding

When the sniff would encounter a parse error/be run during live coding, the sniff could trigger an Undefined array key notice for the use of $stackPtr + 2.

Fixed now by bowing out early if the keyword is found at the end of a file.

Includes additional tests.

Squiz/ScopeKeywordSpacing: add extra tests with static in a union type

This is already handled correctly, the tests just safeguards this.

Suggested changelog entry

  • Squiz/ScopeKeywordSpacing: prevent a PHP notice during live coding

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

... to allow for adding additional test case files for this sniff.
When the sniff would encounter a parse error/be run during live coding, the sniff could trigger an `Undefined array key` notice for the use of `$stackPtr + 2`.

Fixed now by bowing out early if the keyword is found at the end of a file.

Includes additional tests.
This is already handled correctly, the tests just safeguards this.
@jrfnl jrfnl force-pushed the feature/squiz-scopekeywordspacing-defensive-coding branch from f75abc3 to 1a046ac Compare December 15, 2023 12:56
@jrfnl jrfnl merged commit 8b61482 into master Dec 15, 2023
44 checks passed
@jrfnl jrfnl deleted the feature/squiz-scopekeywordspacing-defensive-coding branch December 15, 2023 13:02
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.

1 participant