Problem/Motivation

There are cases where we use value objects, or where providing an interface for an otherwise one-off class will do more harm than good.

Benefits

If we adopted this change, the Drupal Project would benefit by ...

Three supporters required

  1. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
  2. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
  3. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)

Proposed changes

Provide all proposed changes to the Drupal Coding standards. Give a link to each section that will be changed, and show the current text and proposed text as in the following layout:

1.https://www.drupal.org/docs/develop/standards/coding-standards#s-objects

Current text

Use an interface typehint for parameter and return types (not a class). Typehint the most specific interface that encompasses all possible parameter or return values. Do not use stdClass.

Proposed text

Use an interface typehint for parameter and return types where available. Typehint the most specific interface or class that encompasses all possible parameter or return values.

2. https://www.drupal.org/docs/develop/standards/php/object-oriented-code#i...

Current text

Use of interfaces

The use of a separate interface definition from an implementing class is strongly encouraged because it allows more flexibility in extending code later. A separate interface definition also neatly centralizes documentation making it easier to read. All interfaces should be fully documented according to established documentation standards.

If there is even a remote possibility of a class being swapped out for another implementation at some point in the future, split the method definitions off into a formal Interface. A class that is intended to be extended must always provide an Interface that other classes can implement rather than forcing them to extend the base class.

Proposed text

None. The text is to be deleted.

3. https://www.drupal.org/docs/develop/standards/php/object-oriented-code#h...

Current text

Type hinting

PHP supports optional type specification for function and method parameters for classes and arrays. Although called "type hinting" it does make a type required, as passing an object that does not conform to that type will result in a fatal error.

  • DO specify a type when conformity to a specific interface is an assumption made by the function or method. Specifying the required interface makes debugging easier as passing in a bad value will give a more useful error message.
  • DO NOT use a class as the type in type hinting. If specifying a type, always specify an Interface. That allows other developers to provide their own implementations if necessary without modifying existing code.

Example:

// Correct:
function make_cat_speak(FelineInterface $cat) {
  print $cat->meow();
}

// Wrong:
function make_cat_speak(GarfieldTheCat $cat) {
  print $cat->meow();
}
Proposed text

None. The text is to be deleted.

Remaining tasks

\
Needs examples.

  1. Create this issue in the Coding Standards queue, using the defined template
  2. Add supporters
  3. Create a Change Record
  4. Review by the Coding Standards Committee
  5. Coding Standards Committee takes action as required
  6. Discussed by the Core Committer Committee, if it impacts Drupal Core
  7. Final review by Coding Standards Committee
  8. Documentation updates
    1. Edit all pages
    2. Publish change record
    3. Remove 'Needs documentation edits' tag
  9. If applicable, create follow-up issues for PHPCS rules/sniffs changes

For a full explanation of these steps see the Coding Standards project page

Possible follow up to remove to remove https://www.drupal.org/docs/develop/standards/object-oriented-code.

Comments

catch created an issue. See original summary.

catch’s picture

Title: Allow type hinting of classes » Allow type hinting with classes where appropriate
xjm’s picture

I think we should include of examples of a couple places an interface should be used and a couple where a class should be used.

alexpott’s picture

Can we change
"Use an interface typehint for parameter and return types where available. Typehint the most specific interface that encompasses all possible parameter or return values."

to
"Use an interface typehint for parameter and return types where available. Typehint the most specific interface or class that encompasses all possible parameter or return values."

xjm’s picture

As part of this, we should also delete the redundant section at https://www.drupal.org/docs/develop/standards/object-oriented-code#hinting as it's the source of the "typehint interfaces only" coding standard, and write a CR when we adopt this I'd say.

catch’s picture

Issue summary: View changes
xjm’s picture

Can we list out examples of when things should be hinted as classes or interfaces? And possibly reference a different section that documents some examples of when typehinting a class is actually preferable?

These are the other sections we need to change for parity:

The use of a separate interface definition from an implementing class is strongly encouraged because it allows more flexibility in extending code later. A separate interface definition also neatly centralizes documentation making it easier to read. All interfaces should be fully documented according to established documentation standards.

If there is even a remote possibility of a class being swapped out for another implementation at some point in the future, split the method definitions off into a formal Interface. A class that is intended to be extended must always provide an Interface that other classes can implement rather than forcing them to extend the base class.

and:

PHP supports optional type specification for function and method parameters for classes and arrays. Although called "type hinting" it does make a type required, as passing an object that does not conform to that type will result in a fatal error.

DO specify a type when conformity to a specific interface is an assumption made by the function or method. Specifying the required interface makes debugging easier as passing in a bad value will give a more useful error message.

DO NOT use a class as the type in type hinting. If specifying a type, always specify an Interface. That allows other developers to provide their own implementations if necessary without modifying existing code.

Obviously, both of these are kinda bad advice, since there are completely reasonable usecases to not have an interface. If we could instead write a short paragraph about when interfaces make sense and when they don't, we could get rid of the old docs.

(We also can get rid of the bits explaining what typehinting is; it's not some new weird concept and if someone doesn't know, there are better references for PHP basics.)

So let's rewrite those two sections as part of the proposed resolution.

catch’s picture

I think we should remove https://www.drupal.org/docs/develop/standards/object-oriented-code entirely. The two sections in #3263602-7: Allow type hinting with classes where appropriate are either duplicating the point in the main document, or irrelevant.

quietone’s picture

Issue summary: View changes

Convert to new template

quietone’s picture

Issue summary: View changes

Updated the issue summary with items in #7.

quietone’s picture

Issue summary: View changes

Update the issue summary #8.

That leaves adding examples as suggested in #3 and #7

kingdutch’s picture

The proposed improvement in #1158720: Improve text for parameter type hinting in function declaration shows that the type-hinting block is likely not only about interfaces (3 in this issue). As such I don't think it should be removed in its entirety, but we likely want to combine both changes. The wording proposed in that issue may already give more room for this issue since it qualifies interface usage with "if available", which I still think is what people should be doing.