Skip to content

4.0 | Proposal: more consistent type handling of ruleset-set custom properties #708

Closed
@jrfnl

Description

@jrfnl

Current Situation

Follow up on/Inspired by #704.

End-users can set the values for public properties on sniffs via the XML ruleset.

PHPCS receives these values as strings and includes special handling for a few situations:

Value in XML ruleset Value set for the sniff property
'' (empty string) null (null)
'null' null (null)
'false' false (boolean)
'true' true (boolean)

As things are, this special handling:

  • Is not being applied when 'null', 'true' or 'false' are not received in lowercase.
    I.e. a property value of 'False' will remain the string 'False'.
  • Is not applied to the values of array elements being set on an array property.

It is also not applied to the value of array keys.

Proposal

Proposal - part 1

I'd like to propose to apply the special handling of property values more consistently.

Value in XML ruleset Value set for the sniff property in PHPCS 3.x Value set for the sniff property as of PHPCS 4.0
'' (empty string) null (null) null (null)
'null' 'null' (string) null (null)
'false' false (boolean) false (boolean)
'true' true (boolean) true (boolean)
'NULL' (and all other case variations) 'NULL' (string) null (null)
'FALSE' (and all other case variations) 'FALSE' (string) false (boolean)
'TRUE' (and all other case variations) 'TRUE' (string) true (boolean)

Proposal - part 2

I'd also like to propose to apply the same special handling to array element values as of PHPCS 4.0.

What will not change ?

I do NOT intend to change the handling of array keys. These will remain strings at all times.
Changing that could have far bigger breaking impact due to how PHP casts all array keys to integers/strings and duplicate keys are not allowed.

I also do NOT intend to add special handling for integer or float property values at this time.

Impact and timeline

Sniffs which explicitly expect string values for all array elements in a public property may break due to this change if the sniff only takes string values for array elements into account.

Sniffs may also contain special handling to work around the above listed short-comings of the property handling. Especially for array values, where PHPCS currently has no special handling, I can imagine that this sniff-specific special handling could break with this change.

So, with the above in mind, I'm proposing to make this change in the 4.0 release and not before.

Opinions ?

Am I overlooking anything ? Are there more things which could break with this change ? Or do you think this will more likely fix some obscure bugs for end-users ?

Please leave a comment with your thoughts.

/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions