Skip to content

Conversation

@jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 10, 2023

Description

Recreation of upstream PR squizlabs/PHP_CodeSniffer#3833:

The Generic.PHP.LowerCaseType sniff calls the File::getMemberProperties() method to get information about potential properties.

That method throws an exception when the T_VARIABLE token passed is not a property, but will create an Internal.ParseError.InterfaceHasMemberVar warning and return an empty array when the T_VARIABLE passed is an illegal property, i.e. a property in a context in which it is not allowed (interface/enum).

As things were, the sniff did not take a potential return value of an empty array into account, which could result in an Undefined array key "type" PHP notice.

Fixed now.

Includes unit test.


Other relevant notes from the original PR:

This PR will conflict with PR squizlabs/PHP_CodeSniffer#3662 and either one of them will need rebasing if the other gets merged first.

👆🏻 That is PR #49 in this repo

Suggested changelog entry

Generic.PHP.LowerCaseType: fixed potential undefined array index notice

The `Generic.PHP.LowerCaseType` sniff calls the `File::getMemberProperties()` method to get information about potential properties.

That method throws an exception when the `T_VARIABLE` token passed is not a property, but will create an `Internal.ParseError.InterfaceHasMemberVar` warning and return an empty array when the `T_VARIABLE` passed is an _illegal_ property, i.e. a property in a context in which it is not allowed (interface/enum).

As things were, the sniff did not take a potential return value of an empty array into account, which could result in an `Undefined array key "type"` PHP notice.

Fixed now.

Includes unit test.
@jrfnl jrfnl force-pushed the feature/generic-lowercasetype-fix-php-notice branch from 3146735 to fed93f1 Compare December 5, 2023 12:45
@jrfnl jrfnl merged commit ddba452 into master Dec 5, 2023
@jrfnl jrfnl deleted the feature/generic-lowercasetype-fix-php-notice branch December 5, 2023 12:52
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