Add type hinting parameters

I think we should only use type hinting is the argument assumes a certain type for the argument. l() in Drupal 6, for example, assumes that the $options argument is array. If you don't pass it an array, it will result in a PHP error:

This is one example where type hinting really benefits because then it cleanly results in a PHP argument exception rather then an ugly PHP error.... Subscribing.

big +1 for this from me.

There seems to be different opinions on whether to write function foo(array $bar) or function foo(Array $bar). I suggest using the former, as this is the style used in the PHP manual page on type hinting. Also, the array() construct and the array type string use the lowercase variant. The most prominent exception I can find is print_r() that outputs Array( ... ).

Likewise I suggest using stdClass (this is written StdClass a few places in Drupal).

i agree with c960657. i work with Zend Framework, and it uses lowercase for type hinting array as well.

I also agree with #3. Here's an initial patch to standardize the capitalization of the current type hinting before I start searching for parameters that need type hinting.

patch in #5 does not change any functionality, so it is ready to go

Looking at the diff, I find Array $foo more readable than array $foo in differentiating the type hinting from actual parameters, so I'm not sure I agree with the proposed change.

I think the lower-case "array" makes more sense for the type hinting then the upper-case "Array". When creating a new instance of an array, Drupal coding standards ask for the lowercase:

So why not bring that standard over to the type hinting as well? Either that, or also make the creation require the uppercase as well:

I think we should keep both the instantiation and type the same case. Either have both the type and the creation use lowercase, or uppercase, not both. Would also be good to get Crell's thoughts on this.

I agree with Rob. It should be the same case as the coding standard. We shouldn't have two conflicting variable capitalization standard.

Yes, since Crell wrote most of those type hinting instances, I'd liek his input. As above - I don't claim it's consistent. Instead the difference as compared to instantiation (or casting) makes it more readable because it stands out.

I prefer the lowercase "array" for type hints. It's consistent with our widespread use of "array()".

The readability improvement (if worthwhile) would be better implemented with a more radical change to parameter variable naming. I see no reason why the type hints should stand out in function definitions instead of the actual parameter variable names:

function my_func(array $One, $Two, array $Three) {
}

versus Peter's suggestion:

function my_func(Array $one, $two, Array $three) {
}

I'm not saying we should change the standard, just that I think there would be more effective approaches than Peter's.

I'm also in favor of lower case. I committed Dave's patch. Thanks!

Setting this back to active to search for all the type-hinting needed now that we have a standard.

First pass with array type-hinting. My computer is too slow to run all tests, let's see how the testing bot fares.

common.inc changes did not pick up...

The last submitted patch failed testing.

Another round, actually can install now.

The last submitted patch failed testing.

I love how multiple people said that my input would be useful here and I didn't find out about the issue until after it was committed...

I discussed the case question with webchick months and months ago. I originally used capitals in DBTNG because in PHP the only things that can be type hinted are arrays and classes/interfaces. Classes/interfaces are always capitalized (in pretty much every convention, including Drupal core's now), so having all type hints be capitalized except for arrays didn't make much sense at all. It also made them stand out as different from array(), because they're a type hint, not a declaration. webchick agreed with that logic.

With that said, I don't care enough either way to make a huge fuss out of it. Lowercasing array is not wrong, just confusing when writing type hints. :-)

On the whole I am very +1 for adding more type hints, as they make the code more self-documenting and easier to debug since major errors will give a more useful error message. The caveats I will offer are:

1) Type hint all arrays iff we expect only an array, as is the case in l(). So by default don't type hint them unless you're sure. (I do plan to add more type hints to the DB layer once we get rid of the BC layer.)

2) Type hint classes/objects only with interfaces, not with actual class names. I know DBTNG violates that in a few places, but not many. :-) Type hinting to a class means you can only "fudge it" if you subclass from the parent class, which is not always possible. If you type hint to an interface, then you can pass in any object regardless of its hierarchy that implements that interface. That's much more flexible. See the changes made to DatabaseStatement in order to make the SQLite driver possible for a good example.

@Crell In standard C++ and Java code, primitives are lowercase and classes are camelcase. In PHP, arrays are primitives, so we would not be breaking with longstanding convention by keeping "array" lowercase yet preserving the camelcase of class names in hints.

Hm, I crossposted with the bot.

subscribing.

related issue, where type hinting can not be used: #351898-13: Database API is not E_STRICT compatible

Add tag.