Skip to content

[feature-request] behaviour-per-method should catch a nullable parameter that means 'all of them' #524

Description

@jessegall

Proposal:
The rule is written entirely around bool. The commonest disguise in this codebase is a required parameter WIDENED to nullable, where null selects a different question: Inspector::getAttributes(string $attribute) became getAttributes(string|null $attribute = null), so getAttributes(Slot::class) asks 'attributes of this kind' and getAttributes() asks 'every attribute it carries'. That is two questions behind one name, picked by a literal absence at the call site — which says even less than a bare true, because nothing is written there at all. It also arrives disguised as an additive, backward-compatible change, which is why it passes review. The fix is the same as for a flag: two named methods, getAttributes(string) and attributes(). Suggest the principle gain a section on nullable-as-mode, the description mention widening a required parameter to nullable so it can mean 'all', and FlagArgumentDetector consider flagging a parameter whose null branch takes a wholly different path.

Filed via commandments feature-request from a consumer project.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions